[Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)

[Colomban Wendling]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20190415/bc0836d7/attachment-0001.html>