For some reason "project-save" isn't emitted when closing project - see write_config(FALSE) in project_close(). This means that in this case plugins cannot save their configuration into the config file. This doesn't even correspond to the documentation of the signal
"Sent when a project is saved (happens when the project is created, the properties dialog is closed or Geany is exited)"
as the signal isnt emitted when exiting Geany because at this point Geany closes the project.
The comment seems to indicate that the reason is that "project-save" shouldn't be emitted when "project-close" is emitted but I don't see any reason why.
Bump API so plugins can rely on the changed behavior. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1400
-- Commit Summary --
* Always emit the project-save signal when writing project file
-- File Changes --
M src/plugindata.h (2) M src/project.c (20)
-- Patch Links --
https://github.com/geany/geany/pull/1400.patch https://github.com/geany/geany/pull/1400.diff
I ran into this problem when working on https://github.com/geany/geany-plugins/pull/528 - I wanted to improve it further to save/load the list of expanded directories from the sidebar to/from the project file and it didn't work.
@eht16 Do you still remember why you did it this way? Git blame shows you as the author:
https://github.com/geany/geany/commit/748d55216874be532570d5fc5a19958cd12129...
I guess there must have been some reason but I don't understand it.
I also went through the plugins which use this signal and there should be no problem for existing plugins with the changed semantics:
- lua, geanypy: just forward this signal to plugins - projectorganizer, geanyctags: my plugins, there's no problem - treebrowser: seems to update the tree on the signal but there should be no problem with that
LGBI
So much has changed since 2008 (when the original commit happened) it may not matter what the original thought behind it was.
Sorry, I don't remember the reason :(. The fact that I mixed this change with other unrelated changes, doesn't help much.
The changes here look good to me.
It might be good to add to API docs:
Sent when a project is saved (happens when the project is created, the properties dialog is closed, before the project is closed, or when Geany is exited).
Also could be useful to indicate when it happens with respect to "project-before-close" and "project-close", for example:
When a project is closing, the "project-save" signal is emitted after "project-before-close" and before "project-close".
Or something like that.
b4n requested changes on this pull request.
Apart from my pickyness, I don't see why not emitting this signal either, and this change shouldn't lead to logic issues in plugins. So LGBI minus the prototype thing.
@@ -83,7 +83,7 @@ typedef struct _PropertyDialogElements
static gboolean update_config(const PropertyDialogElements *e, gboolean new_project); static void on_file_save_button_clicked(GtkButton *button, PropertyDialogElements *e); static gboolean load_config(const gchar *filename); -static gboolean write_config(gboolean emit_signal); +static gboolean write_config();
C prototype!! `(void)`
- Returns: TRUE if project file was written successfully. */
-static gboolean write_config(gboolean emit_signal) +static gboolean write_config()
ditto
Also, @codebrainz point is good, while at it the docs could be further improved. I'm not 100% sure we should advertise the very order of the signals, but if it can be useful I don't really see why not either (I doubt having it defined would prevent it to make some otherwise compatible changes in the future).
Aha, now I think I know why @eht16 did it - just noticed that this signal is also used in the filebrowser plugin. When "use the project's base directory" is enabled, the "project-save" signal is used to update the directory in the file browser (this signal is probably used so it gets updated also when base path changes in project's config). But you don't want to change the directory when closing the project.
One way to fix it with the new behavior is to have a flag that is set in "project-before-close" and reset in "project-close" handler to indicate whether project closing happens.
But maybe it would be even better to simply remove the "project-save" handler and do the directory change only in "project-open" - I'm not sure if it's a good idea to change the directory when project settings gets updated (imagine you changed the directory to some subdirectory of the project because you work there and just want to update a command-line flag of the compiler in settings - and BAM, your directory is changed).
OK, I updated the patch and also added a patch for the filebrowser (with the removed "project-save" handler which I find better).
I've also updated the signal documentation wording as @codebrainz suggested but didn't explicitly mention the call order of the signal (agree with @b4n but the truth is that I can't imagine how the signals could actually be emitted in a different order).
LGTM
I will be running with this change to see whether it breaks anything for me.
For now, I briefly tried it out with some of my project-related workflows (opening a project from the command line and from the GUI, changing build commands, opening another project, etc.), on Linux GTK+2 and GTK+3. Nothing unexpected so far.
Also I agree with [this](https://github.com/geany/geany/pull/1400#issuecomment-281097024):
I'm not sure if it's a good idea to change the directory when project settings gets updated (imagine you changed the directory to some subdirectory of the project because you work there and just want to update a command-line flag of the compiler in settings - and BAM, your directory is changed)
I don’t use File Browser, but I do use TreeBrowser, which had the same behavior until @techee [fixed it](https://github.com/geany/geany-plugins/pull/536), and it was very annoying.
Merged in e43c8d831445669bd94f960ff289bc1155668cd0 and 3e864317ba051512c102beb2a939caa4698515de.
Closed #1400.
github-comments@lists.geany.org