[Github-comments] [geany/geany] Fix 'changed' flag being altered when all documents are closing. (#1857)

[Colomban Wendling]

b4n requested changes on this pull request.

Great work investigating and fixing this!

I came to the same conclusion as you did trying to see how I'd fix that.
Logic looks good to me, and I couldn't find a case of bad behavior when actually trying it :+1: 

I'd just like a few small fixups and we should be good to go.

> @@ -704,7 +704,7 @@ static gboolean remove_page(guint page_num)
 
 	g_return_val_if_fail(doc != NULL, FALSE);
 
-	if (doc->changed && ! dialogs_show_unsaved_file(doc))
+	if (! main_status.closing_all && doc->changed && ! dialogs_show_unsaved_file(doc))

Please add a comment above, like `/* if we're closing all, document_account_for_unsaved() has been called already, no need to ask again. */`

> @@ -3387,11 +3387,7 @@ gboolean document_account_for_unsaved(void)
 				return FALSE;
 		}
 	}
-	/* all documents should now be accounted for, so ignore any changes */
-	foreach_document (i)
-	{
-		documents[i]->changed = FALSE;
-	}
+

Now you removed this, maybe remove the comment stating that *If successful, this should always be followed up with a call to `document_close_all()`* right above the function.

> @@ -3387,11 +3387,7 @@ gboolean document_account_for_unsaved(void)
 				return FALSE;
 		}
 	}
-	/* all documents should now be accounted for, so ignore any changes */
-	foreach_document (i)
-	{
-		documents[i]->changed = FALSE;
-	}
+

Variable `i` is now unused and should be removed.

> @@ -3400,14 +3396,6 @@ static void force_close_all(void)
 {
 	guint i, len = documents_array->len;

`len` is now unused and should be removed.

> @@ -1262,16 +1262,20 @@ static void queue_free(GQueue *queue)
 }
 
 
-static void do_main_quit(void)
+static gboolean do_main_quit(void)
 {
 	geany_debug("Quitting...");

Could you move the debug info after `docuent_close_all()`?  now it'd be kind of confusing if the user aborted the quitting process.

-- 
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/1857#pullrequestreview-180558213
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20181202/1ce3ff38/attachment.html>