>Things in Geany and plugins may depend on the "unchanged" status to prevent
>them doing stuff when the document is closed at shutdown.
I greped out the contents of the current plugins with this command at the root
of the geany-plugins directory:
```
grep -Rni '\->changed' *
```
and I get these results (you can obviously ignore git-changebar since
changed_color_button is caught but unrelated):
```
geanylua/glspi_doc.c:331: SetTableBool("changed",doc->changed);
geanypy/src/geanypy-document.c:120: if (self->doc->changed)
geanyvc/src/geanyvc.c:595: if (doc->changed)
geanyvc/src/geanyvc.c:683: if (doc->changed)
geanyvc/src/geanyvc.c:831: if (doc->changed)
geanyvc/src/geanyvc.c:896: if (doc->changed)
geanyvc/src/geanyvc.c:974: if (doc->changed)
git-changebar/src/gcb-plugin.c:1605: gtk_color_button_get_color
(GTK_COLOR_BUTTON (cw->changed_color_button),
git-changebar/src/gcb-plugin.c:1664: { "changed-color-button",
&cw->changed_color_button },
git-changebar/src/gcb-plugin.c:1679: gtk_color_button_set_color
(GTK_COLOR_BUTTON (cw->changed_color_button),
scope/src/utils.c:365: document_set_text_changed(doc, doc->changed);
/* to redraw tab & update sidebar */
```
It looks like geanylua, geanypy, geanyvc, and scope are the only plugins that
look at the 'changed' flag.
geanlua uses this flag to query information about documents already open, but
doesn't do anything with document close that I can see.
geanypy uses this flag to provide functions to query the state of the document,
but doesn't do anything with it during document close that I can see.
geanyvc uses this flag to see if it needs to auto save files, which in this
case means the current behavior may cause a bug in this plugin, but I don't see
anything with document-close so probably not. Maybe that plugin author can
chime in? I don't know what the best way is to contact them so they can see
what's going on in this thread.
scope uses this flag when re-painting a document, but never during document
close that I can see.
AFAICT, the only reason Geany does this is because the
'document_account_for_unsaved' function pops up a dialog for all changed
documents before closing *any* documents. If the 'changed' flag isn't set to
FALSE, then the 'remove_page' function that will get called later would cause a
second dialog to pop up for changed documents when closing. When playing around
with this I actually observed this behavior. Setting changed to FALSE fixes
that, and since Geany doesn't save any information WRT the changed flag - just
a list of currently opened files AFAIK - it shouldn't cause any larger effects
within Geany. I suspect this is also why the 'document_account_for_unsaved'
function itself has a note that
indicates a TRUE return from this function should *always* be followed by
'document_close_all', because otherwise opened documents with changes could get
left in a weird invalid state.
I address this double pop up issue instead by inspecting the
main_status.closing_all flag when 'remove_page' is called. This flag gets set
whenever 'document_close_all' is being invoked, and since 'document_close_all'
handles the dialogs for unchanged files with a call to
'document_account_for_unsaved', it makes sense that the 'remove_page' function
should delegate this behavior further upstream in this case.
The only issue I can see with this approach is that
'document_account_for_unsaved' would now get called twice during a call to the
'main_quit' function triggering a double pop up again, so I removed the
'document_account_for_unsaved' function call there, and let the
'document_close_all' function that gets called in 'do_main_quit' call it
instead.
Unfortunately the 'do_main_quit' function never inspects the return value of
'document_close_all' (since changes upstream woulds always return TRUE before),
so I had to change the 'do_main_quit' function to return a FALSE value if the
'project_close' or 'document_close_all' returns FALSE, and TRUE otherwise so
that the 'main_quit' function can still be aborted by the user, just how Geany
works currently. AFAICT, 'document_close_all' and 'main_quit' are the only two
places where the 'document_account_for_unsaved' function is called directly.
>If it was added pre-git any discussions may not have been transferred from
>SVN, but they may also have happened on the dev ML, so the archive would need
>to be checked around the date the function was added.
I tried to track down when 'document_account_for_unsaved' was first added and
the first commit I see is 2008 April 24 with the git SHA of
3afebc701c47f4de34c0bb9b52fadaa2f5adb781. This predates what I can see in the
mailing list archives. Also, it looks like this commit was really just a
refactoring of function calls in 'callbacks.c', which moved the
'account_for_unsaved' function in 'callbacks.c' to 'documents.c' and renamed it.
Prior to this the 'account_for_unsaved' function didn't alter the 'changed'
flag, but instead this behavior was in the 'force_close_all' function in
'callbacks.c' which was invoked during Geany quit. The 'force_close_all'
function first pops up ~3 weeks prior in 2008 April 4 with the git SHA
9a67c39a70f7a13b1c6c56a86a43688b80a2da6c. This commit also looks like it was
updating the close all function to prompt dialogs for unsaved changes as well
as refactoring code using the 'force_close_all' function to simplify the
'quit_app' function which is where this 'changed' flag alteration was occurring
previously.
The first commit that I can see that introduces the 'quit_app' function in
Geany goes back to a commit in 2006 November 7 with the git SHA
20637b939b916f8082a5ce3a5a957c316efc93dd. This commit looks like it is
refactoring the 'on_exit_clicked' function and altering how dialog pop ups are
used when closing a document with unsaved changes.
Prior to that, Geany never touched the 'changed' flag when closing all
documents, and instead let the 'document_remove' function handle the dialog pop
up when closing a document with unsaved changes. I believe this is why the
commit was added in the first place. Prior to the Nov. 7 commit, Geany would
start closing documents when exiting, and if the user canceled in the middle of
it, that would leave everything open except for the documents that had been
closed. That would be really annoying. The message that goes along with this
commit appears to support this:
```
Author: Nick Treleaven <[email protected]>
Date: Tue Nov 7 11:24:22 2006 +0000
Don't close any tabs when quitting until all unsaved changes have
been accounted for; switch to each unsaved file before showing the
unsaved dialog.
Remove limit of ~256 chars for session filenames.
Make dialogs_show_unsaved_file() fail if the Save As dialog was
cancelled.
git-svn-id: https://geany.svn.sourceforge.net/svnroot/geany/trunk@972
ea778897-0a13-0410-b9d1-a72fbfd435f5
```
At the time, if the 'changed' flag was left unaltered the same double pop up
would have happened that I mentioned earlier. So that would explain needing to
set it to FALSE in the first place. Inspecting the main_status.closing_all
flag instead makes this a non-issue however. I couldn't find anything in the
SVN project page in source forge from around that time that referenced this
commit or the behavior it would have addressed, so I don't know what other
discussion would have occurred to talk about these changes.
>From looking at the commits, it looks like Nick was maintaining this code,
>with the 2008 April 24 commit being the last refactoring of this function. The
>function has remained virtually unchanged since and has only been updated as
>variable names referenced were changed or new macros were added. The last
>commit I see from Nick Treleaven (@ntrel) is over a year ago, so I don't know
>how closely he is watching this, but maybe he can chime in if at all possible
>and offer some context.
Are there others who have worked on / have knowledge of these functions that
could comment on whether this looks wrong or not?
--
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#issuecomment-390382781
