New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display full path in recent files menu #1992

Merged
merged 2 commits into from Sep 11, 2018

Conversation

Projects
None yet
2 participants
@gogoprog
Contributor

gogoprog commented Aug 19, 2018

This is useful when different files have the same name

@bjorn

This comment has been minimized.

Owner

bjorn commented Aug 19, 2018

@gogoprog gogoprog force-pushed the firefalcom:patch-1 branch from 2af4b04 to 1b08ea0 Sep 10, 2018

@gogoprog

This comment has been minimized.

Contributor

gogoprog commented Sep 10, 2018

Thanks for reviewing,
here are my new changes

@@ -1328,6 +1329,7 @@ void MainWindow::updateRecentFilesMenu()
mRecentFiles[i]->setText(QFileInfo(files[i]).fileName());
mRecentFiles[i]->setData(files[i]);
mRecentFiles[i]->setVisible(true);
mRecentFiles[i]->setToolTip(QFileInfo(files[i]).filePath());

This comment has been minimized.

@bjorn

bjorn Sep 10, 2018

Owner

Just a small suggestion: please re-use the QFileInfo instance. Not that the code is performance-critical, but in general it's good to avoid duplicating work.

This comment has been minimized.

@gogoprog

gogoprog Sep 10, 2018

Contributor

Sure, I just updated

@gogoprog gogoprog force-pushed the firefalcom:patch-1 branch from 1b08ea0 to 18ca439 Sep 10, 2018

@gogoprog gogoprog force-pushed the firefalcom:patch-1 branch from 18ca439 to ac8f73d Sep 10, 2018

Small style adjustments
* Coding style places & or * against the variable name
* Generally only using auto when it reduces code size

@bjorn bjorn merged commit 1675ced into bjorn:master Sep 11, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@bjorn

This comment has been minimized.

Owner

bjorn commented Sep 11, 2018

Nice addition, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment