Skip to content
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

fix #99626 Preferences obey nativeDialogs setting in .ini #2406

Merged

Conversation

@ericfont
Copy link
Contributor

commented Feb 25, 2016

If on linux, Preferences dialogs for setting folders and files will now obey nativeDialog boolean setting from ini.

Note: this commit doesn't change behavior for Windows and OS X, since Qt says getExistingDirectory() and getOpenFileName() do not obey the QFileDialog::DontUseNativeDialog option.

@ericfont

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2016

passed Travis, and I've tested on my windows x86-64, ArchLinux i686, and ArchLinuxARMv7 machines, and works as I described in commit message.

this,
tr("Choose a directory"),
QString("%1/%2").arg(QDesktopServices::storageLocation(QDesktopServices::DocumentsLocation)).arg(QCoreApplication::applicationName()),
preferences.nativeDialogs ? QFileDialog::Options() : QFileDialog::DontUseNativeDialog

This comment has been minimized.

Copy link
@lasconic

lasconic Feb 25, 2016

Member

If I'm not wrong, the default is QFileDialog::ShowDirsOnly http://doc.qt.io/qt-5/qfiledialog.html#getExistingDirectory. It's probably safer to pass it to not change the bahavior.

This comment has been minimized.

Copy link
@ericfont

ericfont Feb 25, 2016

Author Contributor

ok, I understand what you're saying. so is it better to have a big if statement for all of these?

EDIT: or I could just OR (|) that big statement with the flag QFileDialog::ShowDirsOnly.

This comment has been minimized.

Copy link
@lasconic

lasconic Feb 25, 2016

Member

Something like
QFileDialog::ShowDirsOnly | (preferences.nativeDialogs ? 0 : QFileDialog::DontUseNativeDialog) works for me.

This comment has been minimized.

Copy link
@ericfont

ericfont Feb 25, 2016

Author Contributor

I was having type-casting problems (EDIT: Compiler errors) with 0. Which is why I had to do the dummy zero flag "QFileDialog::Options()"

This comment has been minimized.

Copy link
@ericfont

ericfont Feb 25, 2016

Author Contributor

CORRECTION: Looks like I am able to compile with the full large statment, I believe because the QFileDialog::ShowDirsOnly acts as a cast. I'll make that change for all and resubmit.

@ericfont ericfont force-pushed the ericfont:99626-preferences-obey-nativeDialogs-ini branch from 69b7abe to ccaac15 Feb 25, 2016

@ericfont

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2016

ok, this should be good.

@ericfont

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2016

I believe I need to use QFileDialog::Options() instead of 0.

@ericfont ericfont force-pushed the ericfont:99626-preferences-obey-nativeDialogs-ini branch from ccaac15 to 9b212d1 Feb 25, 2016

@lasconic

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

Oops, you need this only for getExistingDirectory not for getOpenFileName

fix #99626 Preferences obey nativeDialogs setting in .ini
If on linux, Preferences dialogs for setting folders and files will now obey nativeDialog boolean setting from ini.

Note: this commit doesn't change behavior for Windows and OS X, since Qt says getExistingDirectory() and getOpenFileName() do not obey the QFileDialog::DontUseNativeDialog option.

@ericfont ericfont force-pushed the ericfont:99626-preferences-obey-nativeDialogs-ini branch from 9b212d1 to fefc676 Feb 25, 2016

@lasconic

This comment has been minimized.

Copy link
Member

commented Feb 25, 2016

thanks!

@ericfont

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2016

ok. I believe I've updated correctly now: The three getOpenFileName don't have the ShowDirsOnly:

  • selectInstrumentList1()
  • selectInstrumentList2()
  • selectStartWith()

and the rest are getExistingDirectory and do include ShowDirsOnly.

(I apologize...I did indeed read that message "To ensure a native file dialog, ShowDirsOnly must be set." when I was initially making this PR, but I didn't think needed to include that, as I remember trying it out... I think Qt could have written that sentence a bit clearer.)

@ericfont

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2016

I've built and check on my linux machine. Behaves as expected with nativeDialogs=true and =false.

lasconic added a commit that referenced this pull request Feb 26, 2016
Merge pull request #2406 from ericfont/99626-preferences-obey-nativeD…
…ialogs-ini

fix #99626 Preferences obey nativeDialogs setting in .ini

@lasconic lasconic merged commit f874f24 into musescore:master Feb 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.