GNOME Bugzilla – Bug 659437
External tools "save all docs" function should save only documents with modifications
Last modified: 2019-03-23 20:49:03 UTC
When selecting to save "All documents" for a command, all documents are indeed saved, even those which didn't have any modifications (i.e. not showing a little star near the name of the file). This behavior makes external tools quite a pain to use with the make command : since all files were saved, make sees them as new and rebuilds everything each time. Steps to Reproduce: Write a "make" tool that saves all docs and calls the make command, execute it twice. Actual Results: Everything is compiled again the second time you call make. Expected Results: "make: `EXE' is up to date."
I think the solution is to add another option to save modified documents (i.e. still having the possibility to save all documents).
Another consequence of this bug: if you have tabs showing read-only files (e.g. header files in system include directories) then gedit will ask where to save those files when running an external tool which saves all documents. That's an annoyance. Jesse: why even have the possiblity to save unmodified documents? If 'save all documents' only saved modified documents I think that would be just fine.
(In reply to comment #2) > Another consequence of this bug: if you have tabs showing read-only files (e.g. > header files in system include directories) then gedit will ask where to save > those files when running an external tool which saves all documents. That's an > annoyance. > Mmm, when looking at the code this should not happen, if a readonly file is not locally modified it is not added to the list of files to save: static gboolean document_needs_saving (GeditDocument *doc) { if (gtk_text_buffer_get_modified (GTK_TEXT_BUFFER (doc))) return TRUE; ... } ... if (gedit_document_is_untitled (doc) || gedit_document_get_readonly (doc)) { if (document_needs_saving (doc)) { tabs_to_save_as = g_slist_prepend (tabs_to_save_as, t); } }
(In reply to comment #3) > > Mmm, when looking at the code this should not happen, if a readonly file is not > locally modified it is not added to the list of files to save: That may be, and yet I can still reproduce this behavior in git master, so something else must be going on.
Created attachment 247125 [details] [review] Use the correct saving function in external tools Otherwise we bypass gedit's special handling for saving all docs.
Created attachment 247126 [details] [review] Bug 659437 - Only save modified documents in external tools
Created attachment 247129 [details] [review] Bug 659437 - Only save modified documents in external tools v2
Go ahead.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
This code is working in the opposite way it is intended to. I.e. only the unchanged documents are being saved, and the changed documents are left unsaved. On inspecting the code, it is obvious why: if doc.get_modified(): all_docs = False docs.remove(doc) Modified documents are being removed from the list to be saved. Instead it should be: if not doc.get_modified(): It is possible to re-open this bug or shall I open a new bug?
Created attachment 268348 [details] [review] bug 659437 invert test for modified documents
Going through the code I also noticed another bug that is causing save to sometimes fail--a classic Python bug. If an item is deleted from a list while that list is being looped through, then it's like sawing off the branch of a tree you are sitting on. Here is the code: for i in range(len(docs)): doc = docs[i] ... docs.remove(doc) The code will break with 'list index out of range' if a list member is deleted. There is a simple fix--just reverse the direction of the loop: for i in reversed(range(len(docs))): I have submitted a patch that fixes both errors.
Created attachment 268368 [details] [review] bug 659437 invert test for modified documents and fix loop error
I was affected by this bug as well and I confirm that the changes proposed above fix the problem.
Bug confirmed
Thanks Andrew for the analysis. I fixed it with a slightly different patch: https://git.gnome.org/browse/gedit/commit/?id=6c99bb727a8697e37cd55e0ccf40e4069e65f1ce
Thanks Paolo. I tested it and it seems to work ok.
This problem keeps coming up and I realize that it was not actually fixed. There is a single line that contains the bug: if len(docs_to_save) == len(docs) and len(docs) != 0: Problem: -When all_docs is false (i.e. not all docs are to be saved) then docs = active document len(docs) = 1 -So the statement reads: if number of documents to save == 1 and 1 != 0: save all documents The simple fix is this: if len(docs_to_save) == len(window.get_documents()) and len(docs_to_save) != 0: -So the statement now reads: if number of documents to save == all documents, (and not 0): save all documents.
It's fixed in bug #739686, so I close this one.