Their implementation was buggy, use very old Windows APIs and require double implementation and maintenance efforts for us The GTK dialogs work well for all other users already, so they probably will also for Windows users.
Closes #3209. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3219
-- Commit Summary --
* Remove native file and color dialogs on Windows
-- File Changes --
M doc/geany.txt (4) M src/dialogs.c (46) M src/keyfile.c (2) M src/prefs.c (8) M src/project.c (105) M src/tools.c (9) M src/ui_utils.h (2) M src/win32.c (443) M src/win32.h (16)
-- Patch Links --
https://github.com/geany/geany/pull/3219.patch https://github.com/geany/geany/pull/3219.diff
@eht16 commented on this pull request.
@@ -63,7 +63,7 @@ typedef struct GeanyInterfacePrefs
gboolean msgwin_compiler_visible; /**< whether message window's compiler tab is visible */ gboolean msgwin_messages_visible; /**< whether message window's messages tab is visible */ gboolean msgwin_scribble_visible; /**< whether message window's scribble tab is visible */ - /** whether to use native Windows' dialogs (only used on Windows) */ + /** whether to use native Windows' dialogs - ignored and not used anymore */
Since this field is part of the API (and ABI), we cannot easily remove it.
Is it enough to mark it as ignored in the comment or are there better ways to handle this?
@eht16 pushed 1 commit.
f793f722a8e474d380cc7e209808925896c0d5ad Remove native file and color dialogs on Windows
@elextr commented on this pull request.
@@ -63,7 +63,7 @@ typedef struct GeanyInterfacePrefs
gboolean msgwin_compiler_visible; /**< whether message window's compiler tab is visible */ gboolean msgwin_messages_visible; /**< whether message window's messages tab is visible */ gboolean msgwin_scribble_visible; /**< whether message window's scribble tab is visible */ - /** whether to use native Windows' dialogs (only used on Windows) */ + /** whether to use native Windows' dialogs - ignored and not used anymore */
I would suspect that there are not many Windows specific plugins out there, so probably ok.
@elextr commented on this pull request.
@@ -63,7 +63,7 @@ typedef struct GeanyInterfacePrefs
gboolean msgwin_compiler_visible; /**< whether message window's compiler tab is visible */ gboolean msgwin_messages_visible; /**< whether message window's messages tab is visible */ gboolean msgwin_scribble_visible; /**< whether message window's scribble tab is visible */ - /** whether to use native Windows' dialogs (only used on Windows) */ + /** whether to use native Windows' dialogs - ignored and not used anymore */
Actually so long as it is still initialized to say "Windows dialogs not used" then its definitely ok.
@elextr approved this pull request.
- if (app->project && !EMPTY(app->project->base_path))
Why is adding this shortcut removed?
- if (app->project && !EMPTY(app->project->base_path))
Bah!! ignore comment, githubs annoying handling of changed indent confused me.
Just remembered that there was #2794 that was "fixed" by using native dialogs once after install?
@eht16 commented on this pull request.
- if (app->project && !EMPTY(app->project->base_path))
Yeah, sorry. The diffs are bit hard to read because of indentation changes :(:
@eht16 commented on this pull request.
@@ -63,7 +63,7 @@ typedef struct GeanyInterfacePrefs
gboolean msgwin_compiler_visible; /**< whether message window's compiler tab is visible */ gboolean msgwin_messages_visible; /**< whether message window's messages tab is visible */ gboolean msgwin_scribble_visible; /**< whether message window's scribble tab is visible */ - /** whether to use native Windows' dialogs (only used on Windows) */ + /** whether to use native Windows' dialogs - ignored and not used anymore */
It's not initialized explicitly at all. But at least on my Debian Testing, the resulting value when Geany is running is `FALSE`.
Just remembered that there was #2794 that was "fixed" by using native dialogs once after install?
Yes. This PR is the most easiest solution to solve the one problem in #3209. Fixing the dialog crash or even better port the Windows native code to more modern APIs would be far better without a doubt. But I won't do this (not only because after I would have done there were probably more bugs in the code than before :D). For me, it's a trade-off how to spend the available time (as usual and as you most probably agree on).
I'm happy to close this PR unmerged if we could find anyone else volunteering to do the good way.
I'm happy to close this PR unmerged if we could find anyone else volunteering to do the good way.
If it wasn't that #3209 is a crash I'd say leave the windows dialogs until GTK4 forces them out, after all they have been used for a long time without issues.
But since its a crash I'd say merge it, if any white knight rides in on their trusty steed I suspect the olde code would not be much help since the Windows dialog APIs have changed, but they can always choose to start with the commit before merge of after, their choice.
Or perhaps someone will try https://docs.gtk.org/gtk3/class.FileChooserNative.html and see if it works better on windows/macos and flatpack (see docs).
Interesting, didn't know about GtkFileChooserNative. This seems to be available in gtk4 as well.
If nobody stops me, I would merge this in a week or so. This should not hinder any experiements with GtkFileChooserNative at all.
Merged #3219 into master.
github-comments@lists.geany.org