Added suffix handling for Qt SAVE file dialogs#1727
Conversation
|
@wolfpackmars2 This looks much better than the last attempt. Simple and localized to the one file! Unfortunately my Linux box is still broken so I'll have to let someone else test this. I might just trust that you tested it enough since it's so simple and restricted to QT ;-) @WickedSmoke can you give it a look, if not an actual build/test? |
| public: | ||
| QFileDialog fileDialogQ; | ||
| QStringList filters; | ||
| QStringList defaultSuffixes; |
There was a problem hiding this comment.
There's no need to add new data members. I edited this patch to remove them on my qt-ext3 branch.
|
|
||
| // NOTE: The file extension is automatically set by the KDE native | ||
| // dialog but not with the Qt one. | ||
| // dialog and now with the Qt one. |
There was a problem hiding this comment.
This is a comment on the behavior of the platform dialogs, not about this class. I've updated the comment to include Gtk on my qt-ext3 change.
There was a problem hiding this comment.
This is a comment on the behavior of the platform dialogs, not about this class.
this was just a last minute "cheeky" edit.
I will get these tested sometime this week and then get this closed out if no concerns are identified. I'm sure it's fine, just haven't had a chance to look at it yet.
Thanks for reviewing this for me @WickedSmoke.
|
@WickedSmoke Without the defaultSuffixes and related code, the Qt dialog would not automatically add the file suffix, unless the user first modified the Filters combo box. I did update the comments slightly. Long story short, the edits in the qt-ext3 branch will force a file extension, when the user initiates the change in the dialog box, but it didn't set the "default" file suffixes. I considered dropping the added bool, but using it didn't seem any less efficient than querying the .acceptMode of the dialog. |
|
@WickedSmoke I tested your https://github.com/WickedSmoke/solvespace_qt5/ branch at commit 2efb82c with Qt5 on Windows and it works fine. The .slvs. extension is added. Of course with the My suggestion is to merge that branch into master and close this PR. @phkahler, @wolfpackmars2 what do you think? |
4 participants
Resolves #1689
Not much else to say, the code is straightforward and matches how guigtk.cpp handles file extensions as closely as possible.
This also fix how file extensions are handled for other export file types, like .stl etc.
I have functionally tested this in Ubuntu 24.04.