Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Merged #2114 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#event-2690022642
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Great. FWIW, I've been using this patch since a while and have no problems so far. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-537854753
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> Hopefully you'll squash or rebase to avoid the back merge? Done. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-537834548
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 3 commits. 8763d59c7ab9612bf7af1479c6ed964fd7acbe3d Save main and project configuration whenever documents are opened/closed f1f7d9ebe5a153e508394fec9eb03a1bbe5739e1 Add setting, defaulting to TRUE, to disable automatic configuration save 3a9ae46e2ea88a0f1ad734913881b01e6051e672 Document new emission of "project-save" signal -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/1a558f5cc9e362586da864f1cf7ee80fa1951a32..3a9ae46e2ea88a0f1ad734913881b01e6051e672
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Hopefully you'll squash or rebase to avoid the back merge? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-537678162
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
I'm fine with merging. Just solved the merge conflict. If nobody beats me by time or with feedback, I'll merge this on Sunday. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-537650233
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 1 commit. 1a558f5cc9e362586da864f1cf7ee80fa1951a32 Merge branch 'master' into save_config_on_doclist_change -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/41227a4084879de554cda8e982229ed788847353..1a558f5cc9e362586da864f1cf7ee80fa1951a32
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Unfortunately this wasn't merged, even with two approvals and the target milestone 1.36. I'd say merge this quickly so we don't forget it again ;-) @eht16 ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-536592907
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
codebrainz approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#pullrequestreview-271015791
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@b4n any other feedback? Would you be fine with merging this PR and let @codebrainz or whoever do the rest later? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-518405135
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Ahhh, good catch! Thanks, see 41227a4. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-518402783
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 1 commit. 41227a4084879de554cda8e982229ed788847353 Document new emission of "project-save" signal -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/f7d298846d4f18d358217725ddc4407952e1..41227a4084879de554cda8e982229ed788847353
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Documentation I meant. The description of `plugin_save` on [this](https://www.geany.org/manual/reference/pluginsignals_8c.html#ac093c4c2a6ef1ed7c35e5736a27d7f27) page that lists when it happens. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-518039911
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@elextr what do you mean exactly? Should we add a listener on `project-save` or should we extend the documentation of `project-save`? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-518010080
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 I just noticed that the plugin API description of the `project_save` signal lists when it happens, I presume this needs to be added to that list. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-517912985
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@kugel- I was waiting for feedback after I answered the review feedback of @ntrel, @elextr and @b4n. I just squashed the commits after I removed the API version bump. Only @b4n's voice missing. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513543834
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
eht16 commented on this pull request. > @@ -59,7 +59,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 239 +#define GEANY_API_VERSION 240 Hmm, ok. Removed. I still consider it that bad as it is only a bump indicating there are changes (and in this case only additions). It's not like as we would change the ABI. Anyway, I kept the API version as is. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#discussion_r305611953
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
ntrel commented on this pull request. > @@ -59,7 +59,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 239 +#define GEANY_API_VERSION 240 AFAIK, anything without a doc-comment shouldn't be considered part of the API. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#discussion_r305360704
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Oh, ok, I read "I'm too tired of losing my open files" as you hadn't saved them. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513154073
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
The point of this is to save the session list more often, and that's what I'm looking for. I save often enough to not lose contents. Anyway, I am testing this at the moment, so far it works as advertised. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513153255
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@kugel- "save more often" :) but the point is, if you think this is gonna help you and you want it, why not test it. Although it doesn't actually save your files, at least you would have the list of the ones you lost. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513152139
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
What do you refer to with `s` ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513151205
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
kugel- approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#pullrequestreview-264088790
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@kugel- s? On maybe test this PR and say its working for you. But in case its not clear I'm happy with this as is, but with the caveat that I havn't tested it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513149884
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 what's missing on this? I'm too tired of losing my open files because my laptop doesn't resume from suspend properly after 2 weeks of uptime. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-513146973
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> As said above, if you or anyone else want, let's improve the feature > afterwards... :+1:, I might even work on this (again), either like #1257 or something a little less invasive. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-507071495
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
I agree with @elextr: better saving the config too often than too few. The primary intention is to save data loss on crashes (of Geany or the host). Users concerning their disk IO can disable the feature. As said above, if you or anyone else want, let's improve the feature afterwards. The current implementation will probably be already sufficient and help a lot most of the users. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-507030004
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
elextr commented on this pull request.
> @@ -1313,11 +1316,47 @@ void configuration_apply_settings(void)
}
+static gboolean save_configuration_cb(gpointer data)
+{
+ configuration_save();
> "not always"
On further consideration since any project action closes one set of files and
opens another (even if they are the same files) there will be file open/close
signals in all cases except the corner case where no files are open before and
after the project open/close, so don't worry about it I think.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r298830558
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
eht16 commented on this pull request.
> void configuration_init(void)
{
keyfile_groups = g_ptr_array_new();
pref_groups = g_ptr_array_new();
init_pref_groups();
+
+ g_signal_connect(geany_object, "document-new",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-open",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
> It would be nice to avoid saving the conf when saving a file in the common
> case, e.g. when just updating the file's content. AFAIK, we don't affect the
> session then, do we? My point is that I'm
Hmm, I'm not sure if checking `doc->real_path` for `NULL` is sufficient, there
might be more cases when we want to trigger the config save. Also, the
`doc->real_path` check would have to differentiate cases like renaming and
closing documents.
The config save process is pretty lightweight and, as said initially, very fast.
Users trying to reduce disk IO still can disable this feature.
Can we agree to implement more sophisticated checks independent of this PR
afterwards if someone wants to?
> Why is document-new connected to? If there's no file yet it won't affect the
> session.
Removed in
https://github.com/geany/geany/pull/2114/commits/d6ebff6c4f6c04786e5448735ac59462e2f7619a.
Probably not necessary, I just added it for completeness.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r298830316
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 1 commit. d6ebff6c4f6c04786e5448735ac59462e2f7619a Remove probably unnecessary "document-new" signal handling -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/96bbc8357836b5fe7979f7323eea76abcbb9bd2c..d6ebff6c4f6c04786e5448735ac59462e2f7619a
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
eht16 commented on this pull request.
> @@ -1313,11 +1316,47 @@ void configuration_apply_settings(void)
}
+static gboolean save_configuration_cb(gpointer data)
+{
+ configuration_save();
> Just continuing @b4n's theme, do we need to save both config and project? The
> active session is in the project, not the config.
Maybe we do not need to save the config if a project is open regarding the
session files but for all other maybe changed settings and I think it won't
hurt(except disk IO) to leave a consistent state on disk.
> And on that note, shouldn't the config be saved on project open/close as
> well, although it will often happen anyway because files will be
> opened/closed when a project opens, but not always.
In my tests, the `save_configuration_cb()` was always called on project related
actions. What exact cases do you refer to by "not always"?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r298829919
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
eht16 commented on this pull request. > @@ -59,7 +59,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 239 +#define GEANY_API_VERSION 240 Because of the new member in `GeanyFilePrefs` which is part of the API. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#discussion_r298829481
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
I don't think its neccessary to worry about disk IO unless its having an effect on performance, but since save only occurs when config data changes and that is when there is a major screen update (new document, close document etc) its not likely to be noticed. If it does cause performance issues, eg because the config file is on some slow remote filesystem then there is now an option to turn this saving off. As for saving disk life, on my systems something in the OS writes to disk every few seconds (with no user apps open), so I doubt the Geany config saving will have a material effect on lifetime. Only aggregating and only saving on focus loss is ok for user logout since it will be hard to invoke logout or shutdown without de-focussing geany, but it will not handle a crash though. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-499041729
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
It would be good to reduce file writes e.g. to prolong hard disk life. What if we track when session data changes but only write the keyfile when the main window loses focus? That would seem to solve the user clicking logout problem and drastically reduce disk/network writes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-499033828
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
ntrel commented on this pull request.
> void configuration_init(void)
{
keyfile_groups = g_ptr_array_new();
pref_groups = g_ptr_array_new();
init_pref_groups();
+
+ g_signal_connect(geany_object, "document-new",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-open",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
Why is document-new connected to? If there's no file yet it won't affect the
session.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r290675990
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
I want to work on the open feedback to get this finished. Last weeks I was very limited on free time for this because of other priorities and few spare time. But this will change soon, so I'm confident we will get this ready to merge before 1.36. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-493257509
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
What's missing on this? Want this desperately. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-493237583
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
elextr commented on this pull request.
> @@ -1313,11 +1316,47 @@ void configuration_apply_settings(void)
}
+static gboolean save_configuration_cb(gpointer data)
+{
+ configuration_save();
And on that note, shouldn't the config be saved on project open/close as well,
although it will often happen anyway because files will be opened/closed when a
project opens, but not always.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r275567860
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
elextr commented on this pull request.
> @@ -1313,11 +1316,47 @@ void configuration_apply_settings(void)
}
+static gboolean save_configuration_cb(gpointer data)
+{
+ configuration_save();
Just continuing @b4n's theme, do we need to save both config and project? The
active session is in the project, not the config.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#pullrequestreview-226884909
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
elextr commented on this pull request.
> void configuration_init(void)
{
keyfile_groups = g_ptr_array_new();
pref_groups = g_ptr_array_new();
init_pref_groups();
+
+ g_signal_connect(geany_object, "document-new",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-open",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
Well, the session includes cursor position so its probably worth saving.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#discussion_r275544565
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
b4n commented on this pull request.
Looks mostly good, see comments.
> @@ -59,7 +59,7 @@ G_BEGIN_DECLS
* @warning You should not test for values below 200 as previously
* @c GEANY_API_VERSION was defined as an enum value, not a macro.
*/
-#define GEANY_API_VERSION 239
+#define GEANY_API_VERSION 240
Why bumping API?
> void configuration_init(void)
{
keyfile_groups = g_ptr_array_new();
pref_groups = g_ptr_array_new();
init_pref_groups();
+
+ g_signal_connect(geany_object, "document-new",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-open",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
It would be nice to avoid saving the conf when saving a file in the common
case, e.g. when just updating the file's content. AFAIK, we don't affect the
session then, do we? My point is that I'm usually saving my files very often,
and there's no use of saving the config then, only if I saved as another name
or such, is there?
I agree it's probably cheap to save it anyway (although it might matter a
little on SSDs?), but if it's of no use at all I'd rather see it avoided.
Not sure how to implement that though… the only thing I can think of is to
check in `:document-before-save` that `doc->real_path == NULL` and then set a
flag to handle that one in `:document-save`, for when saving is successful? Or
just don't bother to check if save worked, and just do it in
`:document-before-save` with the check, as the likely case is a successful save
anyway.
---
Untested possible implementations:
The simple solution:
```diff
+static void document_before_save_cb(GObject *obj, GeanyDocument *doc, gpointer
data)
+{
+ if (! doc->real_path)
+ document_list_changed_cb(obj, doc, data);
+}
+
/* … */
- g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-before-save",
G_CALLBACK(document_before_save_cb), NULL);
/* … */
```
The convoluted solution:
```diff
+static void document_before_save_cb(GObject *obj, GeanyDocument *doc, gpointer
data)
+{
+ gboolean *saved_as = data;
+ *saved_as = doc->real_path == NULL;
+}
+
+static void document_save_cb(GObject *obj, GeanyDocument *doc, gpointer data)
+{
+ gboolean *saved_as = data;
+ if (*saved_as)
+ document_list_changed_cb(obj, doc, NULL);
+}
+
/* … */
+ gboolean *saved_as_p = g_malloc(sizeof *saved_as_p);
+ g_signal_connect_data(geany_object, "document-before-save",
G_CALLBACK(document_before_save_cb), saved_as_p, (GClosureNotify) g_free, 0);
- g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), NULL);
+ g_signal_connect(geany_object, "document-save",
G_CALLBACK(document_list_changed_cb), saved_as_p);
/* … */
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2114#pullrequestreview-226782405
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Fixed by restarting the build and now it's green again :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-482938784
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Hmm, travis seems to have a problem with that unrelaible download.geany.org :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-482937836
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Seems to work fine, used it in the last weeks and noticed no problems or side effects so far. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-482937345
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
eht16 commented on this pull request. > @@ -2636,6 +2636,16 @@ reload_clean_doc_on_file_change Whether to > automatically reload documentsf on disk. If unsaved changes exist then the user is prompted to reload manually. +save_config_on_file_changeAutomatically save Geany's configuration trueimmediately + to disk once the document list changes + (i.e. new documents are opened, saved or + closed). This helps to accidentally lose Ouch. Thanks for spotting this, I always appreciate your editing pencil! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#discussion_r270661384
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 1 commit. 59317693e36680962fc5bb0956a031e4aa9f213f Reword the doc text to not tell what we want to prevent -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/b07c090b881b60d14c5d7ce514386e5bae4796b9..59317693e36680962fc5bb0956a031e4aa9f213f
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> you probably want to check the documentation change and tell me if it can be > phrased better :). elextr licks lips, extracts purple editing pencil from behind ear ... two comments. also LGBI -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-478191606
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
elextr commented on this pull request. > @@ -2636,6 +2636,16 @@ reload_clean_doc_on_file_change Whether to > automatically reload documentsf on disk. If unsaved changes exist then the user is prompted to reload manually. +save_config_on_file_changeAutomatically save Geany's configuration trueimmediately + to disk once the document list changes + (i.e. new documents are opened, saved or + closed). This helps to accidentally lose I hope it doesn't help to lose stuff, maybe ... to prevent accidentally losing ... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#pullrequestreview-220800204
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Ok, https://github.com/geany/geany/pull/2114/commits/b07c090b881b60d14c5d7ce514386e5bae4796b9 brings you a nice new setting. As you are a grateful guy, you probably want to check the documentation change and tell me if it can be phrased better :). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477988789
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 pushed 1 commit. b07c090b881b60d14c5d7ce514386e5bae4796b9 Add setting, defaulting to TRUE, to disable automatic configuration save -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2114/files/da1b72dc9c2d9649d8d100ee5d80347e2c1ac192..b07c090b881b60d14c5d7ce514386e5bae4796b9
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Agree that multiple instances sharing the same config or project is a separate problem. I always try to discourage people running more than one geany using the same config or project, thats why I had the evil grin icon on the comment :grin: > I have no clue if there are still any users out there who actually store > configs on network shares or the like. Given that NAS appliances are now only a few hundred dollars and as the US _consumer_ market was [half a billion dollars in 2017 and growing](https://www.statista.com/statistics/750899/united-states-consumer-network-attached-storage-market-size/) I suspect that the numbers of user configs and project files on network devices may be increasing, not reducing. > And actually, the code of this PR is triggered by the SaveActions plugin > already via the "document-save" signal. So what you request is already done > :). I do hope your aggregation algorithm works. :grinning: To be clear, In general this change is a good thing to do, I'm just trying to anticipate any issues, and would like having the "off" option hidden in various as a workaround for the ones we didn't think of. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477971575
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@codebrainz I agree that the current implementation is quite arbitrary about when to save. Though, in daily work, I guess this fits for most users. And yes, a more general config system (as you mentioned) with notices and a queue would be awesome but is a whole another story. @elextr this PR doesn't address the issue with multiple instances which overwrite the config mutually. I think we should stop writing the config at all in new instances but again, another story. @elextr I'm not sure how useful a setting for this feature would be. the config we write is only a few kilobytes and I have no clue if there are still any users out there who actually store configs on network shares or the like. Oh, and I think this should *not* go into the SaveActions plugin as the plugin is for (auto) saving documents not Geany's settings. And actually, the code of this PR is triggered by the SaveActions plugin already via the "document-save" signal. So what you request is already done :). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477957247
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
@eht16 maybe this should be in the autosave plugin since it will be most useful when the files that its saving in the session are themselves being saved. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477839927
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> if you used View->Show Indentation Guides, that setting still wouldn't be > saved until one of the places which saves the configuration is hit. Sure, its not comprehensive yet, pull requests are welcome. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477838423
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> If by "where" you mean "when" then I thought settings are now pretty much > always saved when they are changed I mean this PR only adds more places where configuration is saved (compared to only on prefs dialog applied, plugin manager closed, app closed, etc.) but it's still not comprehensive. In a perfect world there would be an interface/abstraction for the config system that whenever any setting is changed, a save is automatically queued, similar to something like Xfconf or GSettings or such. To give an example, even after this PR, if you used `View->Show Indentation Guides`, that setting still wouldn't be saved until one of the arbitrary places which saves the configuration is hit. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477829819
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Oh my, all those people who have two Geanys using the same config are gonna have many more races :grin: And an occasional failure when both try to write at the same time (on those file systems where such access is exclusive). > The only thing I don't like, which could be improved later, is that it's > still pretty arbitrary where the settings are being saved. If by "where" you mean "when" then I thought settings are now pretty much always saved when they are changed, its only the session that saved at the end, and thats what this addresses. Of course because of the way g_key_files work the whole lot has to be saved. But as @codebrainz points out there are potential problems with slow or flakey drives maybe there needs to be a setting to disable it instead of forcing it on everyone. Of course since it only writes to `geany_debug()` on error nobody will notice its failing I guess. Agree best to commit immediately after next release. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477818868
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> At least performance doesn't seem to be a problem: configuration_save takes > about 1-2 milliseconds on my system and IO access happens in the > document-related actions in any way. I guess the only issue is if the config directory is on a slow drive like a network share or something. We could try it and if it causes people problems in reality, maybe look at adding a preference to disable it. I can't see it being much of an issue though as most programs do similar things. The only thing I don't like, which could be improved later, is that it's still pretty arbitrary where the settings are being saved. In a perfect world, it would queue a save whenever the contents of the config file need to be saved. 👍 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477809578
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Also related are #51 and #52. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114#issuecomment-477806783
[Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
The main idea is to save the session file list more often to prevent accidental lost but saving the rest of the configuration might help as well. To prevent too many save attempts, an idle function is used and it's only added once until it was executed. This might help #1826, #1426 and replaces #1860. IIRC at the very beginning I was a bit concerned about IO access and performance when writing the settings too often. At least performance doesn't seem to be a problem: `configuration_save` takes about 1-2 milliseconds on my system and IO access happens in the document-related actions in any way. Even though I tested the code, I would like to use it "in production" for some time to get sure there are no unseen side effects or similar. You can view, comment on, or merge this pull request online at: https://github.com/geany/geany/pull/2114 -- Commit Summary -- * Save main and project configuration whenever documents are opened/closed -- File Changes -- M src/keyfile.c (38) -- Patch Links -- https://github.com/geany/geany/pull/2114.patch https://github.com/geany/geany/pull/2114.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2114
