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.
This is a show stopper for upgrading to 2.0. The GTK open file dialog is trash for a regular windows user. There is no longer a way to open an explorer window OOB.
I suspect those dialog are crashing because you are not passing the right memory allocator to them, perhaps without the right memory options, eg. not requesting global shared, instead of default scope. Not you directly perhaps, but the GTK code.
To be clear, there have been no contributions by Windows users and the previous Windows dialog code related to a crash. Since no Windows contributors addressed the problem, the only option the non-windows contributors had was to remove the windows related code to avoid the crash.
Perhaps "somebody" could try GtkFileChooserNative as was mentioned above, but again none of the Geany contributors use Windows.
Isn't the question rendered moot by #3861? It's gonna be in 2.1, whenever that's released.
Didn't umm, thingy, you know, whatchamacallit .. oh yeah, remember. ;-)
Thanks for the info. I found a recent 2.1 CI build artifact to install. It restores the familiar windows native file dialogs. It works great, so I am going to use it going forward.
Is there anyone out there who has built Geany on a real Windows box with MSYS2 installed? I am thinking about trying that again (I tried with 1.35 or so, IIRC).
Here is one but I'm not proud of it :).
There is also a howto on https://wiki.geany.org/howtos/win32/msys2.
It might be easier to choose the latest CI build from https://github.com/geany/geany/actions/workflows/build.yml?query=event%3Apus... and download the installer from the artifacts section.
For further questions and comments, it'd be if you could open a new discussion or issue.
github-comments@lists.geany.org