From 8a860888f0cec527e74aa0fda4e643d97838c070 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Wed, 25 Jan 2023 18:33:19 +0000 Subject: [PATCH 01/39] Partial progress --- src/Services/Document.vala | 312 +++++++++++++++--------------- src/Services/DocumentManager.vala | 144 ++++++++++++++ src/Widgets/DocumentView.vala | 5 +- src/Widgets/SearchBar.vala | 6 +- 4 files changed, 307 insertions(+), 160 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1db488abf1..1d47a15aff 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -88,11 +88,28 @@ namespace Scratch.Services { public Gtk.Stack main_stack; public Scratch.Widgets.SourceView source_view; + + + public bool saved = true; + public bool delay_saving { + get { + return completion_shown || is_saving; + } + } + + public bool inhibit_saving { + get { + return !can_write () || !loaded; + } + } + + public bool is_saving { get; private set; } + private Scratch.Services.SymbolOutline? outline = null; - public string original_content; + private string original_content; // For restoring to original private string last_save_content; - public bool saved = true; private bool completion_shown = false; + private bool loaded = false; private Gtk.ScrolledWindow scroll; private Gtk.InfoBar info_bar; @@ -101,8 +118,8 @@ namespace Scratch.Services { private GLib.Cancellable save_cancellable; private GLib.Cancellable load_cancellable; - private ulong onchange_handler_id = 0; // It is used to not mark files as changed on load - private bool loaded = false; + // private ulong onchange_handler_id = 0; // It is used to not mark files as changed on load + private bool mounted = true; // Mount state of the file private Mount mount; @@ -179,7 +196,7 @@ namespace Scratch.Services { this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); - toggle_changed_handlers (true); + // toggle_changed_handlers (true); // Focus out event for SourceView this.source_view.focus_out_event.connect (() => { @@ -214,37 +231,37 @@ namespace Scratch.Services { ellipsize_mode = Pango.EllipsizeMode.MIDDLE; } - public void toggle_changed_handlers (bool enabled) { - if (enabled && onchange_handler_id == 0) { - onchange_handler_id = this.source_view.buffer.changed.connect (() => { - if (onchange_handler_id != 0) { - this.source_view.buffer.disconnect (onchange_handler_id); - } - - // Signals for SourceView - uint timeout_saving = 0; - check_undoable_actions (); - onchange_handler_id = source_view.buffer.changed.connect (() => { - check_undoable_actions (); - // Save if autosave is ON - if (Scratch.settings.get_boolean ("autosave")) { - if (timeout_saving > 0) { - Source.remove (timeout_saving); - timeout_saving = 0; - } - timeout_saving = Timeout.add (1000, () => { - save.begin (); - timeout_saving = 0; - return false; - }); - } - }); - }); - } else if (!enabled && onchange_handler_id != 0) { - this.source_view.buffer.disconnect (onchange_handler_id); - onchange_handler_id = 0; - } - } + // public void toggle_changed_handlers (bool enabled) { + // if (enabled && onchange_handler_id == 0) { + // onchange_handler_id = this.source_view.buffer.changed.connect (() => { + // if (onchange_handler_id != 0) { + // this.source_view.buffer.disconnect (onchange_handler_id); + // } + + // // Signals for SourceView + // uint timeout_saving = 0; + // check_undoable_actions (); + // onchange_handler_id = source_view.buffer.changed.connect (() => { + // check_undoable_actions (); + // // Save if autosave is ON + // if (Scratch.settings.get_boolean ("autosave")) { + // if (timeout_saving > 0) { + // Source.remove (timeout_saving); + // timeout_saving = 0; + // } + // timeout_saving = Timeout.add (1000, () => { + // save.begin (); + // timeout_saving = 0; + // return false; + // }); + // } + // }); + // }); + // } else if (!enabled && onchange_handler_id != 0) { + // this.source_view.buffer.disconnect (onchange_handler_id); + // onchange_handler_id = 0; + // } + // } private uint load_timout_id = 0; public async void open (bool force = false) { @@ -458,46 +475,7 @@ namespace Scratch.Services { return result; } - public async bool save (bool force = false) { - if (completion_shown || - !force && (source_view.buffer.get_modified () == false || - !loaded)) { - return false; - } - - this.create_backup (); - - if (Scratch.settings.get_boolean ("strip-trailing-on-save") && force) { - strip_trailing_spaces (); - } - - // Replace old content with the new one - save_cancellable.cancel (); - save_cancellable = new GLib.Cancellable (); - var source_file_saver = new Gtk.SourceFileSaver ((Gtk.SourceBuffer) source_view.buffer, source_file); - try { - yield source_file_saver.save_async (GLib.Priority.DEFAULT, save_cancellable, null); - } catch (Error e) { - // We don't need to send an error message at cancellation (corresponding to error code 19) - if (e.code != 19) - warning ("Cannot save \"%s\": %s", get_basename (), e.message); - return false; - } - - source_view.buffer.set_modified (false); - - if (outline != null) { - outline.parse_symbols (); - } - - this.set_saved_status (true); - last_save_content = source_view.buffer.text; - - debug ("File \"%s\" saved successfully", get_basename ()); - - return true; - } public async bool save_as () { // New file @@ -829,6 +807,20 @@ namespace Scratch.Services { ); } + public void before_undoable_change () { + source_buffer.set_editable (false); + } + + public void after_undoable_change () { + source_view.buffer.set_modified (false); + if (outline != null) { + outline.parse_symbols (); + } + } + + + + // Set saved status public void set_saved_status (bool val) { this.saved = val; @@ -844,22 +836,22 @@ namespace Scratch.Services { } } - // Backup functions - private void create_backup () { - if (!can_write ()) { - return; - } + // // Backup functions + // private void create_backup () { + // if (!can_write ()) { + // return; + // } - var backup = File.new_for_path (this.file.get_path () + "~"); + // var backup = File.new_for_path (this.file.get_path () + "~"); - if (!backup.query_exists ()) { - try { - file.copy (backup, FileCopyFlags.NONE); - } catch (Error e) { - warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); - } - } - } + // if (!backup.query_exists ()) { + // try { + // file.copy (backup, FileCopyFlags.NONE); + // } catch (Error e) { + // warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); + // } + // } + // } private void delete_backup (string? backup_path = null) { string backup_file; @@ -900,21 +892,28 @@ namespace Scratch.Services { return false; } - // Return true if the file is writable - public bool can_write () { + // Return true if the file is writable. Keep testing as may change + private bool can_write () { FileInfo info; - bool writable = false; - try { - info = this.file.query_info (FileAttribute.ACCESS_CAN_WRITE, FileQueryInfoFlags.NONE, null); - writable = info.get_attribute_boolean (FileAttribute.ACCESS_CAN_WRITE); - return writable; + info = this.file.query_info ( + FileAttribute.ACCESS_CAN_WRITE, + FileQueryInfoFlags.NONE, + null + ); + writable = info.get_attribute_boolean ( + FileAttribute.ACCESS_CAN_WRITE + ); } catch (Error e) { - warning ("query_info failed, but filename appears to be correct, allowing as new file"); + debug ( + "Error determining write access: %s. Allowing write", + e.message + ); writable = true; - return writable; } + + return writable; } // Return true if the file exists @@ -977,61 +976,64 @@ namespace Scratch.Services { text.scroll_to_iter (iter, 0.0, true, 0.5, 0.5); } - /* Pull the buffer into an array and then work out which parts are to be deleted. - * Do not strip line currently being edited unless forced */ - private void strip_trailing_spaces () { - if (!loaded || source_view.language == null) { - return; - } - - var source_buffer = (Gtk.SourceBuffer)source_view.buffer; - Gtk.TextIter iter; - - var cursor_pos = source_buffer.cursor_position; - source_buffer.get_iter_at_offset (out iter, cursor_pos); - var orig_line = iter.get_line (); - var orig_offset = iter.get_line_offset (); - - var text = source_buffer.text; - - string[] lines = Regex.split_simple ("""[\r\n]""", text); - if (lines.length == 0) { // Can legitimately happen at startup or new document - return; - } - - if (lines.length != source_buffer.get_line_count ()) { - critical ("Mismatch between line counts when stripping trailing spaces, not continuing"); - debug ("lines.length %u, buffer lines %u \n %s", lines.length, source_buffer.get_line_count (), text); - return; - } - - MatchInfo info; - Gtk.TextIter start_delete, end_delete; - Regex whitespace; - - try { - whitespace = new Regex ("[ \t]+$", 0); - } catch (RegexError e) { - critical ("Error while building regex to replace trailing whitespace: %s", e.message); - return; - } - - for (int line_no = 0; line_no < lines.length; line_no++) { - if (whitespace.match (lines[line_no], 0, out info)) { - - source_buffer.get_iter_at_line (out start_delete, line_no); - start_delete.forward_to_line_end (); - end_delete = start_delete; - end_delete.backward_chars (info.fetch (0).length); - - source_buffer.begin_not_undoable_action (); - source_buffer.@delete (ref start_delete, ref end_delete); - source_buffer.end_not_undoable_action (); - } - } - - source_buffer.get_iter_at_line_offset (out iter, orig_line, orig_offset); - source_buffer.place_cursor (iter); - } - } + // /* Pull the buffer into an array and then work out which parts are to be deleted. + // * Do not strip line currently being edited unless forced */ + // private void strip_trailing_spaces () { + // if (!loaded || source_view.language == null) { + // return; + // } + + // stripping = true; + // var source_buffer = (Gtk.SourceBuffer)source_view.buffer; + // Gtk.TextIter iter; + + // var cursor_pos = source_buffer.cursor_position; + // source_buffer.get_iter_at_offset (out iter, cursor_pos); + // var orig_line = iter.get_line (); + // var orig_offset = iter.get_line_offset (); + + // var text = source_buffer.text; + + // string[] lines = Regex.split_simple ("""[\r\n]""", text); + // if (lines.length == 0) { // Can legitimately happen at startup or new document + // return; + // } + + // if (lines.length != source_buffer.get_line_count ()) { + // critical ("Mismatch between line counts when stripping trailing spaces, not continuing"); + // debug ("lines.length %u, buffer lines %u \n %s", lines.length, source_buffer.get_line_count (), text); + // return; + // } + + // MatchInfo info; + // Gtk.TextIter start_delete, end_delete; + // Regex whitespace; + + // try { + // whitespace = new Regex ("[ \t]+$", 0); + // } catch (RegexError e) { + // critical ("Error while building regex to replace trailing whitespace: %s", e.message); + // return; + // } + + // for (int line_no = 0; line_no < lines.length; line_no++) { + // if (whitespace.match (lines[line_no], 0, out info)) { + + // source_buffer.get_iter_at_line (out start_delete, line_no); + // start_delete.forward_to_line_end (); + // end_delete = start_delete; + // end_delete.backward_chars (info.fetch (0).length); + + // source_buffer.begin_not_undoable_action (); + // source_buffer.@delete (ref start_delete, ref end_delete); + // source_buffer.end_not_undoable_action (); + // } + // } + + // source_buffer.get_iter_at_line_offset (out iter, orig_line, orig_offset); + // source_buffer.place_cursor (iter); + // } + + // stripping = false; + // } } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 24edde5a4b..baf0790a99 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -21,6 +21,7 @@ public class Scratch.Services.DocumentManager : Object { static Gee.HashMultiMap project_restorable_docs_map; static Gee.HashMultiMap project_open_docs_map; + static Gee.HashMap doc_timeout_map; static DocumentManager? instance; public static DocumentManager get_instance () { @@ -34,6 +35,7 @@ static construct { project_restorable_docs_map = new Gee.HashMultiMap (); project_open_docs_map = new Gee.HashMultiMap (); + doc_timeout_map = new Gee.HashMap (); } public void make_restorable (Document doc) { @@ -73,4 +75,146 @@ public uint open_for_project (string project_path) { return project_open_docs_map.@get (project_path).size; } + + /* Code to manage safe saving of documents */ + public void save_request (Document doc, bool is_closing) { + if (doc.inhibit_saving) { + return; + } + // Always save on closing. Otherwise only if autosave active. + if (!(is_closing || Scratch.settings.get_boolean ("autosave"))) { + return; + } + // Tab is closing or whole app is closing save immediately + if (is_closing) { + Source.remove (doc_timeout_map[doc]); + doc_timeout_map[doc] = null; + save_doc (doc); + } else if (doc_timeout_map[doc] == null { + doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => + if (doc.delay_saving) { + return Source.CONTINUE; + } + warning ("autosave doc %s", doc.file.get_path ()); + doc_timeout_map[doc] = null; + return Source.REMOVE; + }) + }) + } + + // This must only be called once the save is expected to succeed. + private void save_doc (Document doc) { + var save_buffer = new Gtk.SourceBuffer (null); + var source_buffer = doc.source_view.buffer; + save_buffer.text = source_buffer.text; + doc.before_save (); + + if (Scratch.settings.get_boolean ("strip-trailing-on-save") && + doc.source_view.language != null) { + Gtk.TextIter iter; + var cursor_pos = source_buffer.cursor_position; + source_buffer.get_iter_at_offset (out iter, cursor_pos); + var orig_line = iter.get_line (); + var orig_offset = iter.get_line_offset (); + strip_trailing_spaces (save_buffer); + source_buffer.begin_not_undoable_action (); + source_buffer.text = save_buffer.text + source_buffer.get_iter_at_line_offset ( + out iter, + orig_line, + orig_offset + ); + source_buffer.place_cursor (iter); + source_buffer.end_not_undoable_action (); + } + + doc.create_backup (); + // Replace old content with the new one + save_cancellable.cancel (); + save_cancellable = new GLib.Cancellable (); + var source_file_saver = new Gtk.SourceFileSaver ( + source_buffer, + doc.source_file + ); + + source_file_saver.save_async.begin ( + GLib.Priority.DEFAULT, + save_cancellable, + null, + + (obj, res) => { + try { + if (source_file_saver.save_async.end (res)) { + doc.set_saved_status (success); + doc.last_save_content = save_buffer.text; + debug ("File \"%s\" saved successfully", get_basename ()); + } + + doc.after_undoable_change (); + } catch { + if (e.code != 19) // Not cancelled + critical ( + "Cannot save \"%s\": %s", + get_basename (), + e.message + ); + } + } + } + ); + } + + private void create_doc_backup (Document doc) { + if (!can_write ()) { + return; + } + + var backup = File.new_for_path (this.file.get_path () + "~"); + if (!backup.query_exists ()) { + try { + file.copy (backup, FileCopyFlags.NONE); + } catch (Error e) { + warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); + } + } + } + + private void strip_trailing_spaces_before_save (Gtk.SourceBuffer save_buffer) { + + var text = save_buffer.text; + + string[] lines = Regex.split_simple ("""[\r\n]""", text); + if (lines.length == 0) { // Can legitimately happen at startup or new document + return; + } + + if (lines.length != save_buffer.get_line_count ()) { + critical ("Stripping: Mismatch between line counts, not continuing"); + return; + } + + MatchInfo info; + Gtk.TextIter start_delete, end_delete; + Regex whitespace; + + try { + whitespace = new Regex ("[ \t]+$", 0); + } catch (RegexError e) { + critical ("Stripping: error building regex", e.message); + assert_not_reached (); // Regex is constant so trap errors on dev + } + + for (int line_no = 0; line_no < lines.length; line_no++) { + if (whitespace.match (lines[line_no], 0, out info)) { + save_buffer.get_iter_at_line (out start_delete, line_no); + start_delete.forward_to_line_end (); + end_delete = start_delete; + end_delete.backward_chars (info.fetch (0).length); + + + save_buffer.@delete (ref start_delete, ref end_delete); + + } + } + } } diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index 30ed6016e6..f135634ae7 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -265,9 +265,8 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { var doc = new Services.Document (window.actions, file); doc.source_view.set_text (original.get_text ()); doc.source_view.language = original.source_view.language; - if (Scratch.settings.get_boolean ("autosave")) { - doc.save.begin (true); - } + DocumentManager.get_instance ().save_request (doc, false). + insert_document (doc, -1); current_document = doc; diff --git a/src/Widgets/SearchBar.vala b/src/Widgets/SearchBar.vala index 52e5a2dd4a..37da93b035 100644 --- a/src/Widgets/SearchBar.vala +++ b/src/Widgets/SearchBar.vala @@ -245,7 +245,8 @@ namespace Scratch.Widgets { } string replace_string = replace_entry.text; - this.window.get_current_document ().toggle_changed_handlers (false); + this.window.get_current_document ().before_undoable_change (); + // this.window.get_current_document ().toggle_changed_handlers (false); try { cancel_update_search_occurence_label (); search_context.replace_all (replace_string, replace_string.length); @@ -256,7 +257,8 @@ namespace Scratch.Widgets { critical (e.message); } - this.window.get_current_document ().toggle_changed_handlers (true); + // this.window.get_current_document ().toggle_changed_handlers (true); + this.window.get_current_document ().after_undoable_change (); } private void on_search_entry_text_changed () { From ff4e93d6966d6a2f5e0f57f0fc211852d0fbf6cb Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Thu, 26 Jan 2023 16:20:35 +0000 Subject: [PATCH 02/39] Partial fix 2 --- src/Services/Document.vala | 134 +++++++++++++++++------------- src/Services/DocumentManager.vala | 13 ++- 2 files changed, 86 insertions(+), 61 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1d47a15aff..2562a8ba48 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -209,7 +209,8 @@ namespace Scratch.Services { source_view.buffer.changed.connect (() => { if (source_view.buffer.text != last_save_content) { - saved = false; + DocumentManager.get_instance ().save_request (doc); + // saved = false; if (!Scratch.settings.get_boolean ("autosave")) { set_saved_status (false); } @@ -398,10 +399,16 @@ namespace Scratch.Services { return true; } + bool needs_save = source_view.buffer.modified || + (app_closing && is_file_temporary && !delete_temporary_file ()) bool ret_value = true; - if (Scratch.settings.get_boolean ("autosave") && !saved) { + if (source_view.buffer.text.modified) { + // if (Scratch.settings.get_boolean ("autosave") && !saved) { save_with_hold (); - } else if (app_closing && is_file_temporary && !delete_temporary_file ()) { + } else if (app_closing && + is_file_temporary && + !delete_temporary_file ()) { + // Save temp files with temp uri debug ("Save temporary file!"); save_with_hold (); } @@ -453,34 +460,41 @@ namespace Scratch.Services { return ret_value; } - public bool save_with_hold (bool force = false) { - GLib.Application.get_default ().hold (); - bool result = false; - save.begin (force, (obj, res) => { - result = save.end (res); - GLib.Application.get_default ().release (); - }); - - return result; + public bool save_with_hold () { + DocumentManager.get_instance ().save_request (doc, true); + // public bool save_with_hold (bool force = false) { + // GLib.Application.get_default ().hold (); + // bool result = false; + // save.begin (force, (obj, res) => { + // result = save.end (res); + // GLib.Application.get_default ().release (); + // }); + + // return result; } public bool save_as_with_hold () { - GLib.Application.get_default ().hold (); - bool result = false; - save_as.begin ((obj, res) => { - result = save_as.end (res); - GLib.Application.get_default ().release (); - }); + var new_path = get_save_as_uri (); + if (new_path = "") { + return false; + } - return result; + return save_with_hold (); } + // GLib.Application.get_default ().hold (); + // bool result = false; + // save_as.begin ((obj, res) => { + // result = save_as.end (res); + // GLib.Application.get_default ().release (); + // }); + + // return result; + // } - - - public async bool save_as () { - // New file + public string get_save_as_uri () { + // Get new path to save to from user if (!loaded) { - return false; + return ""; } var all_files_filter = new Gtk.FileFilter (); @@ -501,42 +515,46 @@ namespace Scratch.Services { file_chooser.add_filter (all_files_filter); file_chooser.add_filter (text_files_filter); file_chooser.do_overwrite_confirmation = true; - file_chooser.set_current_folder_uri (Utils.last_path ?? GLib.Environment.get_home_dir ()); + file_chooser.set_current_folder_uri ( + Utils.last_path ?? GLib.Environment.get_home_dir () + ); - var success = false; + var new_path = ""; var current_file = file.get_path (); var is_current_file_temporary = this.is_file_temporary; - if (file_chooser.run () == Gtk.ResponseType.ACCEPT) { file = File.new_for_uri (file_chooser.get_uri ()); // Update last visited path - Utils.last_path = Path.get_dirname (file_chooser.get_file ().get_uri ()); - success = true; + new_path = file_chooser.get_file ().get_uri (); + Utils.last_path = Path.get_dirname (new_path); } - if (success) { - source_view.buffer.set_modified (true); - var is_saved = yield save (true); + file_chooser.destroy (); + return new_path; + } + // if (success) { + // source_view.buffer.set_modified (true); + // var is_saved = yield save (true); + // if (is_saved && is_current_file_temporary) { + // try { + // // Delete temporary file + // File.new_for_path (current_file).delete (); + // } catch (Error err) { + // warning ("Temporary file cannot be deleted: %s", current_file); + // } + // } - if (is_saved && is_current_file_temporary) { - try { - // Delete temporary file - File.new_for_path (current_file).delete (); - } catch (Error err) { - warning ("Temporary file cannot be deleted: %s", current_file); - } - } + // delete_backup (current_file + "~"); + // this.source_view.change_syntax_highlight_from_file (this.file); + // } - delete_backup (current_file + "~"); - this.source_view.change_syntax_highlight_from_file (this.file); - } + // /* We delay destruction of file chooser dialog til now to avoid + // * the document focussing in, + // * which triggers premature loading of overwritten content. + // */ - /* We delay destruction of file chooser dialog til to avoid the document focussing in, - * which triggers premature loading of overwritten content. - */ - file_chooser.destroy (); - return success; - } + // return success; + // } public bool move (File new_dest) { this.file = new_dest; @@ -589,9 +607,13 @@ namespace Scratch.Services { } // Set InfoBars message - public void set_message (Gtk.MessageType type, string label, - string? button1 = null, owned VoidFunc? callback1 = null, - string? button2 = null, owned VoidFunc? callback2 = null) { + public void set_message ( + Gtk.MessageType type, string label, + string? button1 = null, + owned VoidFunc? callback1 = null, + string? button2 = null, + owned VoidFunc? callback2 = null + ) { // Show InfoBar info_bar.no_show_all = false; @@ -714,6 +736,7 @@ namespace Scratch.Services { } // Check if the file was deleted/changed by an external source + // Only called on focus in public void check_file_status () { // If the file does not exist anymore if (!exists ()) { @@ -818,15 +841,10 @@ namespace Scratch.Services { } } - - - // Set saved status public void set_saved_status (bool val) { - this.saved = val; - + // this.saved = val; string unsaved_identifier = "* "; - if (!val) { if (!(unsaved_identifier in this.label)) { tab_name = unsaved_identifier + this.label; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index baf0790a99..8e83b89ce2 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -89,13 +89,14 @@ if (is_closing) { Source.remove (doc_timeout_map[doc]); doc_timeout_map[doc] = null; - save_doc (doc); + save_doc (doc, true); } else if (doc_timeout_map[doc] == null { doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => if (doc.delay_saving) { return Source.CONTINUE; } warning ("autosave doc %s", doc.file.get_path ()); + save_doc (doc, false); doc_timeout_map[doc] = null; return Source.REMOVE; }) @@ -103,7 +104,7 @@ } // This must only be called once the save is expected to succeed. - private void save_doc (Document doc) { + private void save_doc (Document doc, bool with_hold) { var save_buffer = new Gtk.SourceBuffer (null); var source_buffer = doc.source_view.buffer; save_buffer.text = source_buffer.text; @@ -137,11 +138,13 @@ doc.source_file ); + if (with_hold) { + GLib.Application.get_default ().hold (); + } source_file_saver.save_async.begin ( GLib.Priority.DEFAULT, save_cancellable, null, - (obj, res) => { try { if (source_file_saver.save_async.end (res)) { @@ -159,6 +162,10 @@ e.message ); } + } finally { + if (with_hold) { + GLib.Application.get_default ().release (); + } } } ); From b38170978ca55c00f40113de1005d1460040a70f Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Fri, 27 Jan 2023 11:55:55 +0000 Subject: [PATCH 03/39] Partial fix 3 --- src/Services/Document.vala | 161 +++++++++++++++--------------- src/Services/DocumentManager.vala | 99 +++++++++++++++--- 2 files changed, 166 insertions(+), 94 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 2562a8ba48..066fd84b17 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -107,7 +107,7 @@ namespace Scratch.Services { private Scratch.Services.SymbolOutline? outline = null; private string original_content; // For restoring to original - private string last_save_content; + private string last_save_content; // For detecting unsaved content private bool completion_shown = false; private bool loaded = false; @@ -198,22 +198,24 @@ namespace Scratch.Services { // toggle_changed_handlers (true); - // Focus out event for SourceView - this.source_view.focus_out_event.connect (() => { - if (Scratch.settings.get_boolean ("autosave")) { - save.begin (); - } + //TODO Reimplement this with DocumentManager + // // Focus out event for SourceView + // this.source_view.focus_out_event.connect (() => { + // if (Scratch.settings.get_boolean ("autosave")) { + // save.begin (); + // } - return false; - }); + // return false; + // }); source_view.buffer.changed.connect (() => { if (source_view.buffer.text != last_save_content) { - DocumentManager.get_instance ().save_request (doc); + set_saved_status (false); // For visibility while developing + DocumentManager.get_instance ().save_request (doc, false); // saved = false; - if (!Scratch.settings.get_boolean ("autosave")) { - set_saved_status (false); - } + // if (!Scratch.settings.get_boolean ("autosave")) { + // set_saved_status (false); + // } } else { set_saved_status (true); } @@ -373,7 +375,7 @@ namespace Scratch.Services { source_view.buffer.set_modified (false); original_content = source_view.buffer.text; - last_save_content = source_view.buffer.text; + last_save_content = original_content; set_saved_status (true); doc_opened (); @@ -391,6 +393,8 @@ namespace Scratch.Services { return; } + // Return "true" allows app to close else quitting will be cancelled + // Note: return "false" does not stop a tab from closing (atm)?? public bool do_close (bool app_closing = false) { debug ("Closing \"%s\"", get_basename ()); @@ -399,66 +403,74 @@ namespace Scratch.Services { return true; } - bool needs_save = source_view.buffer.modified || - (app_closing && is_file_temporary && !delete_temporary_file ()) - bool ret_value = true; - if (source_view.buffer.text.modified) { - // if (Scratch.settings.get_boolean ("autosave") && !saved) { - save_with_hold (); - } else if (app_closing && - is_file_temporary && - !delete_temporary_file ()) { - // Save temp files with temp uri - debug ("Save temporary file!"); - save_with_hold (); - } - // Check for unsaved changes - else if (!this.saved || (!app_closing && is_file_temporary && !delete_temporary_file ())) { - var parent_window = source_view.get_toplevel () as Gtk.Window; - - var dialog = new Granite.MessageDialog ( - _("Save changes to \"%s\" before closing?").printf (this.get_basename ()), - _("If you don't save, changes will be permanently lost."), - new ThemedIcon ("dialog-warning"), - Gtk.ButtonsType.NONE - ); - dialog.transient_for = parent_window; - - var no_save_button = (Gtk.Button) dialog.add_button (_("Close Without Saving"), Gtk.ResponseType.NO); - no_save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION); - - dialog.add_button (_("Cancel"), Gtk.ResponseType.CANCEL); - dialog.add_button (_("Save"), Gtk.ResponseType.YES); - dialog.set_default_response (Gtk.ResponseType.YES); - - int response = dialog.run (); - switch (response) { - case Gtk.ResponseType.CANCEL: - case Gtk.ResponseType.DELETE_EVENT: - ret_value = false; - break; - case Gtk.ResponseType.YES: - if (this.is_file_temporary) - save_as_with_hold (); - else - save_with_hold (); - break; - case Gtk.ResponseType.NO: - if (this.is_file_temporary) - delete_temporary_file (true); - break; - } - dialog.destroy (); - } - - if (ret_value) { - // Delete backup copy file + if (DocumentManager.get_instance ().save_request (doc, true)) { delete_backup (); doc_closed (); + return true; } - return ret_value; + return false; } + // bool needs_save = source_view.buffer.modified || + // (app_closing && is_file_temporary && !delete_temporary_file ()) + // bool ret_value = true; + // if (source_view.buffer.text.modified) { + // // if (Scratch.settings.get_boolean ("autosave") && !saved) { + // save_with_hold (); + // } else if (app_closing && + // is_file_temporary && + // !delete_temporary_file ()) { + // // Save temp files with temp uri + // debug ("Save temporary file!"); + // save_with_hold (); + // } + // // Check for unsaved changes + // else if (!this.saved || (!app_closing && is_file_temporary && !delete_temporary_file ())) { + // var parent_window = source_view.get_toplevel () as Gtk.Window; + + // var dialog = new Granite.MessageDialog ( + // _("Save changes to \"%s\" before closing?").printf (this.get_basename ()), + // _("If you don't save, changes will be permanently lost."), + // new ThemedIcon ("dialog-warning"), + // Gtk.ButtonsType.NONE + // ); + // dialog.transient_for = parent_window; + + // var no_save_button = (Gtk.Button) dialog.add_button (_("Close Without Saving"), Gtk.ResponseType.NO); + // no_save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION); + + // dialog.add_button (_("Cancel"), Gtk.ResponseType.CANCEL); + // dialog.add_button (_("Save"), Gtk.ResponseType.YES); + // dialog.set_default_response (Gtk.ResponseType.YES); + + // int response = dialog.run (); + // switch (response) { + // case Gtk.ResponseType.CANCEL: + // case Gtk.ResponseType.DELETE_EVENT: + // ret_value = false; + // break; + // case Gtk.ResponseType.YES: + // if (this.is_file_temporary) + // save_as_with_hold (); + // else + // save_with_hold (); + // break; + // case Gtk.ResponseType.NO: + // if (this.is_file_temporary) + // delete_temporary_file (true); + // break; + // } + // dialog.destroy (); + // } + + // if (ret_value) { + // // Delete backup copy file + // delete_backup (); + // doc_closed (); + // } + + // return ret_value; + // } public bool save_with_hold () { DocumentManager.get_instance ().save_request (doc, true); @@ -895,20 +907,7 @@ namespace Scratch.Services { } } - private bool delete_temporary_file (bool force = false) { - if (!is_file_temporary || (get_text ().length > 0 && !force)) { - return false; - } - - try { - file.delete (); - return true; - } catch (Error e) { - warning ("Cannot delete temporary file \"%s\": %s", file.get_uri (), e.message); - } - return false; - } // Return true if the file is writable. Keep testing as may change private bool can_write () { diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 8e83b89ce2..9c08719114 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -77,22 +77,43 @@ } /* Code to manage safe saving of documents */ - public void save_request (Document doc, bool is_closing) { + /*******************************************/ + + // @force is "true" when tab or app is closing or when user activated "action-save" + // Returns "false" if operation cancelled by user + public bool save_request (Document doc, bool force) { if (doc.inhibit_saving) { - return; + //TODO Confirm with user whether to proceed? + return true; } + + var autosave = Scratch.settings.get_boolean ("autosave"); // Always save on closing. Otherwise only if autosave active. - if (!(is_closing || Scratch.settings.get_boolean ("autosave"))) { + if (!(force || autosave)) { + // Comes here when changed return; } - // Tab is closing or whole app is closing save immediately - if (is_closing) { - Source.remove (doc_timeout_map[doc]); - doc_timeout_map[doc] = null; - save_doc (doc, true); + + if (force) { + Source.remove (doc_timeout_map[doc]); + doc_timeout_map[doc] = null; + if (!autosave && doc.is_changed) { + bool save_changes; + if (!query_save_changes (doc, out save_changes)) { + return false; + } + + if (!save_changes) { + delete_if_temporary (doc); + return true; + } + } + + save_doc (doc, true); } else if (doc_timeout_map[doc] == null { + // Autosaving - need to create new timeout doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => - if (doc.delay_saving) { + if (doc.delay_saving || doc.is_saving) { return Source.CONTINUE; } warning ("autosave doc %s", doc.file.get_path ()); @@ -103,7 +124,8 @@ }) } - // This must only be called once the save is expected to succeed. + // This must only be called when the save is expected to succeed + // e.g. during autosave. private void save_doc (Document doc, bool with_hold) { var save_buffer = new Gtk.SourceBuffer (null); var source_buffer = doc.source_view.buffer; @@ -187,9 +209,7 @@ } private void strip_trailing_spaces_before_save (Gtk.SourceBuffer save_buffer) { - var text = save_buffer.text; - string[] lines = Regex.split_simple ("""[\r\n]""", text); if (lines.length == 0) { // Can legitimately happen at startup or new document return; @@ -224,4 +244,57 @@ } } } - } + + private bool delete_if_temporary (Document doc) { + if (!is_file_temporary) { + return false; + } + + try { + file.delete (); + return true; + } catch (Error e) { + warning ("Cannot delete temporary file \"%s\": %s", file.get_uri (), e.message); + } + + return false; + } + + private bool query_save_changes (Document doc, out bool save_changes) { + var parent_window = source_view.get_toplevel () as Gtk.Window; + var dialog = new Granite.MessageDialog ( + _("Save changes to \"%s\" before closing?").printf (this.get_basename ()), + _("If you don't save, changes will be permanently lost."), + new ThemedIcon ("dialog-warning"), + Gtk.ButtonsType.NONE + ); + dialog.transient_for = parent_window; + var no_save_button = (Gtk.Button) dialog.add_button ( + _("Close Without Saving"), + Gtk.ResponseType.NO + ); + no_save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION); + dialog.add_button (_("Cancel"), Gtk.ResponseType.CANCEL); + dialog.add_button (_("Save"), Gtk.ResponseType.YES); + dialog.set_default_response (Gtk.ResponseType.YES); + int response = dialog.run (); + bool close_document; + switch (response) { + case Gtk.ResponseType.CANCEL: + case Gtk.ResponseType.DELETE_EVENT: + save_changes = false; + close_document = false; + break; + case Gtk.ResponseType.YES: + save_changes = true; + close_document = true; + break; + case Gtk.ResponseType.NO: + save_changes = false; + close_document = true; + } + + dialog.destroy (); + return close_document; + } +} From 9fef13bbe29e4fbb154b52eea0cf0d037704e341 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Fri, 27 Jan 2023 14:40:26 +0000 Subject: [PATCH 04/39] Cancel closing project folder if cannot close a document --- src/FolderManager/FileView.vala | 45 +++++++++++++++--------- src/FolderManager/ProjectFolderItem.vala | 4 --- src/MainWindow.vala | 8 ++++- src/Services/Document.vala | 1 + src/Services/GitManager.vala | 1 - src/Widgets/DocumentView.vala | 11 ++++-- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/FolderManager/FileView.vala b/src/FolderManager/FileView.vala index 2e3ae4a33c..233b3c258f 100644 --- a/src/FolderManager/FileView.vala +++ b/src/FolderManager/FileView.vala @@ -173,9 +173,10 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P public ProjectFolderItem? get_project_for_file (GLib.File file) { foreach (var item in root.children) { if (item is ProjectFolderItem) { - var folder = (ProjectFolderItem)item; - if (folder.contains_file (file)) { - return folder; + var folder_file = ((ProjectFolderItem)item).file.file; + if (folder_file.equal (file) || + folder_file.get_relative_path (file) != null) { + return (ProjectFolderItem)item; } } } @@ -262,16 +263,10 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P folder_root.expanded = expand; folder_root.closed.connect (() => { - toplevel_action_group.activate_action (MainWindow.ACTION_CLOSE_PROJECT_DOCS, new Variant.string (folder_root.path)); - root.remove (folder_root); - foreach (var child in root.children) { - var child_folder = (ProjectFolderItem) child; - if (child_folder.name != child_folder.file.name) { - rename_items_with_same_name (child_folder); - } - } - Scratch.Services.GitManager.get_instance ().remove_project (folder_root); - write_settings (); + toplevel_action_group.activate_action ( + MainWindow.ACTION_CLOSE_PROJECT_DOCS, + new Variant.string (folder_root.path) + ); }); folder_root.close_all_except.connect (() => { @@ -279,17 +274,33 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P var project_folder_item = (ProjectFolderItem)child; if (project_folder_item != folder_root) { toplevel_action_group.activate_action (MainWindow.ACTION_CLOSE_PROJECT_DOCS, new Variant.string (project_folder_item.path)); - root.remove (project_folder_item); - Scratch.Services.GitManager.get_instance ().remove_project (project_folder_item); } } - - write_settings (); }); write_settings (); } + // Callback for MainWindow.ACTION_CLOSE_PROJECT_DOCS when not cancelled + public void remove_project_for_path (string path) { + var folder_root = get_project_for_file (GLib.File.new_for_path (path)); + if (folder_root == null) { + critical ("Could not find project for path %s", path); + return; + } + + root.remove (folder_root); + foreach (var child in root.children) { + var child_folder = (ProjectFolderItem) child; + if (child_folder.name != child_folder.file.name) { + rename_items_with_same_name (child_folder); + } + } + + Scratch.Services.GitManager.get_instance ().remove_project (folder_root); + write_settings (); + } + private bool is_open (File folder) { foreach (var child in root.children) if (folder.path == ((Item) child).path) diff --git a/src/FolderManager/ProjectFolderItem.vala b/src/FolderManager/ProjectFolderItem.vala index cc4dca33cc..438fb9f771 100644 --- a/src/FolderManager/ProjectFolderItem.vala +++ b/src/FolderManager/ProjectFolderItem.vala @@ -264,10 +264,6 @@ namespace Scratch.FolderManager { }); } - public bool contains_file (GLib.File descendant) { - return file.file.get_relative_path (descendant) != null; - } - private void deprioritize_git_ignored () requires (monitored_repo != null) { visible_item_list.@foreach ((visible_item) => { var item = visible_item.item; diff --git a/src/MainWindow.vala b/src/MainWindow.vala index 53b8635edb..318f00b83e 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -949,16 +949,22 @@ namespace Scratch { unowned var docs = document_view.docs; docs.foreach ((doc) => { if (doc.file.get_path ().has_prefix (project_path)) { - document_view.close_document (doc); + if (!(document_view.close_document (doc))) { + return; + } + if (make_restorable) { document_manager.make_restorable (doc); } } }); + if (!make_restorable) { document_manager.remove_project (project_path); } + + folder_manager_view.remove_project_for_path (project_path); } private void restore_project_docs (string project_path) { diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1db488abf1..a117188772 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -373,6 +373,7 @@ namespace Scratch.Services { return; } + // Returns "false" only if user cancelled operation public bool do_close (bool app_closing = false) { debug ("Closing \"%s\"", get_basename ()); diff --git a/src/Services/GitManager.vala b/src/Services/GitManager.vala index 891fdc1a3f..04fd6f30c8 100644 --- a/src/Services/GitManager.vala +++ b/src/Services/GitManager.vala @@ -82,7 +82,6 @@ namespace Scratch.Services { public void remove_project (FolderManager.ProjectFolderItem root_folder) { var root_path = root_folder.file.file.get_path (); - uint position; if (project_liststore.find (root_folder, out position)) { project_liststore.remove (position); diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index 30ed6016e6..cc45943100 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -303,9 +303,14 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } } - public void close_document (Services.Document doc) { - remove_tab (doc); - doc.do_close (); + public bool close_document (Services.Document doc) { + if (doc.do_close ()) { + remove_tab (doc); + return true; + } + + return false; + } public void close_current_document () { From abb5d0515162ca8c097c74d5cf31e1fda8a2519a Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Fri, 27 Jan 2023 16:10:00 +0000 Subject: [PATCH 05/39] First compilable version --- src/MainWindow.vala | 4 +- src/Services/Document.vala | 115 +++++++++++++++++++----------- src/Services/DocumentManager.vala | 91 ++++++++++++++--------- src/Widgets/DocumentView.vala | 4 +- 4 files changed, 134 insertions(+), 80 deletions(-) diff --git a/src/MainWindow.vala b/src/MainWindow.vala index 318f00b83e..dee4c26f5e 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -872,7 +872,7 @@ namespace Scratch { if (doc.is_file_temporary == true) { action_save_as (); } else { - doc.save.begin (true); + doc.save (); } } } @@ -880,7 +880,7 @@ namespace Scratch { private void action_save_as () { var doc = get_current_document (); if (doc != null) { - doc.save_as.begin (); + doc.save_as (); } } diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 5ba9e88b2c..0547d034c1 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -43,7 +43,8 @@ namespace Scratch.Services { } } - private Gtk.SourceFile source_file; + public Gtk.SourceFile source_file { get; private set; } + public GLib.File file { get { return source_file.location; @@ -116,10 +117,11 @@ namespace Scratch.Services { private Gtk.SourceMap source_map; private Gtk.Paned outline_widget_pane; - private GLib.Cancellable save_cancellable; - private GLib.Cancellable load_cancellable; - // private ulong onchange_handler_id = 0; // It is used to not mark files as changed on load + // Used by DocumentManager + public GLib.Cancellable save_cancellable; + public GLib.Cancellable load_cancellable; + // private ulong onchange_handler_id = 0; // It is used to not mark files as changed on load private bool mounted = true; // Mount state of the file private Mount mount; @@ -211,7 +213,7 @@ namespace Scratch.Services { source_view.buffer.changed.connect (() => { if (source_view.buffer.text != last_save_content) { set_saved_status (false); // For visibility while developing - DocumentManager.get_instance ().save_request (doc, false); + DocumentManager.get_instance ().save_request (this, false); // saved = false; // if (!Scratch.settings.get_boolean ("autosave")) { // set_saved_status (false); @@ -402,7 +404,7 @@ namespace Scratch.Services { return true; } - if (DocumentManager.get_instance ().save_request (doc, true)) { + if (DocumentManager.get_instance ().save_request (this, true)) { delete_backup (); doc_closed (); return true; @@ -471,8 +473,9 @@ namespace Scratch.Services { // return ret_value; // } - public bool save_with_hold () { - DocumentManager.get_instance ().save_request (doc, true); + public bool save () { + // public bool save_with_hold () { + return DocumentManager.get_instance ().save_request (this, true); // public bool save_with_hold (bool force = false) { // GLib.Application.get_default ().hold (); // bool result = false; @@ -484,13 +487,26 @@ namespace Scratch.Services { // return result; } - public bool save_as_with_hold () { - var new_path = get_save_as_uri (); - if (new_path = "") { + public bool save_as () { + var new_uri = get_save_as_uri (); + if (new_uri != null) { + var old_uri = file.get_uri (); + file = GLib.File.new_for_uri (new_uri); + if (!DocumentManager.get_instance ().save_request (this, true)) { + file = GLib.File.new_for_uri (old_uri); + return false; + } else { + return true; + } + } else { return false; } + // var new_path = get_save_as_uri (); + // if (new_path = "") { + // return false; + // } - return save_with_hold (); + // DocumentManager.get_instance ().save_request (this, true); } // GLib.Application.get_default ().hold (); // bool result = false; @@ -531,8 +547,8 @@ namespace Scratch.Services { ); var new_path = ""; - var current_file = file.get_path (); - var is_current_file_temporary = this.is_file_temporary; + // var current_file = file.get_path (); + // var is_current_file_temporary = this.is_file_temporary; if (file_chooser.run () == Gtk.ResponseType.ACCEPT) { file = File.new_for_uri (file_chooser.get_uri ()); // Update last visited path @@ -569,9 +585,7 @@ namespace Scratch.Services { public bool move (File new_dest) { this.file = new_dest; - this.save.begin (); - - return true; + return DocumentManager.get_instance ().save_request (this, true); } private void restore_settings () { @@ -584,9 +598,11 @@ namespace Scratch.Services { scroll.vscrollbar_policy = Gtk.PolicyType.AUTOMATIC; } - if (Scratch.settings.get_boolean ("strip-trailing-on-save")) { - strip_trailing_spaces (); - } + // Stripping spaces only happens before save + + // if (Scratch.settings.get_boolean ("strip-trailing-on-save")) { + // strip_trailing_spaces (); + // } } // Focus the SourceView @@ -757,8 +773,16 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save As…"), () => { - this.save_as.begin (); + // this.save_as.begin (); hide_info_bar (); + var new_uri = get_save_as_uri (); + if (new_uri != null) { + var old_uri = file.get_uri (); + file = GLib.File.new_for_uri (new_uri); + if (!DocumentManager.get_instance ().save_request (this, true)) { + file = GLib.File.new_for_uri (old_uri); + } + } }); } else { string message = _( @@ -766,8 +790,8 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { - this.save.begin (); hide_info_bar (); + DocumentManager.get_instance ().save_request (this, true); }); } @@ -783,8 +807,16 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save changes elsewhere"), () => { - this.save_as.begin (); hide_info_bar (); + //TODO DRY this block + var new_uri = get_save_as_uri (); + if (new_uri != null) { + var old_uri = file.get_uri (); + file = GLib.File.new_for_uri (new_uri); + if (!DocumentManager.get_instance ().save_request (this, true)) { + file = GLib.File.new_for_uri (old_uri); + } + } }); Utils.action_from_group (MainWindow.ACTION_SAVE, actions).set_enabled (false); @@ -811,8 +843,13 @@ namespace Scratch.Services { return; } + // "is_modified" returns false if no internal edits made since last time + // the app saved the content. if (!source_view.buffer.get_modified ()) { + // The source_view.buffer has not been modified in app + //TODO Needs explanation? if (Scratch.settings.get_boolean ("autosave")) { + // Not sure why we do not confirm in this case. source_view.set_text (new_buffer.text, false); } else { string message = _( @@ -826,6 +863,10 @@ namespace Scratch.Services { hide_info_bar (); }); } + } else { + critical ("Possibly conflicting external and in app edits"); + //TODO Handle this case? + // Currently assume that the in app edits take priority over any external edits. } }); } @@ -842,11 +883,12 @@ namespace Scratch.Services { } public void before_undoable_change () { - source_buffer.set_editable (false); + source_view.set_editable (false); } public void after_undoable_change () { source_view.buffer.set_modified (false); + source_view.set_editable (true); if (outline != null) { outline.parse_symbols (); } @@ -861,26 +903,16 @@ namespace Scratch.Services { tab_name = unsaved_identifier + this.label; } } else { - tab_name = this.label.replace (unsaved_identifier, ""); + if (!source_view.buffer.get_modified ()) { + tab_name = this.label.replace (unsaved_identifier, ""); + last_save_content = source_view.buffer.text; + } else { + critical ("Buffer contents changed during save operation"); + } } } - // // Backup functions - // private void create_backup () { - // if (!can_write ()) { - // return; - // } - - // var backup = File.new_for_path (this.file.get_path () + "~"); - // if (!backup.query_exists ()) { - // try { - // file.copy (backup, FileCopyFlags.NONE); - // } catch (Error e) { - // warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); - // } - // } - // } private void delete_backup (string? backup_path = null) { string backup_file; @@ -909,7 +941,7 @@ namespace Scratch.Services { // Return true if the file is writable. Keep testing as may change - private bool can_write () { + public bool can_write () { FileInfo info; bool writable = false; try { @@ -991,6 +1023,7 @@ namespace Scratch.Services { text.buffer.place_cursor (iter); text.scroll_to_iter (iter, 0.0, true, 0.5, 0.5); } + } // /* Pull the buffer into an array and then work out which parts are to be deleted. // * Do not strip line currently being edited unless forced */ diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 9c08719114..d7d992fcce 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -22,6 +22,7 @@ static Gee.HashMultiMap project_restorable_docs_map; static Gee.HashMultiMap project_open_docs_map; static Gee.HashMap doc_timeout_map; + uint AUTOSAVE_RATE_MSEC = 1000; static DocumentManager? instance; public static DocumentManager get_instance () { @@ -91,13 +92,15 @@ // Always save on closing. Otherwise only if autosave active. if (!(force || autosave)) { // Comes here when changed - return; + return true; } if (force) { - Source.remove (doc_timeout_map[doc]); - doc_timeout_map[doc] = null; - if (!autosave && doc.is_changed) { + if (doc_timeout_map.has_key (doc)) { + Source.remove (doc_timeout_map[doc]); + doc_timeout_map.unset (doc); + } + if (doc.source_view.buffer.get_modified ()) { bool save_changes; if (!query_save_changes (doc, out save_changes)) { return false; @@ -110,27 +113,29 @@ } save_doc (doc, true); - } else if (doc_timeout_map[doc] == null { + } else if (!doc_timeout_map.has_key (doc)) { // Autosaving - need to create new timeout - doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => + doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { if (doc.delay_saving || doc.is_saving) { return Source.CONTINUE; } warning ("autosave doc %s", doc.file.get_path ()); save_doc (doc, false); - doc_timeout_map[doc] = null; + doc_timeout_map.unset (doc); return Source.REMOVE; - }) - }) + }); + } + + return true; } // This must only be called when the save is expected to succeed // e.g. during autosave. private void save_doc (Document doc, bool with_hold) { var save_buffer = new Gtk.SourceBuffer (null); - var source_buffer = doc.source_view.buffer; + var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); save_buffer.text = source_buffer.text; - doc.before_save (); + doc.before_undoable_change (); if (Scratch.settings.get_boolean ("strip-trailing-on-save") && doc.source_view.language != null) { @@ -139,9 +144,9 @@ source_buffer.get_iter_at_offset (out iter, cursor_pos); var orig_line = iter.get_line (); var orig_offset = iter.get_line_offset (); - strip_trailing_spaces (save_buffer); + strip_trailing_spaces_before_save (save_buffer); source_buffer.begin_not_undoable_action (); - source_buffer.text = save_buffer.text + source_buffer.text = save_buffer.text; source_buffer.get_iter_at_line_offset ( out iter, orig_line, @@ -151,10 +156,11 @@ source_buffer.end_not_undoable_action (); } - doc.create_backup (); + create_doc_backup (doc); // Replace old content with the new one - save_cancellable.cancel (); - save_cancellable = new GLib.Cancellable (); + //TODO Handle cancellables internally + doc.save_cancellable.cancel (); + doc.save_cancellable = new GLib.Cancellable (); var source_file_saver = new Gtk.SourceFileSaver ( source_buffer, doc.source_file @@ -165,22 +171,21 @@ } source_file_saver.save_async.begin ( GLib.Priority.DEFAULT, - save_cancellable, + doc.save_cancellable, null, (obj, res) => { try { if (source_file_saver.save_async.end (res)) { - doc.set_saved_status (success); - doc.last_save_content = save_buffer.text; - debug ("File \"%s\" saved successfully", get_basename ()); + doc.set_saved_status (true); + debug ("File \"%s\" saved successfully", doc.get_basename ()); } doc.after_undoable_change (); - } catch { - if (e.code != 19) // Not cancelled + } catch (Error e) { + if (e.code != 19) { // Not cancelled critical ( "Cannot save \"%s\": %s", - get_basename (), + doc.get_basename (), e.message ); } @@ -193,17 +198,34 @@ ); } + // // Backup functions + // private void create_backup () { + // if (!can_write ()) { + // return; + // } + + // var backup = File.new_for_path (this.file.get_path () + "~"); + + // if (!backup.query_exists ()) { + // try { + // file.copy (backup, FileCopyFlags.NONE); + // } catch (Error e) { + // warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); + // } + // } + // } + private void create_doc_backup (Document doc) { - if (!can_write ()) { + if (!doc.can_write ()) { return; } - var backup = File.new_for_path (this.file.get_path () + "~"); + var backup = File.new_for_path (doc.file.get_path () + "~"); if (!backup.query_exists ()) { try { - file.copy (backup, FileCopyFlags.NONE); + doc.file.copy (backup, FileCopyFlags.NONE); } catch (Error e) { - warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); + warning ("Cannot create backup copy for file \"%s\": %s", doc.get_basename (), e.message); } } } @@ -227,7 +249,7 @@ try { whitespace = new Regex ("[ \t]+$", 0); } catch (RegexError e) { - critical ("Stripping: error building regex", e.message); + critical ("Stripping: error building regex: %s", e.message); assert_not_reached (); // Regex is constant so trap errors on dev } @@ -246,24 +268,24 @@ } private bool delete_if_temporary (Document doc) { - if (!is_file_temporary) { + if (!doc.is_file_temporary) { return false; } try { - file.delete (); + doc.file.delete (); return true; } catch (Error e) { - warning ("Cannot delete temporary file \"%s\": %s", file.get_uri (), e.message); + warning ("Cannot delete temporary file \"%s\": %s", doc.file.get_uri (), e.message); } return false; } private bool query_save_changes (Document doc, out bool save_changes) { - var parent_window = source_view.get_toplevel () as Gtk.Window; + var parent_window = doc.source_view.get_toplevel () as Gtk.Window; var dialog = new Granite.MessageDialog ( - _("Save changes to \"%s\" before closing?").printf (this.get_basename ()), + _("Save changes to \"%s\" before closing?").printf (doc.get_basename ()), _("If you don't save, changes will be permanently lost."), new ThemedIcon ("dialog-warning"), Gtk.ButtonsType.NONE @@ -278,7 +300,7 @@ dialog.add_button (_("Save"), Gtk.ResponseType.YES); dialog.set_default_response (Gtk.ResponseType.YES); int response = dialog.run (); - bool close_document; + bool close_document = false; switch (response) { case Gtk.ResponseType.CANCEL: case Gtk.ResponseType.DELETE_EVENT: @@ -292,6 +314,7 @@ case Gtk.ResponseType.NO: save_changes = false; close_document = true; + break; } dialog.destroy (); diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index b897c42b84..c101f911c1 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -265,8 +265,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { var doc = new Services.Document (window.actions, file); doc.source_view.set_text (original.get_text ()); doc.source_view.language = original.source_view.language; - DocumentManager.get_instance ().save_request (doc, false). - + // Document will be either autosaved or treeted as unsaved document insert_document (doc, -1); current_document = doc; @@ -309,7 +308,6 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } return false; - } public void close_current_document () { From 9c12dec4092e9978e255e5ce45d3ef8439ebae9b Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sat, 28 Jan 2023 17:06:08 +0000 Subject: [PATCH 06/39] Provide branch build from as Constant.BRANCH; show in header --- src/Widgets/HeaderBar.vala | 9 +++++++++ src/config.vala.in | 1 + src/meson.build | 3 +++ 3 files changed, 13 insertions(+) diff --git a/src/Widgets/HeaderBar.vala b/src/Widgets/HeaderBar.vala index 780fd8bf1f..bd4e454901 100644 --- a/src/Widgets/HeaderBar.vala +++ b/src/Widgets/HeaderBar.vala @@ -245,11 +245,20 @@ public class Scratch.HeaderBar : Hdy.HeaderBar { }; set_custom_title (format_bar); + var branch_label = new Gtk.Label ( + _("Running Development Branch: %s").printf (Constants.BRANCH) + ); + + if (branch_label.label == "master" || branch_label.label == "main") { + branch_label.visible = false; + branch_label.no_show_all = true; + } pack_start (open_button); pack_start (templates_button); pack_start (save_button); pack_start (save_as_button); pack_start (revert_button); + pack_start (branch_label); pack_end (app_menu); pack_end (share_app_menu); diff --git a/src/config.vala.in b/src/config.vala.in index 6f6e2b4077..28b87ae2d6 100644 --- a/src/config.vala.in +++ b/src/config.vala.in @@ -6,4 +6,5 @@ namespace Constants { public const string INSTALL_PREFIX = @PREFIX@; public const string DATADIR = @DATADIR@; public const string LOCALEDIR = @LOCALEDIR@; + public const string BRANCH = @BRANCH@; } diff --git a/src/meson.build b/src/meson.build index 2535e6fd96..f62e01d116 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,3 +1,5 @@ +output = run_command('git','branch','--show-current') +branch = output.stdout().strip() conf_data = configuration_data() conf_data.set_quoted('PROJECT_NAME', meson.project_name()) conf_data.set_quoted('GETTEXT_PACKAGE', meson.project_name()) @@ -6,6 +8,7 @@ conf_data.set_quoted('PREFIX', get_option('prefix')) conf_data.set_quoted('PLUGINDIR', pluginsdir) conf_data.set_quoted('DATADIR', join_paths (get_option('prefix'), get_option('datadir'))) conf_data.set_quoted('LOCALEDIR', join_paths (get_option('prefix'), get_option('localedir'))) +conf_data.set_quoted('BRANCH', branch) config_header = configure_file( input : 'config.vala.in', output : 'config.vala', From e7fc1138755b4d856b17ee4ac8158d58cf3ccba2 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sat, 28 Jan 2023 17:15:23 +0000 Subject: [PATCH 07/39] Only insert branch label if will be shown; tooltip --- src/Widgets/HeaderBar.vala | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Widgets/HeaderBar.vala b/src/Widgets/HeaderBar.vala index bd4e454901..80da9fd22e 100644 --- a/src/Widgets/HeaderBar.vala +++ b/src/Widgets/HeaderBar.vala @@ -245,20 +245,26 @@ public class Scratch.HeaderBar : Hdy.HeaderBar { }; set_custom_title (format_bar); - var branch_label = new Gtk.Label ( - _("Running Development Branch: %s").printf (Constants.BRANCH) - ); - - if (branch_label.label == "master" || branch_label.label == "main") { - branch_label.visible = false; - branch_label.no_show_all = true; - } pack_start (open_button); pack_start (templates_button); pack_start (save_button); pack_start (save_as_button); pack_start (revert_button); - pack_start (branch_label); + + if (Constants.BRANCH != null && + Constants.BRANCH != "" && + Constants.BRANCH != "master" && + Constants.BRANCH != "main") { + + //TODO Decide on best place to expose this information + //Putting in headerbar for immediate visibility while developing + var branch_label = new Gtk.Label (Constants.BRANCH) { + tooltip_text = _("Branch of source code currently running") + }; + + pack_start (branch_label); + } + pack_end (app_menu); pack_end (share_app_menu); From b55cdd5869d1ba26b8391615902378b4af0f8e9b Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sat, 28 Jan 2023 19:34:16 +0000 Subject: [PATCH 08/39] Use SaveReason --- src/Services/Document.vala | 162 ++++++++----------- src/Services/DocumentManager.vala | 248 +++++++++++++++++------------- 2 files changed, 201 insertions(+), 209 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 0547d034c1..eeecfa3bb2 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -106,9 +106,10 @@ namespace Scratch.Services { public bool is_saving { get; private set; } - private Scratch.Services.SymbolOutline? outline = null; + public Scratch.Services.SymbolOutline? outline { get; private set; default = null; } private string original_content; // For restoring to original - private string last_save_content; // For detecting unsaved content + //TODO Do we need this AND buffer.get_modified ()? + public string last_save_content; // For detecting unsaved content private bool completion_shown = false; private bool loaded = false; @@ -198,28 +199,14 @@ namespace Scratch.Services { this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); - // toggle_changed_handlers (true); - - //TODO Reimplement this with DocumentManager - // // Focus out event for SourceView - // this.source_view.focus_out_event.connect (() => { - // if (Scratch.settings.get_boolean ("autosave")) { - // save.begin (); - // } - - // return false; - // }); + // Focus out event for SourceView + this.source_view.focus_out_event.connect (() => { + return DocumentManager.get_instance ().save_request (this, SaveReason.FOCUS_OUT); + }); source_view.buffer.changed.connect (() => { if (source_view.buffer.text != last_save_content) { - set_saved_status (false); // For visibility while developing - DocumentManager.get_instance ().save_request (this, false); - // saved = false; - // if (!Scratch.settings.get_boolean ("autosave")) { - // set_saved_status (false); - // } - } else { - set_saved_status (true); + DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); } }); @@ -404,7 +391,10 @@ namespace Scratch.Services { return true; } - if (DocumentManager.get_instance ().save_request (this, true)) { + if (DocumentManager.get_instance ().save_request ( + this, + app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING + )) { delete_backup (); doc_closed (); return true; @@ -474,17 +464,7 @@ namespace Scratch.Services { // } public bool save () { - // public bool save_with_hold () { - return DocumentManager.get_instance ().save_request (this, true); - // public bool save_with_hold (bool force = false) { - // GLib.Application.get_default ().hold (); - // bool result = false; - // save.begin (force, (obj, res) => { - // result = save.end (res); - // GLib.Application.get_default ().release (); - // }); - - // return result; + return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } public bool save_as () { @@ -492,7 +472,7 @@ namespace Scratch.Services { if (new_uri != null) { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, true)) { + if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { file = GLib.File.new_for_uri (old_uri); return false; } else { @@ -501,22 +481,7 @@ namespace Scratch.Services { } else { return false; } - // var new_path = get_save_as_uri (); - // if (new_path = "") { - // return false; - // } - - // DocumentManager.get_instance ().save_request (this, true); } - // GLib.Application.get_default ().hold (); - // bool result = false; - // save_as.begin ((obj, res) => { - // result = save_as.end (res); - // GLib.Application.get_default ().release (); - // }); - - // return result; - // } public string get_save_as_uri () { // Get new path to save to from user @@ -585,7 +550,7 @@ namespace Scratch.Services { public bool move (File new_dest) { this.file = new_dest; - return DocumentManager.get_instance ().save_request (this, true); + return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } private void restore_settings () { @@ -779,7 +744,7 @@ namespace Scratch.Services { if (new_uri != null) { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, true)) { + if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { file = GLib.File.new_for_uri (old_uri); } } @@ -791,7 +756,7 @@ namespace Scratch.Services { set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { hide_info_bar (); - DocumentManager.get_instance ().save_request (this, true); + DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); }); } @@ -813,7 +778,7 @@ namespace Scratch.Services { if (new_uri != null) { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, true)) { + if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { file = GLib.File.new_for_uri (old_uri); } } @@ -826,50 +791,51 @@ namespace Scratch.Services { this.source_view.editable = true; } - // Detect external changes - if (loaded) { - var new_buffer = new Gtk.SourceBuffer (null); - var source_file_loader = new Gtk.SourceFileLoader (new_buffer, source_file); - source_file_loader.load_async.begin (GLib.Priority.DEFAULT, null, null, (obj, res) => { - try { - source_file_loader.load_async.end (res); - } catch (Error e) { - critical (e.message); - show_default_load_error_view (); - return; - } + //TODO Fix this so that it does not trigger due to internal changes + // // Detect external changes + // if (loaded) { + // var new_buffer = new Gtk.SourceBuffer (null); + // var source_file_loader = new Gtk.SourceFileLoader (new_buffer, source_file); + // source_file_loader.load_async.begin (GLib.Priority.DEFAULT, null, null, (obj, res) => { + // try { + // source_file_loader.load_async.end (res); + // } catch (Error e) { + // critical (e.message); + // show_default_load_error_view (); + // return; + // } - if (source_view.buffer.text == new_buffer.text) { - return; - } + // if (source_view.buffer.text == new_buffer.text) { + // return; + // } - // "is_modified" returns false if no internal edits made since last time - // the app saved the content. - if (!source_view.buffer.get_modified ()) { - // The source_view.buffer has not been modified in app - //TODO Needs explanation? - if (Scratch.settings.get_boolean ("autosave")) { - // Not sure why we do not confirm in this case. - source_view.set_text (new_buffer.text, false); - } else { - string message = _( - "File \"%s\" was modified by an external application. Do you want to load it again or continue your editing?" - ).printf ("%s".printf (get_basename ())); - - set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { - this.source_view.set_text (new_buffer.text, false); - hide_info_bar (); - }, _("Continue"), () => { - hide_info_bar (); - }); - } - } else { - critical ("Possibly conflicting external and in app edits"); - //TODO Handle this case? - // Currently assume that the in app edits take priority over any external edits. - } - }); - } + // // "is_modified" returns false if no internal edits made since last time + // // the app saved the content. + // if (!source_view.buffer.get_modified ()) { + // // The source_view.buffer has not been modified in app + // //TODO Needs explanation? + // if (Scratch.settings.get_boolean ("autosave")) { + // // Not sure why we do not confirm in this case. + // source_view.set_text (new_buffer.text, false); + // } else { + // string message = _( + // "File \"%s\" was modified by an external application. Do you want to load it again or continue your editing?" + // ).printf ("%s".printf (get_basename ())); + + // set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { + // this.source_view.set_text (new_buffer.text, false); + // hide_info_bar (); + // }, _("Continue"), () => { + // hide_info_bar (); + // }); + // } + // } else { + // critical ("Possibly conflicting external and in app edits"); + // //TODO Handle this case? + // // Currently assume that the in app edits take priority over any external edits. + // } + // }); + // } } // Set Undo/Redo action sensitive property @@ -882,18 +848,18 @@ namespace Scratch.Services { ); } + // Used by SearchBar when search/replacing public void before_undoable_change () { source_view.set_editable (false); } public void after_undoable_change () { - source_view.buffer.set_modified (false); source_view.set_editable (true); if (outline != null) { outline.parse_symbols (); } } - + // Set saved status public void set_saved_status (bool val) { // this.saved = val; @@ -912,8 +878,6 @@ namespace Scratch.Services { } } - - private void delete_backup (string? backup_path = null) { string backup_file; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index d7d992fcce..d4e9e4bbbc 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -18,7 +18,21 @@ * Authored by: Jeremy Wootten */ - public class Scratch.Services.DocumentManager : Object { +public enum Scratch.SaveReason { + USER_REQUEST, + TAB_CLOSING, + APP_CLOSING, + AUTOSAVE, + FOCUS_OUT +} +public enum Scratch.SaveStatus { + SAVED, + UNSAVED, + SAVING, + SAVE_ERROR +} + +public class Scratch.Services.DocumentManager : Object { static Gee.HashMultiMap project_restorable_docs_map; static Gee.HashMultiMap project_open_docs_map; static Gee.HashMap doc_timeout_map; @@ -82,80 +96,130 @@ // @force is "true" when tab or app is closing or when user activated "action-save" // Returns "false" if operation cancelled by user - public bool save_request (Document doc, bool force) { + public bool save_request (Document doc, Scratch.SaveReason reason) { if (doc.inhibit_saving) { //TODO Confirm with user whether to proceed? return true; } - var autosave = Scratch.settings.get_boolean ("autosave"); - // Always save on closing. Otherwise only if autosave active. - if (!(force || autosave)) { - // Comes here when changed + var buffer_modified = doc.source_view.buffer.get_modified (); + var autosave_on = Scratch.settings.get_boolean ("auto-save"); + if (reason == SaveReason.AUTOSAVE) { + if (autosave_on) { + if (!doc_timeout_map.has_key (doc)) { + doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { + if (doc.delay_saving || doc.is_saving) { + return Source.CONTINUE; + } + warning ("autosave doc %s", doc.file.get_path ()); + start_to_save (doc, reason); + doc_timeout_map.unset (doc); + return Source.REMOVE; + }); + } + } else { + remove_autosave_for_doc (doc); + } + return true; - } + } - if (force) { - if (doc_timeout_map.has_key (doc)) { - Source.remove (doc_timeout_map[doc]); - doc_timeout_map.unset (doc); - } - if (doc.source_view.buffer.get_modified ()) { - bool save_changes; - if (!query_save_changes (doc, out save_changes)) { - return false; - } + remove_autosave_for_doc (doc); - if (!save_changes) { - delete_if_temporary (doc); - return true; + bool confirm, closing; + switch (reason) { + case USER_REQUEST: + case AUTOSAVE: + case FOCUS_OUT: + confirm = false; + closing = false; + break; + case TAB_CLOSING: + case APP_CLOSING: + if (!doc.is_file_temporary) { + confirm = !autosave_on; + } else { + //Always give opportunity to save as permanent file + confirm = true; } + + closing = true; + break; + default: + assert_not_reached (); + } + + if (confirm) { + bool save_changes; + if (!query_save_changes (doc, out save_changes)) { + // User cancelled operation + return false; } - save_doc (doc, true); - } else if (!doc_timeout_map.has_key (doc)) { - // Autosaving - need to create new timeout - doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { - if (doc.delay_saving || doc.is_saving) { - return Source.CONTINUE; - } - warning ("autosave doc %s", doc.file.get_path ()); - save_doc (doc, false); - doc_timeout_map.unset (doc); - return Source.REMOVE; - }); + if (!save_changes && doc.is_file_temporary) { + //User chose to discard the temporary file rather than save + delete_doc_file (doc); + return true; + } } + start_to_save (doc, reason); + //Saving was successfully started (but may yet fail asynchronously) return true; } - // This must only be called when the save is expected to succeed - // e.g. during autosave. - private void save_doc (Document doc, bool with_hold) { - var save_buffer = new Gtk.SourceBuffer (null); - var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); - save_buffer.text = source_buffer.text; + private void remove_autosave_for_doc (Document doc) { + if (doc_timeout_map.has_key (doc)) { + Source.remove (doc_timeout_map[doc]); + doc_timeout_map.unset (doc); + } + } + + private void start_to_save (Document doc, SaveReason reason) { + //Assume buffer was editable if a save request was generated doc.before_undoable_change (); + if (reason != SaveReason.AUTOSAVE && + reason != SaveReason.FOCUS_OUT && + Scratch.settings.get_boolean ("strip-trailing-on-save")) { - if (Scratch.settings.get_boolean ("strip-trailing-on-save") && - doc.source_view.language != null) { - Gtk.TextIter iter; - var cursor_pos = source_buffer.cursor_position; - source_buffer.get_iter_at_offset (out iter, cursor_pos); - var orig_line = iter.get_line (); - var orig_offset = iter.get_line_offset (); - strip_trailing_spaces_before_save (save_buffer); - source_buffer.begin_not_undoable_action (); - source_buffer.text = save_buffer.text; - source_buffer.get_iter_at_line_offset ( - out iter, - orig_line, - orig_offset - ); - source_buffer.place_cursor (iter); - source_buffer.end_not_undoable_action (); + strip_trailing_spaces_before_save (doc); } + // Saving to the location given in the doc source file will be attempted + save_doc.begin (doc, reason, (obj, res) => { + try { + if (save_doc.end (res)) { + doc.set_saved_status (true); + doc.source_view.buffer.set_modified (false); + doc.last_save_content = doc.source_view.buffer.text; + + if (doc.outline != null) { + doc.outline.parse_symbols (); + } + debug ("File \"%s\" saved successfully", doc.get_basename ()); + } + } catch (Error e) { + if (e.code != 19) { // Not cancelled + //TODO Inform user of failure + critical ( + "Cannot save \"%s\": %s", + doc.get_basename (), + e.message + ); + doc.set_saved_status (false); + } + } finally { + doc.after_undoable_change (); + } + }); + } + // This must only be called when the save is expected to succeed + // It is expected that the document buffer will not change during this process + // Any stripping or other automatic change has already taken place + private async bool save_doc (Document doc, SaveReason reason) throws Error { + var save_buffer = new Gtk.SourceBuffer (null); + var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); + save_buffer.text = source_buffer.text; create_doc_backup (doc); // Replace old content with the new one //TODO Handle cancellables internally @@ -166,54 +230,22 @@ doc.source_file ); - if (with_hold) { + if (reason == SaveReason.APP_CLOSING) { GLib.Application.get_default ().hold (); } - source_file_saver.save_async.begin ( + + var success = yield source_file_saver.save_async ( GLib.Priority.DEFAULT, doc.save_cancellable, - null, - (obj, res) => { - try { - if (source_file_saver.save_async.end (res)) { - doc.set_saved_status (true); - debug ("File \"%s\" saved successfully", doc.get_basename ()); - } - - doc.after_undoable_change (); - } catch (Error e) { - if (e.code != 19) { // Not cancelled - critical ( - "Cannot save \"%s\": %s", - doc.get_basename (), - e.message - ); - } - } finally { - if (with_hold) { - GLib.Application.get_default ().release (); - } - } - } + null ); - } - - // // Backup functions - // private void create_backup () { - // if (!can_write ()) { - // return; - // } - // var backup = File.new_for_path (this.file.get_path () + "~"); - - // if (!backup.query_exists ()) { - // try { - // file.copy (backup, FileCopyFlags.NONE); - // } catch (Error e) { - // warning ("Cannot create backup copy for file \"%s\": %s", get_basename (), e.message); - // } - // } - // } + if (reason == SaveReason.APP_CLOSING) { + GLib.Application.get_default ().release (); + } + + return success; + } private void create_doc_backup (Document doc) { if (!doc.can_write ()) { @@ -230,14 +262,15 @@ } } - private void strip_trailing_spaces_before_save (Gtk.SourceBuffer save_buffer) { - var text = save_buffer.text; + private void strip_trailing_spaces_before_save (Document doc) { + var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); + var text = source_buffer.text; string[] lines = Regex.split_simple ("""[\r\n]""", text); if (lines.length == 0) { // Can legitimately happen at startup or new document return; } - if (lines.length != save_buffer.get_line_count ()) { + if (lines.length != source_buffer.get_line_count ()) { critical ("Stripping: Mismatch between line counts, not continuing"); return; } @@ -255,23 +288,16 @@ for (int line_no = 0; line_no < lines.length; line_no++) { if (whitespace.match (lines[line_no], 0, out info)) { - save_buffer.get_iter_at_line (out start_delete, line_no); + source_buffer.get_iter_at_line (out start_delete, line_no); start_delete.forward_to_line_end (); end_delete = start_delete; end_delete.backward_chars (info.fetch (0).length); - - - save_buffer.@delete (ref start_delete, ref end_delete); - + source_buffer.@delete (ref start_delete, ref end_delete); } } } - private bool delete_if_temporary (Document doc) { - if (!doc.is_file_temporary) { - return false; - } - + private bool delete_doc_file (Document doc) { try { doc.file.delete (); return true; @@ -315,6 +341,8 @@ save_changes = false; close_document = true; break; + default: + assert_not_reached (); } dialog.destroy (); From 87373d27adfe21c5b02bcb79c86ddb92d3bb8672 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 13:12:11 +0000 Subject: [PATCH 09/39] Fix autosave delay; disable focus_out temporarily --- src/Services/Document.vala | 20 +++++++++----------- src/Services/DocumentManager.vala | 30 +++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index eeecfa3bb2..b783c28ab9 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -92,15 +92,11 @@ namespace Scratch.Services { public bool saved = true; - public bool delay_saving { - get { - return completion_shown || is_saving; - } - } + public bool delay_autosaving { get; set; } public bool inhibit_saving { get { - return !can_write () || !loaded; + return !can_write () || !loaded || completion_shown; } } @@ -199,11 +195,13 @@ namespace Scratch.Services { this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); - // Focus out event for SourceView - this.source_view.focus_out_event.connect (() => { - return DocumentManager.get_instance ().save_request (this, SaveReason.FOCUS_OUT); - }); - + // // Focus out event for SourceView + // this.source_view.focus_out_event.connect (() => { + // warning ("focus out"); + // // DocumentManager.get_instance ().save_request (this, SaveReason.FOCUS_OUT); + // return false; + // }); + source_view.buffer.changed.connect (() => { if (source_view.buffer.text != last_save_content) { DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index d4e9e4bbbc..f270eb9ff0 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -97,25 +97,29 @@ public class Scratch.Services.DocumentManager : Object { // @force is "true" when tab or app is closing or when user activated "action-save" // Returns "false" if operation cancelled by user public bool save_request (Document doc, Scratch.SaveReason reason) { +warning ("save request reason %s", reason.to_string ()); if (doc.inhibit_saving) { //TODO Confirm with user whether to proceed? return true; } var buffer_modified = doc.source_view.buffer.get_modified (); - var autosave_on = Scratch.settings.get_boolean ("auto-save"); + var autosave_on = Scratch.settings.get_boolean ("autosave"); if (reason == SaveReason.AUTOSAVE) { if (autosave_on) { if (!doc_timeout_map.has_key (doc)) { doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { - if (doc.delay_saving || doc.is_saving) { + if (doc.delay_autosaving || doc.is_saving) { + doc.delay_autosaving = false; return Source.CONTINUE; } - warning ("autosave doc %s", doc.file.get_path ()); + start_to_save (doc, reason); doc_timeout_map.unset (doc); return Source.REMOVE; }); + } else { + doc.delay_autosaving = true; } } else { remove_autosave_for_doc (doc); @@ -136,14 +140,17 @@ public class Scratch.Services.DocumentManager : Object { break; case TAB_CLOSING: case APP_CLOSING: - if (!doc.is_file_temporary) { - confirm = !autosave_on; + if (buffer_modified) { + if (!doc.is_file_temporary) { + confirm = !autosave_on; + } else { + //Always give opportunity to save as permanent file + confirm = true; + } } else { - //Always give opportunity to save as permanent file - confirm = true; + confirm = false; } - closing = true; break; default: assert_not_reached (); @@ -176,6 +183,7 @@ public class Scratch.Services.DocumentManager : Object { } private void start_to_save (Document doc, SaveReason reason) { +warning ("start to save"); //Assume buffer was editable if a save request was generated doc.before_undoable_change (); if (reason != SaveReason.AUTOSAVE && @@ -217,6 +225,7 @@ public class Scratch.Services.DocumentManager : Object { // It is expected that the document buffer will not change during this process // Any stripping or other automatic change has already taken place private async bool save_doc (Document doc, SaveReason reason) throws Error { +warning ("save doc"); var save_buffer = new Gtk.SourceBuffer (null); var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); save_buffer.text = source_buffer.text; @@ -248,6 +257,7 @@ public class Scratch.Services.DocumentManager : Object { } private void create_doc_backup (Document doc) { +warning ("create doc backup"); if (!doc.can_write ()) { return; } @@ -263,6 +273,7 @@ public class Scratch.Services.DocumentManager : Object { } private void strip_trailing_spaces_before_save (Document doc) { +warning ("strup "); var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); var text = source_buffer.text; string[] lines = Regex.split_simple ("""[\r\n]""", text); @@ -307,8 +318,9 @@ public class Scratch.Services.DocumentManager : Object { return false; } - + private bool query_save_changes (Document doc, out bool save_changes) { +warning ("query save"); var parent_window = doc.source_view.get_toplevel () as Gtk.Window; var dialog = new Granite.MessageDialog ( _("Save changes to \"%s\" before closing?").printf (doc.get_basename ()), From 90acad9460d253bf06f7197a89f6d9364c3d8b25 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 14:45:59 +0000 Subject: [PATCH 10/39] Rely on contents changed; inline single use function --- src/Services/Document.vala | 18 ++++++---------- src/Services/DocumentManager.vala | 36 +++++++++++++------------------ 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index b783c28ab9..db7010692e 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -203,9 +203,9 @@ namespace Scratch.Services { // }); source_view.buffer.changed.connect (() => { - if (source_view.buffer.text != last_save_content) { + // if (source_view.buffer.text != last_save_content) { DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); - } + // } }); source_view.completion.show.connect (() => { @@ -363,7 +363,7 @@ namespace Scratch.Services { source_view.buffer.set_modified (false); original_content = source_view.buffer.text; last_save_content = original_content; - set_saved_status (true); + set_saved_status (); doc_opened (); source_view.sensitive = true; @@ -859,20 +859,16 @@ namespace Scratch.Services { } // Set saved status - public void set_saved_status (bool val) { + public void set_saved_status () { // this.saved = val; string unsaved_identifier = "* "; - if (!val) { + var contents_changed = last_save_content != source_view.buffer.text; + if (contents_changed) { if (!(unsaved_identifier in this.label)) { tab_name = unsaved_identifier + this.label; } } else { - if (!source_view.buffer.get_modified ()) { - tab_name = this.label.replace (unsaved_identifier, ""); - last_save_content = source_view.buffer.text; - } else { - critical ("Buffer contents changed during save operation"); - } + tab_name = this.label.replace (unsaved_identifier, ""); } } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index f270eb9ff0..d2b834cb84 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -97,16 +97,14 @@ public class Scratch.Services.DocumentManager : Object { // @force is "true" when tab or app is closing or when user activated "action-save" // Returns "false" if operation cancelled by user public bool save_request (Document doc, Scratch.SaveReason reason) { -warning ("save request reason %s", reason.to_string ()); if (doc.inhibit_saving) { - //TODO Confirm with user whether to proceed? return true; } - var buffer_modified = doc.source_view.buffer.get_modified (); + var content_changed = doc.source_view.buffer.text != doc.last_save_content; var autosave_on = Scratch.settings.get_boolean ("autosave"); if (reason == SaveReason.AUTOSAVE) { - if (autosave_on) { + if (autosave_on && content_changed) { if (!doc_timeout_map.has_key (doc)) { doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { if (doc.delay_autosaving || doc.is_saving) { @@ -125,6 +123,7 @@ warning ("save request reason %s", reason.to_string ()); remove_autosave_for_doc (doc); } + doc.set_saved_status (); return true; } @@ -133,14 +132,14 @@ warning ("save request reason %s", reason.to_string ()); bool confirm, closing; switch (reason) { case USER_REQUEST: - case AUTOSAVE: + // case AUTOSAVE: case FOCUS_OUT: confirm = false; closing = false; break; case TAB_CLOSING: case APP_CLOSING: - if (buffer_modified) { + if (content_changed) { if (!doc.is_file_temporary) { confirm = !autosave_on; } else { @@ -156,7 +155,7 @@ warning ("save request reason %s", reason.to_string ()); assert_not_reached (); } - if (confirm) { + if (content_changed && confirm) { bool save_changes; if (!query_save_changes (doc, out save_changes)) { // User cancelled operation @@ -165,7 +164,13 @@ warning ("save request reason %s", reason.to_string ()); if (!save_changes && doc.is_file_temporary) { //User chose to discard the temporary file rather than save - delete_doc_file (doc); + try { + doc.file.delete (); + } catch (Error e) { + //TODO Inform user in UI? + warning ("Cannot delete temporary file \"%s\": %s", doc.file.get_uri (), e.message); + } + return true; } } @@ -197,7 +202,7 @@ warning ("start to save"); save_doc.begin (doc, reason, (obj, res) => { try { if (save_doc.end (res)) { - doc.set_saved_status (true); + doc.set_saved_status (); doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; @@ -214,7 +219,7 @@ warning ("start to save"); doc.get_basename (), e.message ); - doc.set_saved_status (false); + doc.set_saved_status (); } } finally { doc.after_undoable_change (); @@ -308,17 +313,6 @@ warning ("strup "); } } - private bool delete_doc_file (Document doc) { - try { - doc.file.delete (); - return true; - } catch (Error e) { - warning ("Cannot delete temporary file \"%s\": %s", doc.file.get_uri (), e.message); - } - - return false; - } - private bool query_save_changes (Document doc, out bool save_changes) { warning ("query save"); var parent_window = doc.source_view.get_toplevel () as Gtk.Window; From d72e80fa6f0f2527fff0ef8805f21821c67e5259 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 15:46:24 +0000 Subject: [PATCH 11/39] Fix reverting --- src/Services/Document.vala | 150 +++++------------------------- src/Services/DocumentManager.vala | 3 +- src/Widgets/HeaderBar.vala | 2 +- 3 files changed, 23 insertions(+), 132 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index db7010692e..945340c723 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -103,9 +103,9 @@ namespace Scratch.Services { public bool is_saving { get; private set; } public Scratch.Services.SymbolOutline? outline { get; private set; default = null; } - private string original_content; // For restoring to original + private string original_content = ""; // For restoring to original //TODO Do we need this AND buffer.get_modified ()? - public string last_save_content; // For detecting unsaved content + public string last_save_content = ""; // For detecting unsaved content private bool completion_shown = false; private bool loaded = false; @@ -200,12 +200,18 @@ namespace Scratch.Services { // warning ("focus out"); // // DocumentManager.get_instance ().save_request (this, SaveReason.FOCUS_OUT); // return false; - // }); - - source_view.buffer.changed.connect (() => { - // if (source_view.buffer.text != last_save_content) { + // }) + set_saved_status (); + check_undoable_actions (); + source_view.buffer.modified_changed.connect ((buffer) => { + warning ("modified changed"); + // This signal triggers even when modified is not changed + if (buffer.get_modified ()) { DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); - // } + } + + set_saved_status (); + check_undoable_actions (); }); source_view.completion.show.connect (() => { @@ -221,38 +227,6 @@ namespace Scratch.Services { ellipsize_mode = Pango.EllipsizeMode.MIDDLE; } - // public void toggle_changed_handlers (bool enabled) { - // if (enabled && onchange_handler_id == 0) { - // onchange_handler_id = this.source_view.buffer.changed.connect (() => { - // if (onchange_handler_id != 0) { - // this.source_view.buffer.disconnect (onchange_handler_id); - // } - - // // Signals for SourceView - // uint timeout_saving = 0; - // check_undoable_actions (); - // onchange_handler_id = source_view.buffer.changed.connect (() => { - // check_undoable_actions (); - // // Save if autosave is ON - // if (Scratch.settings.get_boolean ("autosave")) { - // if (timeout_saving > 0) { - // Source.remove (timeout_saving); - // timeout_saving = 0; - // } - // timeout_saving = Timeout.add (1000, () => { - // save.begin (); - // timeout_saving = 0; - // return false; - // }); - // } - // }); - // }); - // } else if (!enabled && onchange_handler_id != 0) { - // this.source_view.buffer.disconnect (onchange_handler_id); - // onchange_handler_id = 0; - // } - // } - private uint load_timout_id = 0; public async void open (bool force = false) { /* Loading improper files may hang so we cancel after a certain time as a fallback. @@ -364,6 +338,7 @@ namespace Scratch.Services { original_content = source_view.buffer.text; last_save_content = original_content; set_saved_status (); + check_undoable_actions (); doc_opened (); source_view.sensitive = true; @@ -400,66 +375,6 @@ namespace Scratch.Services { return false; } - // bool needs_save = source_view.buffer.modified || - // (app_closing && is_file_temporary && !delete_temporary_file ()) - // bool ret_value = true; - // if (source_view.buffer.text.modified) { - // // if (Scratch.settings.get_boolean ("autosave") && !saved) { - // save_with_hold (); - // } else if (app_closing && - // is_file_temporary && - // !delete_temporary_file ()) { - // // Save temp files with temp uri - // debug ("Save temporary file!"); - // save_with_hold (); - // } - // // Check for unsaved changes - // else if (!this.saved || (!app_closing && is_file_temporary && !delete_temporary_file ())) { - // var parent_window = source_view.get_toplevel () as Gtk.Window; - - // var dialog = new Granite.MessageDialog ( - // _("Save changes to \"%s\" before closing?").printf (this.get_basename ()), - // _("If you don't save, changes will be permanently lost."), - // new ThemedIcon ("dialog-warning"), - // Gtk.ButtonsType.NONE - // ); - // dialog.transient_for = parent_window; - - // var no_save_button = (Gtk.Button) dialog.add_button (_("Close Without Saving"), Gtk.ResponseType.NO); - // no_save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION); - - // dialog.add_button (_("Cancel"), Gtk.ResponseType.CANCEL); - // dialog.add_button (_("Save"), Gtk.ResponseType.YES); - // dialog.set_default_response (Gtk.ResponseType.YES); - - // int response = dialog.run (); - // switch (response) { - // case Gtk.ResponseType.CANCEL: - // case Gtk.ResponseType.DELETE_EVENT: - // ret_value = false; - // break; - // case Gtk.ResponseType.YES: - // if (this.is_file_temporary) - // save_as_with_hold (); - // else - // save_with_hold (); - // break; - // case Gtk.ResponseType.NO: - // if (this.is_file_temporary) - // delete_temporary_file (true); - // break; - // } - // dialog.destroy (); - // } - - // if (ret_value) { - // // Delete backup copy file - // delete_backup (); - // doc_closed (); - // } - - // return ret_value; - // } public bool save () { return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); @@ -510,8 +425,6 @@ namespace Scratch.Services { ); var new_path = ""; - // var current_file = file.get_path (); - // var is_current_file_temporary = this.is_file_temporary; if (file_chooser.run () == Gtk.ResponseType.ACCEPT) { file = File.new_for_uri (file_chooser.get_uri ()); // Update last visited path @@ -522,29 +435,6 @@ namespace Scratch.Services { file_chooser.destroy (); return new_path; } - // if (success) { - // source_view.buffer.set_modified (true); - // var is_saved = yield save (true); - // if (is_saved && is_current_file_temporary) { - // try { - // // Delete temporary file - // File.new_for_path (current_file).delete (); - // } catch (Error err) { - // warning ("Temporary file cannot be deleted: %s", current_file); - // } - // } - - // delete_backup (current_file + "~"); - // this.source_view.change_syntax_highlight_from_file (this.file); - // } - - // /* We delay destruction of file chooser dialog til now to avoid - // * the document focussing in, - // * which triggers premature loading of overwritten content. - // */ - - // return success; - // } public bool move (File new_dest) { this.file = new_dest; @@ -663,19 +553,19 @@ namespace Scratch.Services { // Undo public void undo () { this.source_view.undo (); - check_undoable_actions (); } // Redo public void redo () { this.source_view.redo (); - check_undoable_actions (); } // Revert public void revert () { this.source_view.set_text (original_content, false); + source_view.buffer.set_modified (false); check_undoable_actions (); + set_saved_status (); } // Get text @@ -842,8 +732,11 @@ namespace Scratch.Services { Utils.action_from_group (MainWindow.ACTION_UNDO, actions).set_enabled (source_buffer.can_undo); Utils.action_from_group (MainWindow.ACTION_REDO, actions).set_enabled (source_buffer.can_redo); Utils.action_from_group (MainWindow.ACTION_REVERT, actions).set_enabled ( - original_content != source_buffer.text + //This reverts to original loaded content, not to last saved content! + //TODO Warn user if this would overwrite saved content? + source_view.buffer.text != original_content ); + warning ("revertable %s", (source_view.buffer.text != original_content).to_string ()); } // Used by SearchBar when search/replacing @@ -862,8 +755,7 @@ namespace Scratch.Services { public void set_saved_status () { // this.saved = val; string unsaved_identifier = "* "; - var contents_changed = last_save_content != source_view.buffer.text; - if (contents_changed) { + if (source_view.buffer.get_modified ()) { if (!(unsaved_identifier in this.label)) { tab_name = unsaved_identifier + this.label; } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index d2b834cb84..5130c9f520 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -202,7 +202,6 @@ warning ("start to save"); save_doc.begin (doc, reason, (obj, res) => { try { if (save_doc.end (res)) { - doc.set_saved_status (); doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; @@ -219,10 +218,10 @@ warning ("start to save"); doc.get_basename (), e.message ); - doc.set_saved_status (); } } finally { doc.after_undoable_change (); + doc.set_saved_status (); } }); } diff --git a/src/Widgets/HeaderBar.vala b/src/Widgets/HeaderBar.vala index 80da9fd22e..d173b0dadd 100644 --- a/src/Widgets/HeaderBar.vala +++ b/src/Widgets/HeaderBar.vala @@ -49,7 +49,7 @@ public class Scratch.HeaderBar : Hdy.HeaderBar { app_instance.get_accels_for_action (save_button.action_name), _("Save this file") ); - + var save_as_button = new Gtk.Button.from_icon_name ("document-save-as", Gtk.IconSize.LARGE_TOOLBAR) { action_name = MainWindow.ACTION_PREFIX + MainWindow.ACTION_SAVE_AS }; From 682785154a7baec10604e7edc7ad478725623434 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 16:03:39 +0000 Subject: [PATCH 12/39] Fix parse symbols after save --- src/Services/Document.vala | 6 ++++-- src/Services/DocumentManager.vala | 15 +++++++-------- src/Widgets/HeaderBar.vala | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 945340c723..e44fd1667e 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -365,7 +365,7 @@ namespace Scratch.Services { } if (DocumentManager.get_instance ().save_request ( - this, + this, app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING )) { delete_backup (); @@ -745,12 +745,14 @@ namespace Scratch.Services { } public void after_undoable_change () { +warning ("after undoable change"); source_view.set_editable (true); + set_saved_status (); if (outline != null) { outline.parse_symbols (); } } - + // Set saved status public void set_saved_status () { // this.saved = val; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 5130c9f520..90861282bf 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -125,7 +125,7 @@ public class Scratch.Services.DocumentManager : Object { doc.set_saved_status (); return true; - } + } remove_autosave_for_doc (doc); @@ -149,7 +149,7 @@ public class Scratch.Services.DocumentManager : Object { } else { confirm = false; } - + break; default: assert_not_reached (); @@ -204,10 +204,10 @@ warning ("start to save"); if (save_doc.end (res)) { doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; - - if (doc.outline != null) { - doc.outline.parse_symbols (); - } + + // if (doc.outline != null) { + // doc.outline.parse_symbols (); + // } debug ("File \"%s\" saved successfully", doc.get_basename ()); } } catch (Error e) { @@ -221,7 +221,6 @@ warning ("start to save"); } } finally { doc.after_undoable_change (); - doc.set_saved_status (); } }); } @@ -256,7 +255,7 @@ warning ("save doc"); if (reason == SaveReason.APP_CLOSING) { GLib.Application.get_default ().release (); } - + return success; } diff --git a/src/Widgets/HeaderBar.vala b/src/Widgets/HeaderBar.vala index d173b0dadd..80da9fd22e 100644 --- a/src/Widgets/HeaderBar.vala +++ b/src/Widgets/HeaderBar.vala @@ -49,7 +49,7 @@ public class Scratch.HeaderBar : Hdy.HeaderBar { app_instance.get_accels_for_action (save_button.action_name), _("Save this file") ); - + var save_as_button = new Gtk.Button.from_icon_name ("document-save-as", Gtk.IconSize.LARGE_TOOLBAR) { action_name = MainWindow.ACTION_PREFIX + MainWindow.ACTION_SAVE_AS }; From d137467bf9fc16c8df300ce9114918beb7ea32c9 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 16:38:30 +0000 Subject: [PATCH 13/39] Fix regression in autosave --- src/Services/Document.vala | 17 +++++----------- src/Services/DocumentManager.vala | 32 ++++++++++++------------------- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index e44fd1667e..eb8a207bc3 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -204,16 +204,13 @@ namespace Scratch.Services { set_saved_status (); check_undoable_actions (); source_view.buffer.modified_changed.connect ((buffer) => { - warning ("modified changed"); - // This signal triggers even when modified is not changed - if (buffer.get_modified ()) { - DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); - } - set_saved_status (); check_undoable_actions (); }); + source_view.buffer.changed.connect ((buffer) => { + DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); + }); source_view.completion.show.connect (() => { completion_shown = true; }); @@ -736,16 +733,14 @@ namespace Scratch.Services { //TODO Warn user if this would overwrite saved content? source_view.buffer.text != original_content ); - warning ("revertable %s", (source_view.buffer.text != original_content).to_string ()); } - // Used by SearchBar when search/replacing + // Two functoins Used by SearchBar when search/replacing as well as + // DocumentManager while saving. public void before_undoable_change () { source_view.set_editable (false); } - public void after_undoable_change () { -warning ("after undoable change"); source_view.set_editable (true); set_saved_status (); if (outline != null) { @@ -790,8 +785,6 @@ warning ("after undoable change"); } } - - // Return true if the file is writable. Keep testing as may change public bool can_write () { FileInfo info; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 90861282bf..115bc3d771 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -101,10 +101,9 @@ public class Scratch.Services.DocumentManager : Object { return true; } - var content_changed = doc.source_view.buffer.text != doc.last_save_content; var autosave_on = Scratch.settings.get_boolean ("autosave"); if (reason == SaveReason.AUTOSAVE) { - if (autosave_on && content_changed) { + if (autosave_on) { if (!doc_timeout_map.has_key (doc)) { doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { if (doc.delay_autosaving || doc.is_saving) { @@ -119,6 +118,8 @@ public class Scratch.Services.DocumentManager : Object { } else { doc.delay_autosaving = true; } + // Do not set saved status when autosave is on + return true; } else { remove_autosave_for_doc (doc); } @@ -139,15 +140,11 @@ public class Scratch.Services.DocumentManager : Object { break; case TAB_CLOSING: case APP_CLOSING: - if (content_changed) { - if (!doc.is_file_temporary) { - confirm = !autosave_on; - } else { - //Always give opportunity to save as permanent file - confirm = true; - } + if (!doc.is_file_temporary) { + confirm = !autosave_on; } else { - confirm = false; + //Always give opportunity to save as permanent file + confirm = true; } break; @@ -155,7 +152,10 @@ public class Scratch.Services.DocumentManager : Object { assert_not_reached (); } - if (content_changed && confirm) { + // Only ask user if there are some changes + if (confirm && + doc.source_view.buffer.text != doc.last_save_content) { + bool save_changes; if (!query_save_changes (doc, out save_changes)) { // User cancelled operation @@ -188,7 +188,6 @@ public class Scratch.Services.DocumentManager : Object { } private void start_to_save (Document doc, SaveReason reason) { -warning ("start to save"); //Assume buffer was editable if a save request was generated doc.before_undoable_change (); if (reason != SaveReason.AUTOSAVE && @@ -204,10 +203,6 @@ warning ("start to save"); if (save_doc.end (res)) { doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; - - // if (doc.outline != null) { - // doc.outline.parse_symbols (); - // } debug ("File \"%s\" saved successfully", doc.get_basename ()); } } catch (Error e) { @@ -221,6 +216,7 @@ warning ("start to save"); } } finally { doc.after_undoable_change (); + doc.set_saved_status (); } }); } @@ -228,7 +224,6 @@ warning ("start to save"); // It is expected that the document buffer will not change during this process // Any stripping or other automatic change has already taken place private async bool save_doc (Document doc, SaveReason reason) throws Error { -warning ("save doc"); var save_buffer = new Gtk.SourceBuffer (null); var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); save_buffer.text = source_buffer.text; @@ -260,7 +255,6 @@ warning ("save doc"); } private void create_doc_backup (Document doc) { -warning ("create doc backup"); if (!doc.can_write ()) { return; } @@ -276,7 +270,6 @@ warning ("create doc backup"); } private void strip_trailing_spaces_before_save (Document doc) { -warning ("strup "); var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); var text = source_buffer.text; string[] lines = Regex.split_simple ("""[\r\n]""", text); @@ -312,7 +305,6 @@ warning ("strup "); } private bool query_save_changes (Document doc, out bool save_changes) { -warning ("query save"); var parent_window = doc.source_view.get_toplevel () as Gtk.Window; var dialog = new Granite.MessageDialog ( _("Save changes to \"%s\" before closing?").printf (doc.get_basename ()), From 7ae2e82d22137e7a72d188c86190e51859b0fa91 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 17:08:47 +0000 Subject: [PATCH 14/39] Reduce time uneditable, show working while saving --- src/Services/Document.vala | 14 +++++++++----- src/Services/DocumentManager.vala | 11 +++++++---- src/Widgets/DocumentView.vala | 1 - 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index eb8a207bc3..b4d69f1a9c 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -172,10 +172,9 @@ namespace Scratch.Services { restore_settings (); settings.changed.connect (restore_settings); - /* Block user editing while working */ - source_view.key_press_event.connect (() => { - return working; - }); + + // Use `set_editable ()`(undoable actions) or `sensitive` (while loading) when we wish to inhibit editing + // This gives audible feedback var source_grid = new Gtk.Grid () { orientation = Gtk.Orientation.HORIZONTAL, @@ -209,7 +208,12 @@ namespace Scratch.Services { }); source_view.buffer.changed.connect ((buffer) => { - DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); + // May need to wait for completion to close which would otherwise inhibit + // saving + Idle.add (() => { + DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); + return Source.REMOVE; + }); }); source_view.completion.show.connect (() => { completion_shown = true; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 115bc3d771..c77f6c865e 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -36,7 +36,7 @@ public class Scratch.Services.DocumentManager : Object { static Gee.HashMultiMap project_restorable_docs_map; static Gee.HashMultiMap project_open_docs_map; static Gee.HashMap doc_timeout_map; - uint AUTOSAVE_RATE_MSEC = 1000; + const uint AUTOSAVE_RATE_MSEC = 1000; static DocumentManager? instance; public static DocumentManager get_instance () { @@ -153,7 +153,7 @@ public class Scratch.Services.DocumentManager : Object { } // Only ask user if there are some changes - if (confirm && + if (confirm && doc.source_view.buffer.text != doc.last_save_content) { bool save_changes; @@ -189,12 +189,14 @@ public class Scratch.Services.DocumentManager : Object { private void start_to_save (Document doc, SaveReason reason) { //Assume buffer was editable if a save request was generated - doc.before_undoable_change (); + doc.working = true; if (reason != SaveReason.AUTOSAVE && reason != SaveReason.FOCUS_OUT && Scratch.settings.get_boolean ("strip-trailing-on-save")) { + doc.before_undoable_change (); strip_trailing_spaces_before_save (doc); + doc.after_undoable_change (); } // Saving to the location given in the doc source file will be attempted @@ -215,11 +217,12 @@ public class Scratch.Services.DocumentManager : Object { ); } } finally { - doc.after_undoable_change (); doc.set_saved_status (); + doc.working = false; } }); } + // This must only be called when the save is expected to succeed // It is expected that the document buffer will not change during this process // Any stripping or other automatic change has already taken place diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index c101f911c1..1133f5b026 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -175,7 +175,6 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { private void insert_document (Scratch.Services.Document doc, int pos) { insert_tab (doc, pos); if (Scratch.saved_state.get_boolean ("outline-visible")) { - warning ("setting outline visible"); doc.show_outline (true); } } From 532f4fdb8395acbf2adbf50d433d1ad30161b283 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 18:17:11 +0000 Subject: [PATCH 15/39] Fix handling external changes --- src/Services/Document.vala | 160 ++++++++++------------------------ src/Widgets/DocumentView.vala | 2 +- 2 files changed, 48 insertions(+), 114 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index b4d69f1a9c..e65d43b301 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -4,6 +4,7 @@ Copyright (C) 2011-2012 Giulio Collura 2013 Mario Guerriero + 2023 elementary LLC. This program is free software: you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License version 3, as published by the Free Software Foundation. @@ -451,12 +452,6 @@ namespace Scratch.Services { source_map.no_show_all = true; scroll.vscrollbar_policy = Gtk.PolicyType.AUTOMATIC; } - - // Stripping spaces only happens before save - - // if (Scratch.settings.get_boolean ("strip-trailing-on-save")) { - // strip_trailing_spaces (); - // } } // Focus the SourceView @@ -618,7 +613,7 @@ namespace Scratch.Services { // Check if the file was deleted/changed by an external source // Only called on focus in - public void check_file_status () { + private void check_file_status () { // If the file does not exist anymore if (!exists ()) { if (mounted == false) { @@ -680,51 +675,51 @@ namespace Scratch.Services { this.source_view.editable = true; } - //TODO Fix this so that it does not trigger due to internal changes - // // Detect external changes - // if (loaded) { - // var new_buffer = new Gtk.SourceBuffer (null); - // var source_file_loader = new Gtk.SourceFileLoader (new_buffer, source_file); - // source_file_loader.load_async.begin (GLib.Priority.DEFAULT, null, null, (obj, res) => { - // try { - // source_file_loader.load_async.end (res); - // } catch (Error e) { - // critical (e.message); - // show_default_load_error_view (); - // return; - // } - - // if (source_view.buffer.text == new_buffer.text) { - // return; - // } - - // // "is_modified" returns false if no internal edits made since last time - // // the app saved the content. - // if (!source_view.buffer.get_modified ()) { - // // The source_view.buffer has not been modified in app - // //TODO Needs explanation? - // if (Scratch.settings.get_boolean ("autosave")) { - // // Not sure why we do not confirm in this case. - // source_view.set_text (new_buffer.text, false); - // } else { - // string message = _( - // "File \"%s\" was modified by an external application. Do you want to load it again or continue your editing?" - // ).printf ("%s".printf (get_basename ())); - - // set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { - // this.source_view.set_text (new_buffer.text, false); - // hide_info_bar (); - // }, _("Continue"), () => { - // hide_info_bar (); - // }); - // } - // } else { - // critical ("Possibly conflicting external and in app edits"); - // //TODO Handle this case? - // // Currently assume that the in app edits take priority over any external edits. - // } - // }); - // } + // Detect external changes + if (loaded) { + var new_buffer = new Gtk.SourceBuffer (null); + var source_file_loader = new Gtk.SourceFileLoader (new_buffer, source_file); + source_file_loader.load_async.begin (GLib.Priority.DEFAULT, null, null, (obj, res) => { + try { + source_file_loader.load_async.end (res); + } catch (Error e) { + critical (e.message); + show_default_load_error_view (); + return; + } + + if (source_view.buffer.text == new_buffer.text) { + return; + } + + //TODO Give opportunity to rename document in case of conflict + string message = _( +"File \"%s\" was modified by an external application.\n Do you want to load it again and lose your changes or continue editing and overwrite external changes if you save this document?" + ).printf ("%s".printf (get_basename ())); + if (!source_view.buffer.get_modified ()) { + set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { + source_view.set_text (new_buffer.text, false); + last_save_content = new_buffer.text; + // Should already be in "saved" state + hide_info_bar (); + }, _("Continue"), () => { + hide_info_bar (); + }); + } else { + set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { + source_view.set_text (new_buffer.text, false); + // Put in "saved" state + last_save_content = new_buffer.text; + source_view.buffer.set_modified (false); + check_undoable_actions (); + set_saved_status (); + hide_info_bar (); + }, _("Continue"), () => { + hide_info_bar (); + }); + } + }); + } } // Set Undo/Redo action sensitive property @@ -873,65 +868,4 @@ namespace Scratch.Services { text.scroll_to_iter (iter, 0.0, true, 0.5, 0.5); } } - - // /* Pull the buffer into an array and then work out which parts are to be deleted. - // * Do not strip line currently being edited unless forced */ - // private void strip_trailing_spaces () { - // if (!loaded || source_view.language == null) { - // return; - // } - - // stripping = true; - // var source_buffer = (Gtk.SourceBuffer)source_view.buffer; - // Gtk.TextIter iter; - - // var cursor_pos = source_buffer.cursor_position; - // source_buffer.get_iter_at_offset (out iter, cursor_pos); - // var orig_line = iter.get_line (); - // var orig_offset = iter.get_line_offset (); - - // var text = source_buffer.text; - - // string[] lines = Regex.split_simple ("""[\r\n]""", text); - // if (lines.length == 0) { // Can legitimately happen at startup or new document - // return; - // } - - // if (lines.length != source_buffer.get_line_count ()) { - // critical ("Mismatch between line counts when stripping trailing spaces, not continuing"); - // debug ("lines.length %u, buffer lines %u \n %s", lines.length, source_buffer.get_line_count (), text); - // return; - // } - - // MatchInfo info; - // Gtk.TextIter start_delete, end_delete; - // Regex whitespace; - - // try { - // whitespace = new Regex ("[ \t]+$", 0); - // } catch (RegexError e) { - // critical ("Error while building regex to replace trailing whitespace: %s", e.message); - // return; - // } - - // for (int line_no = 0; line_no < lines.length; line_no++) { - // if (whitespace.match (lines[line_no], 0, out info)) { - - // source_buffer.get_iter_at_line (out start_delete, line_no); - // start_delete.forward_to_line_end (); - // end_delete = start_delete; - // end_delete.backward_chars (info.fetch (0).length); - - // source_buffer.begin_not_undoable_action (); - // source_buffer.@delete (ref start_delete, ref end_delete); - // source_buffer.end_not_undoable_action (); - // } - // } - - // source_buffer.get_iter_at_line_offset (out iter, orig_line, orig_offset); - // source_buffer.place_cursor (iter); - // } - - // stripping = false; - // } } diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index 1133f5b026..e5e86fa5f4 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -2,7 +2,7 @@ /*** BEGIN LICENSE - Copyright (C) 2013 Mario Guerriero + Copyright (C) 2023 elementary LLC. This program is free software: you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License version 3, as published by the Free Software Foundation. From 6177b59c5300f3b65d87b89377fe195b71856670 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 19:03:37 +0000 Subject: [PATCH 16/39] Revert "Cancel closing project folder if cannot close a document" This reverts commit 9fef13bbe29e4fbb154b52eea0cf0d037704e341. # Conflicts fixed: # src/Widgets/DocumentView.vala --- src/FolderManager/FileView.vala | 45 +++++++++--------------- src/FolderManager/ProjectFolderItem.vala | 4 +++ src/MainWindow.vala | 8 +---- src/Services/Document.vala | 1 - src/Services/GitManager.vala | 1 + 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/src/FolderManager/FileView.vala b/src/FolderManager/FileView.vala index 233b3c258f..2e3ae4a33c 100644 --- a/src/FolderManager/FileView.vala +++ b/src/FolderManager/FileView.vala @@ -173,10 +173,9 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P public ProjectFolderItem? get_project_for_file (GLib.File file) { foreach (var item in root.children) { if (item is ProjectFolderItem) { - var folder_file = ((ProjectFolderItem)item).file.file; - if (folder_file.equal (file) || - folder_file.get_relative_path (file) != null) { - return (ProjectFolderItem)item; + var folder = (ProjectFolderItem)item; + if (folder.contains_file (file)) { + return folder; } } } @@ -263,10 +262,16 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P folder_root.expanded = expand; folder_root.closed.connect (() => { - toplevel_action_group.activate_action ( - MainWindow.ACTION_CLOSE_PROJECT_DOCS, - new Variant.string (folder_root.path) - ); + toplevel_action_group.activate_action (MainWindow.ACTION_CLOSE_PROJECT_DOCS, new Variant.string (folder_root.path)); + root.remove (folder_root); + foreach (var child in root.children) { + var child_folder = (ProjectFolderItem) child; + if (child_folder.name != child_folder.file.name) { + rename_items_with_same_name (child_folder); + } + } + Scratch.Services.GitManager.get_instance ().remove_project (folder_root); + write_settings (); }); folder_root.close_all_except.connect (() => { @@ -274,30 +279,14 @@ public class Scratch.FolderManager.FileView : Granite.Widgets.SourceList, Code.P var project_folder_item = (ProjectFolderItem)child; if (project_folder_item != folder_root) { toplevel_action_group.activate_action (MainWindow.ACTION_CLOSE_PROJECT_DOCS, new Variant.string (project_folder_item.path)); + root.remove (project_folder_item); + Scratch.Services.GitManager.get_instance ().remove_project (project_folder_item); } } - }); - - write_settings (); - } - // Callback for MainWindow.ACTION_CLOSE_PROJECT_DOCS when not cancelled - public void remove_project_for_path (string path) { - var folder_root = get_project_for_file (GLib.File.new_for_path (path)); - if (folder_root == null) { - critical ("Could not find project for path %s", path); - return; - } - - root.remove (folder_root); - foreach (var child in root.children) { - var child_folder = (ProjectFolderItem) child; - if (child_folder.name != child_folder.file.name) { - rename_items_with_same_name (child_folder); - } - } + write_settings (); + }); - Scratch.Services.GitManager.get_instance ().remove_project (folder_root); write_settings (); } diff --git a/src/FolderManager/ProjectFolderItem.vala b/src/FolderManager/ProjectFolderItem.vala index 438fb9f771..cc4dca33cc 100644 --- a/src/FolderManager/ProjectFolderItem.vala +++ b/src/FolderManager/ProjectFolderItem.vala @@ -264,6 +264,10 @@ namespace Scratch.FolderManager { }); } + public bool contains_file (GLib.File descendant) { + return file.file.get_relative_path (descendant) != null; + } + private void deprioritize_git_ignored () requires (monitored_repo != null) { visible_item_list.@foreach ((visible_item) => { var item = visible_item.item; diff --git a/src/MainWindow.vala b/src/MainWindow.vala index dee4c26f5e..da6cbb6b44 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -949,22 +949,16 @@ namespace Scratch { unowned var docs = document_view.docs; docs.foreach ((doc) => { if (doc.file.get_path ().has_prefix (project_path)) { - if (!(document_view.close_document (doc))) { - return; - } - + document_view.close_document (doc); if (make_restorable) { document_manager.make_restorable (doc); } } }); - if (!make_restorable) { document_manager.remove_project (project_path); } - - folder_manager_view.remove_project_for_path (project_path); } private void restore_project_docs (string project_path) { diff --git a/src/Services/Document.vala b/src/Services/Document.vala index e65d43b301..c0ff13c5f9 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -357,7 +357,6 @@ namespace Scratch.Services { return; } - // Returns "false" only if user cancelled operation public bool do_close (bool app_closing = false) { debug ("Closing \"%s\"", get_basename ()); diff --git a/src/Services/GitManager.vala b/src/Services/GitManager.vala index 04fd6f30c8..891fdc1a3f 100644 --- a/src/Services/GitManager.vala +++ b/src/Services/GitManager.vala @@ -82,6 +82,7 @@ namespace Scratch.Services { public void remove_project (FolderManager.ProjectFolderItem root_folder) { var root_path = root_folder.file.file.get_path (); + uint position; if (project_liststore.find (root_folder, out position)) { project_liststore.remove (position); From 30bd7adbdaa56da653a49ff41c97b1818a0c08d8 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Sun, 29 Jan 2023 19:07:37 +0000 Subject: [PATCH 17/39] Revert "Provide branch build from as Constant.BRANCH; show in header" This reverts commit 9c12dec4092e9978e255e5ce45d3ef8439ebae9b. # Conflicts fixed: # src/Widgets/HeaderBar.vala --- src/Widgets/HeaderBar.vala | 15 --------------- src/config.vala.in | 1 - src/meson.build | 3 --- 3 files changed, 19 deletions(-) diff --git a/src/Widgets/HeaderBar.vala b/src/Widgets/HeaderBar.vala index 80da9fd22e..780fd8bf1f 100644 --- a/src/Widgets/HeaderBar.vala +++ b/src/Widgets/HeaderBar.vala @@ -250,21 +250,6 @@ public class Scratch.HeaderBar : Hdy.HeaderBar { pack_start (save_button); pack_start (save_as_button); pack_start (revert_button); - - if (Constants.BRANCH != null && - Constants.BRANCH != "" && - Constants.BRANCH != "master" && - Constants.BRANCH != "main") { - - //TODO Decide on best place to expose this information - //Putting in headerbar for immediate visibility while developing - var branch_label = new Gtk.Label (Constants.BRANCH) { - tooltip_text = _("Branch of source code currently running") - }; - - pack_start (branch_label); - } - pack_end (app_menu); pack_end (share_app_menu); diff --git a/src/config.vala.in b/src/config.vala.in index 28b87ae2d6..6f6e2b4077 100644 --- a/src/config.vala.in +++ b/src/config.vala.in @@ -6,5 +6,4 @@ namespace Constants { public const string INSTALL_PREFIX = @PREFIX@; public const string DATADIR = @DATADIR@; public const string LOCALEDIR = @LOCALEDIR@; - public const string BRANCH = @BRANCH@; } diff --git a/src/meson.build b/src/meson.build index f62e01d116..2535e6fd96 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,5 +1,3 @@ -output = run_command('git','branch','--show-current') -branch = output.stdout().strip() conf_data = configuration_data() conf_data.set_quoted('PROJECT_NAME', meson.project_name()) conf_data.set_quoted('GETTEXT_PACKAGE', meson.project_name()) @@ -8,7 +6,6 @@ conf_data.set_quoted('PREFIX', get_option('prefix')) conf_data.set_quoted('PLUGINDIR', pluginsdir) conf_data.set_quoted('DATADIR', join_paths (get_option('prefix'), get_option('datadir'))) conf_data.set_quoted('LOCALEDIR', join_paths (get_option('prefix'), get_option('localedir'))) -conf_data.set_quoted('BRANCH', branch) config_header = configure_file( input : 'config.vala.in', output : 'config.vala', From 3740e7fcf7a050bd251c26ada33a68295c552483 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 10:09:15 +0000 Subject: [PATCH 18/39] Partial fix focus out --- src/Services/Document.vala | 59 +++++++++++++++---------------- src/Services/DocumentManager.vala | 7 ++-- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index c0ff13c5f9..5f5579399a 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -195,12 +195,6 @@ namespace Scratch.Services { this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); - // // Focus out event for SourceView - // this.source_view.focus_out_event.connect (() => { - // warning ("focus out"); - // // DocumentManager.get_instance ().save_request (this, SaveReason.FOCUS_OUT); - // return false; - // }) set_saved_status (); check_undoable_actions (); source_view.buffer.modified_changed.connect ((buffer) => { @@ -326,13 +320,14 @@ namespace Scratch.Services { } // Focus in event for SourceView - this.source_view.focus_in_event.connect (() => { + focus_in_event.connect (() => { +warning ("focus in event"); check_file_status (); check_undoable_actions (); return false; }); - + // Change syntax highlight this.source_view.change_syntax_highlight_from_file (this.file); @@ -687,36 +682,40 @@ namespace Scratch.Services { return; } - if (source_view.buffer.text == new_buffer.text) { + if (last_save_content == new_buffer.text) { return; } - //TODO Give opportunity to rename document in case of conflict - string message = _( -"File \"%s\" was modified by an external application.\n Do you want to load it again and lose your changes or continue editing and overwrite external changes if you save this document?" - ).printf ("%s".printf (get_basename ())); + // In case of conflict, either discard current changes and load external changes + // or continue. If continuing, the user can later rename this document to keep + // external changes or overwrite them by saving with the same name. + string message; if (!source_view.buffer.get_modified ()) { - set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { + message = _( +"File \"%s\" was modified by an external application.\n Do you want to load the external changes or continue and overwrite the external changes if you save this document?" + ).printf ("%s".printf (get_basename ())); + } else { + message = _( +"File \"%s\" was modified by an external application while you were also making changes.\n Do you want to load the external changes and lose your changes or continue and overwrite the external changes if you save this document?" + ).printf ("%s".printf (get_basename ())); + } + + set_message (Gtk.MessageType.WARNING, message, + _("Load"), + () => { source_view.set_text (new_buffer.text, false); + // Put in "saved" state last_save_content = new_buffer.text; - // Should already be in "saved" state + source_view.buffer.set_modified (false); + check_undoable_actions (); + set_saved_status (); hide_info_bar (); - }, _("Continue"), () => { + }, + _("Continue"), + () => { hide_info_bar (); - }); - } else { - set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { - source_view.set_text (new_buffer.text, false); - // Put in "saved" state - last_save_content = new_buffer.text; - source_view.buffer.set_modified (false); - check_undoable_actions (); - set_saved_status (); - hide_info_bar (); - }, _("Continue"), () => { - hide_info_bar (); - }); - } + } + ); }); } } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index c77f6c865e..7f1b781cfe 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -22,8 +22,7 @@ public enum Scratch.SaveReason { USER_REQUEST, TAB_CLOSING, APP_CLOSING, - AUTOSAVE, - FOCUS_OUT + AUTOSAVE } public enum Scratch.SaveStatus { SAVED, @@ -133,11 +132,10 @@ public class Scratch.Services.DocumentManager : Object { bool confirm, closing; switch (reason) { case USER_REQUEST: - // case AUTOSAVE: - case FOCUS_OUT: confirm = false; closing = false; break; + case TAB_CLOSING: case APP_CLOSING: if (!doc.is_file_temporary) { @@ -191,7 +189,6 @@ public class Scratch.Services.DocumentManager : Object { //Assume buffer was editable if a save request was generated doc.working = true; if (reason != SaveReason.AUTOSAVE && - reason != SaveReason.FOCUS_OUT && Scratch.settings.get_boolean ("strip-trailing-on-save")) { doc.before_undoable_change (); From 728666c3590cd6197968d51414abbfd915cdb7fe Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 10:20:12 +0000 Subject: [PATCH 19/39] Ignore focus_in_event after on hide_info_bar () --- src/Services/Document.vala | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 5f5579399a..3a6ede5294 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -320,14 +320,8 @@ namespace Scratch.Services { } // Focus in event for SourceView - focus_in_event.connect (() => { -warning ("focus in event"); - check_file_status (); - check_undoable_actions (); + focus_in_event.connect (on_focus_in); - return false; - }); - // Change syntax highlight this.source_view.change_syntax_highlight_from_file (this.file); @@ -535,8 +529,13 @@ warning ("focus in event"); // Hide InfoBar when not needed public void hide_info_bar () { + source_view.focus_in_event.disconnect (on_focus_in); info_bar.no_show_all = true; info_bar.visible = false; + Idle.add (() => { + source_view.focus_in_event.connect (on_focus_in); + return Source.REMOVE; + }); } // SourceView related functions @@ -605,6 +604,11 @@ warning ("focus in event"); main_stack.set_visible_child (alert_view); } + private bool on_focus_in () { + check_file_status (); + check_undoable_actions (); + return false; + } // Check if the file was deleted/changed by an external source // Only called on focus in private void check_file_status () { @@ -699,9 +703,9 @@ warning ("focus in event"); "File \"%s\" was modified by an external application while you were also making changes.\n Do you want to load the external changes and lose your changes or continue and overwrite the external changes if you save this document?" ).printf ("%s".printf (get_basename ())); } - + set_message (Gtk.MessageType.WARNING, message, - _("Load"), + _("Load"), () => { source_view.set_text (new_buffer.text, false); // Put in "saved" state @@ -711,7 +715,7 @@ warning ("focus in event"); set_saved_status (); hide_info_bar (); }, - _("Continue"), + _("Continue"), () => { hide_info_bar (); } @@ -732,7 +736,7 @@ warning ("focus in event"); ); } - // Two functoins Used by SearchBar when search/replacing as well as + // Two functoins Used by SearchBar when search/replacing as well as // DocumentManager while saving. public void before_undoable_change () { source_view.set_editable (false); From 3f08adc557fdf60ce77e39dc8f43e98d72975083 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 11:10:06 +0000 Subject: [PATCH 20/39] Small cleanups, comments, avoid possible duplicate signal connection --- src/Services/Document.vala | 21 ++++++++++----------- src/Services/DocumentManager.vala | 1 + 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 3a6ede5294..d27f6e13bd 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -174,9 +174,6 @@ namespace Scratch.Services { settings.changed.connect (restore_settings); - // Use `set_editable ()`(undoable actions) or `sensitive` (while loading) when we wish to inhibit editing - // This gives audible feedback - var source_grid = new Gtk.Grid () { orientation = Gtk.Orientation.HORIZONTAL, column_homogeneous = false @@ -203,8 +200,8 @@ namespace Scratch.Services { }); source_view.buffer.changed.connect ((buffer) => { - // May need to wait for completion to close which would otherwise inhibit - // saving + // May need to wait for completion to close + // which would otherwise inhibit saving Idle.add (() => { DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); return Source.REMOVE; @@ -226,7 +223,10 @@ namespace Scratch.Services { private uint load_timout_id = 0; public async void open (bool force = false) { /* Loading improper files may hang so we cancel after a certain time as a fallback. - * In most cases, an error will be thrown and caught. */ + * In most cases, an error will be thrown and caught. */ + if (loaded) { + focus_in_event.disconnect (on_focus_in); + } loaded = false; if (load_cancellable != null) { /* just in case */ load_cancellable.cancel (); @@ -731,13 +731,13 @@ namespace Scratch.Services { Utils.action_from_group (MainWindow.ACTION_REDO, actions).set_enabled (source_buffer.can_redo); Utils.action_from_group (MainWindow.ACTION_REVERT, actions).set_enabled ( //This reverts to original loaded content, not to last saved content! - //TODO Warn user if this would overwrite saved content? source_view.buffer.text != original_content ); } - // Two functoins Used by SearchBar when search/replacing as well as - // DocumentManager while saving. + // Two functions Used by SearchBar when search/replacing as well as + // DocumentManager while saving in order to prevent user changing the + // the document during critical operations, and to update things after. public void before_undoable_change () { source_view.set_editable (false); } @@ -749,9 +749,8 @@ namespace Scratch.Services { } } - // Set saved status + // Show whether there are unsaved changes in the tab label public void set_saved_status () { - // this.saved = val; string unsaved_identifier = "* "; if (source_view.buffer.get_modified ()) { if (!(unsaved_identifier in this.label)) { diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 7f1b781cfe..e3651428e4 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -173,6 +173,7 @@ public class Scratch.Services.DocumentManager : Object { } } + // Save even when no changes as may need to overwrite external changes start_to_save (doc, reason); //Saving was successfully started (but may yet fail asynchronously) return true; From df39f2c0d3711244dbd822b290a9d4d42468b154 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 11:35:21 +0000 Subject: [PATCH 21/39] DRY check_file_status, inline create_backup --- src/Services/Document.vala | 31 ++++++++++----------------- src/Services/DocumentManager.vala | 35 ++++++++++++++++--------------- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index d27f6e13bd..191a3a2360 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -620,15 +620,8 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save As…"), () => { - // this.save_as.begin (); - hide_info_bar (); - var new_uri = get_save_as_uri (); - if (new_uri != null) { - var old_uri = file.get_uri (); - file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { - file = GLib.File.new_for_uri (old_uri); - } + if (save_as ()) { + hide_info_bar (); } }); } else { @@ -637,8 +630,11 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { - hide_info_bar (); - DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); + if (save ()) { + hide_info_bar (); + } else { + //TODO Provide feedback on failure + } }); } @@ -654,15 +650,10 @@ namespace Scratch.Services { ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save changes elsewhere"), () => { - hide_info_bar (); - //TODO DRY this block - var new_uri = get_save_as_uri (); - if (new_uri != null) { - var old_uri = file.get_uri (); - file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { - file = GLib.File.new_for_uri (old_uri); - } + if (save_as ()) { + hide_info_bar (); + } else { + //TODO Provide feedback on failure } }); diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index e3651428e4..ad5f2cf286 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -221,14 +221,28 @@ public class Scratch.Services.DocumentManager : Object { }); } - // This must only be called when the save is expected to succeed + // This is only called once but is split out for clarity // It is expected that the document buffer will not change during this process // Any stripping or other automatic change has already taken place private async bool save_doc (Document doc, SaveReason reason) throws Error { var save_buffer = new Gtk.SourceBuffer (null); var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); save_buffer.text = source_buffer.text; - create_doc_backup (doc); + + var backup = File.new_for_path (doc.file.get_path () + "~"); + if (!backup.query_exists ()) { + try { + doc.file.copy (backup, FileCopyFlags.NONE); + } catch (Error e) { + warning ( + "Cannot create backup copy for file \"%s\": %s", + doc.get_basename (), + e.message + ); + //Should we return fail now? The actual save will probably fail too + } + } + // Replace old content with the new one //TODO Handle cancellables internally doc.save_cancellable.cancel (); @@ -255,21 +269,7 @@ public class Scratch.Services.DocumentManager : Object { return success; } - private void create_doc_backup (Document doc) { - if (!doc.can_write ()) { - return; - } - - var backup = File.new_for_path (doc.file.get_path () + "~"); - if (!backup.query_exists ()) { - try { - doc.file.copy (backup, FileCopyFlags.NONE); - } catch (Error e) { - warning ("Cannot create backup copy for file \"%s\": %s", doc.get_basename (), e.message); - } - } - } - + // This is only called once but is split out for clarity private void strip_trailing_spaces_before_save (Document doc) { var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); var text = source_buffer.text; @@ -305,6 +305,7 @@ public class Scratch.Services.DocumentManager : Object { } } + // This is only called once but is split out for clarity private bool query_save_changes (Document doc, out bool save_changes) { var parent_window = doc.source_view.get_toplevel () as Gtk.Window; var dialog = new Granite.MessageDialog ( From aeaa9dfe8751c607e62a44609c3bd185b24cb497 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 11:48:15 +0000 Subject: [PATCH 22/39] Move delete_backup to document manager --- src/Services/Document.vala | 26 +------------------------- src/Services/DocumentManager.vala | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 191a3a2360..3bfc0011cc 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -358,7 +358,7 @@ namespace Scratch.Services { this, app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING )) { - delete_backup (); + // DocumentManager will delete any backup doc_closed (); return true; } @@ -752,30 +752,6 @@ namespace Scratch.Services { } } - private void delete_backup (string? backup_path = null) { - string backup_file; - - if (backup_path == null) { - backup_file = file.get_path () + "~"; - } else { - backup_file = backup_path; - } - - debug ("Backup file deleting: %s", backup_file); - var backup = File.new_for_path (backup_file); - if (backup == null || !backup.query_exists ()) { - debug ("Backup file doesn't exists: %s", backup.get_path ()); - return; - } - - try { - backup.delete (); - debug ("Backup file deleted: %s", backup_file); - } catch (Error e) { - warning ("Cannot delete backup for file \"%s\": %s", get_basename (), e.message); - } - } - // Return true if the file is writable. Keep testing as may change public bool can_write () { FileInfo info; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index ad5f2cf286..172f219b68 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -204,6 +204,24 @@ public class Scratch.Services.DocumentManager : Object { doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; debug ("File \"%s\" saved successfully", doc.get_basename ()); + if (reason == SaveReason.APP_CLOSING || + reason == SaveReason.TAB_CLOSING) { + // Delete Backup + var backup_file_path = doc.file.get_path () + "~"; + debug ("Backup file deleting: %s", backup_file_path); + var backup = File.new_for_path (backup_file_path); + if (backup == null || !backup.query_exists ()) { + critical ("Backup file doesn't exists: %s", backup_file_path); + return; + } + + try { + backup.delete (); + debug ("Backup file deleted: %s", backup_file_path); + } catch (Error e) { + critical ("Cannot delete backup \"%s\": %s", backup_file_path, e.message); + } + } } } catch (Error e) { if (e.code != 19) { // Not cancelled From d6f9aaddaf9c51ca5fe35d59dd21e9cb178ca0c2 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 11:59:28 +0000 Subject: [PATCH 23/39] Silence startup warnings --- src/Services/Document.vala | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 3bfc0011cc..0a4606331e 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -192,8 +192,6 @@ namespace Scratch.Services { this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); - set_saved_status (); - check_undoable_actions (); source_view.buffer.modified_changed.connect ((buffer) => { set_saved_status (); check_undoable_actions (); From fcf943b812103fd2029d45eac95ba1b015f7e1e2 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 12:11:08 +0000 Subject: [PATCH 24/39] Do not call after_undoable_changes if closing; fix terminal errors --- src/Services/DocumentManager.vala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 172f219b68..a359d788a2 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -189,12 +189,18 @@ public class Scratch.Services.DocumentManager : Object { private void start_to_save (Document doc, SaveReason reason) { //Assume buffer was editable if a save request was generated doc.working = true; + var closing = reason == SaveReason.APP_CLOSING || + reason == SaveReason.TAB_CLOSING; + if (reason != SaveReason.AUTOSAVE && Scratch.settings.get_boolean ("strip-trailing-on-save")) { doc.before_undoable_change (); strip_trailing_spaces_before_save (doc); - doc.after_undoable_change (); + if (!closing) { + // Only call if not closing to avoid terminal errors + doc.after_undoable_change (); + } } // Saving to the location given in the doc source file will be attempted @@ -204,8 +210,7 @@ public class Scratch.Services.DocumentManager : Object { doc.source_view.buffer.set_modified (false); doc.last_save_content = doc.source_view.buffer.text; debug ("File \"%s\" saved successfully", doc.get_basename ()); - if (reason == SaveReason.APP_CLOSING || - reason == SaveReason.TAB_CLOSING) { + if (closing) { // Delete Backup var backup_file_path = doc.file.get_path () + "~"; debug ("Backup file deleting: %s", backup_file_path); From c8d022018989daee469bfeee3b43e1fb86687498 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 14:36:17 +0000 Subject: [PATCH 25/39] Fix cancel save as --- src/Services/Document.vala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 0a4606331e..c1f2fb9876 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -365,12 +365,14 @@ namespace Scratch.Services { } public bool save () { + warning ("action save"); return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } public bool save_as () { var new_uri = get_save_as_uri (); - if (new_uri != null) { + assert_nonnull (new_uri); + if (new_uri != "") { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { @@ -387,6 +389,7 @@ namespace Scratch.Services { public string get_save_as_uri () { // Get new path to save to from user if (!loaded) { + warning ("not loaded"); return ""; } @@ -452,6 +455,7 @@ namespace Scratch.Services { // Get file name public string get_basename () { + assert_nonnull (file); if (is_file_temporary) { return _("New Document"); } else { From 81d8a9820e3b1a98808d7ceb3dec45c8302fbd2d Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 14:57:28 +0000 Subject: [PATCH 26/39] Dont check file status while saving in progress, check after if fails --- src/Services/Document.vala | 17 +++++++++++------ src/Services/DocumentManager.vala | 6 ++++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index c1f2fb9876..856bce9681 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -365,7 +365,6 @@ namespace Scratch.Services { } public bool save () { - warning ("action save"); return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } @@ -379,6 +378,7 @@ namespace Scratch.Services { file = GLib.File.new_for_uri (old_uri); return false; } else { + // Save may not have completed as it happens async return true; } } else { @@ -389,7 +389,6 @@ namespace Scratch.Services { public string get_save_as_uri () { // Get new path to save to from user if (!loaded) { - warning ("not loaded"); return ""; } @@ -607,13 +606,18 @@ namespace Scratch.Services { } private bool on_focus_in () { - check_file_status (); - check_undoable_actions (); + // Ignore if saving underway. DocumentManager will perform same + // operations when finished. + if (!working) { + check_file_status (); + check_undoable_actions (); + } + return false; } // Check if the file was deleted/changed by an external source - // Only called on focus in - private void check_file_status () { + // Called on focus in and after failed saving + public void check_file_status () { // If the file does not exist anymore if (!exists ()) { if (mounted == false) { @@ -737,6 +741,7 @@ namespace Scratch.Services { public void after_undoable_change () { source_view.set_editable (true); set_saved_status (); + if (outline != null) { outline.parse_symbols (); } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index a359d788a2..05adc1ff75 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -230,16 +230,18 @@ public class Scratch.Services.DocumentManager : Object { } } catch (Error e) { if (e.code != 19) { // Not cancelled - //TODO Inform user of failure critical ( "Cannot save \"%s\": %s", doc.get_basename (), e.message ); + + // Inform user of failure + doc.check_file_status (); } } finally { - doc.set_saved_status (); doc.working = false; + doc.set_saved_status (); } }); } From 3d15e9ab881e3fd12c90f1d020f5cfdd09ea1882 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 15:36:55 +0000 Subject: [PATCH 27/39] Do not set new file before confirming writable --- src/Services/Document.vala | 28 ++++++++++++++++++---------- src/Services/DocumentManager.vala | 4 +++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 856bce9681..f5c2221b35 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -52,6 +52,7 @@ namespace Scratch.Services { } private set { + warning ("setting file to %s", value.get_path ()); source_file.set_location (value); source_view.location = value; file_changed (); @@ -97,12 +98,10 @@ namespace Scratch.Services { public bool inhibit_saving { get { - return !can_write () || !loaded || completion_shown; + return !loaded || completion_shown; } } - public bool is_saving { get; private set; } - public Scratch.Services.SymbolOutline? outline { get; private set; default = null; } private string original_content = ""; // For restoring to original //TODO Do we need this AND buffer.get_modified ()? @@ -365,10 +364,12 @@ namespace Scratch.Services { } public bool save () { +warning ("save"); return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } public bool save_as () { +warning ("save as"); var new_uri = get_save_as_uri (); assert_nonnull (new_uri); if (new_uri != "") { @@ -382,6 +383,7 @@ namespace Scratch.Services { return true; } } else { + warning ("new uri null ignore"); return false; } } @@ -391,7 +393,7 @@ namespace Scratch.Services { if (!loaded) { return ""; } - +warning ("get save as uri"); var all_files_filter = new Gtk.FileFilter (); all_files_filter.set_filter_name (_("All files")); all_files_filter.add_pattern ("*"); @@ -416,13 +418,19 @@ namespace Scratch.Services { var new_path = ""; if (file_chooser.run () == Gtk.ResponseType.ACCEPT) { - file = File.new_for_uri (file_chooser.get_uri ()); // Update last visited path new_path = file_chooser.get_file ().get_uri (); Utils.last_path = Path.get_dirname (new_path); } file_chooser.destroy (); + + //Check that the location is writable + var new_file = File.new_for_path (new_path); + if (!can_write (new_file)) { + new_path = ""; + } + return new_path; } @@ -760,11 +768,11 @@ namespace Scratch.Services { } // Return true if the file is writable. Keep testing as may change - public bool can_write () { + public bool can_write (GLib.File test_file = this.file) { FileInfo info; bool writable = false; try { - info = this.file.query_info ( + info = test_file.query_info ( FileAttribute.ACCESS_CAN_WRITE, FileQueryInfoFlags.NONE, null @@ -773,11 +781,11 @@ namespace Scratch.Services { FileAttribute.ACCESS_CAN_WRITE ); } catch (Error e) { - debug ( - "Error determining write access: %s. Allowing write", + critical ( + "Error determining write access: %s. Not allowing write", e.message ); - writable = true; + writable = false; } return writable; diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 05adc1ff75..a5ad6c7f50 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -105,7 +105,7 @@ public class Scratch.Services.DocumentManager : Object { if (autosave_on) { if (!doc_timeout_map.has_key (doc)) { doc_timeout_map[doc] = Timeout.add (AUTOSAVE_RATE_MSEC, () => { - if (doc.delay_autosaving || doc.is_saving) { + if (doc.delay_autosaving || doc.working) { doc.delay_autosaving = false; return Source.CONTINUE; } @@ -227,6 +227,8 @@ public class Scratch.Services.DocumentManager : Object { critical ("Cannot delete backup \"%s\": %s", backup_file_path, e.message); } } + } else { + critical ("saving failed without error thrown"); } } catch (Error e) { if (e.code != 19) { // Not cancelled From a7202abc54e7bc705e52c2cc7a7e045dfd89a2db Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 19:58:22 +0000 Subject: [PATCH 28/39] Propagate async saving up to MainWindow --- src/MainWindow.vala | 52 ++++++---- src/Services/Document.vala | 72 +++++++------- src/Services/DocumentManager.vala | 158 +++++++++++++++--------------- src/Widgets/DocumentView.vala | 47 +++++---- 4 files changed, 174 insertions(+), 155 deletions(-) diff --git a/src/MainWindow.vala b/src/MainWindow.vala index da6cbb6b44..0245ebdc76 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -595,7 +595,7 @@ namespace Scratch { protected override bool delete_event (Gdk.EventAny event) { handle_quit (); - return !check_unsaved_changes (); + return Gdk.EVENT_STOP; } // Set sensitive property for 'delicate' Widgets/GtkActions while @@ -643,18 +643,18 @@ namespace Scratch { document_view.close_document (doc); } - // Check if there no unsaved changes - private bool check_unsaved_changes () { - document_view.is_closing = true; - foreach (var doc in document_view.docs) { - if (!doc.do_close (true)) { - document_view.current_document = doc; - return false; - } - } + // // Check if there no unsaved changes + // private bool check_unsaved_changes () { + // document_view.is_closing = true; + // foreach (var doc in document_view.docs) { + // if (!doc.do_close (true)) { + // document_view.current_document = doc; + // return false; + // } + // } - return true; - } + // return true; + // } // Save session information different from window state private void restore_saved_state_extra () { @@ -708,8 +708,22 @@ namespace Scratch { // For exit cleanup private void handle_quit () { - document_view.save_opened_files (); - update_saved_state (); + save_all_documents.begin ((obj, res) => { + if (save_all_documents.end (res)) { + destroy (); + } + }); + } + + private async bool save_all_documents () { + unowned var docs = document_view.docs; + var success = true; + var docs_copy = docs.copy (); + foreach (var doc in docs_copy) { + success = success && yield doc.do_close (true); + } + + return success; } public void set_default_zoom () { @@ -798,9 +812,9 @@ namespace Scratch { private void action_quit () { handle_quit (); - if (check_unsaved_changes ()) { - destroy (); - } + // if (check_unsaved_changes ()) { + // destroy (); + // } } private void action_open () { @@ -872,7 +886,7 @@ namespace Scratch { if (doc.is_file_temporary == true) { action_save_as (); } else { - doc.save (); + doc.save.begin (); } } } @@ -880,7 +894,7 @@ namespace Scratch { private void action_save_as () { var doc = get_current_document (); if (doc != null) { - doc.save_as (); + doc.save_as.begin (); } } diff --git a/src/Services/Document.vala b/src/Services/Document.vala index f5c2221b35..1806fd1ac6 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -200,7 +200,9 @@ namespace Scratch.Services { // May need to wait for completion to close // which would otherwise inhibit saving Idle.add (() => { - DocumentManager.get_instance ().save_request (this, SaveReason.AUTOSAVE); + DocumentManager.get_instance ().save_request.begin ( + this, SaveReason.AUTOSAVE + ); return Source.REMOVE; }); }); @@ -343,7 +345,7 @@ namespace Scratch.Services { return; } - public bool do_close (bool app_closing = false) { + public async bool do_close (bool app_closing) { debug ("Closing \"%s\"", get_basename ()); if (!loaded) { @@ -351,7 +353,7 @@ namespace Scratch.Services { return true; } - if (DocumentManager.get_instance ().save_request ( + if (yield DocumentManager.get_instance ().save_request ( this, app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING )) { @@ -363,27 +365,27 @@ namespace Scratch.Services { return false; } - public bool save () { + public async bool save () { warning ("save"); - return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); + return yield DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } - public bool save_as () { -warning ("save as"); + public async bool save_as () { var new_uri = get_save_as_uri (); assert_nonnull (new_uri); if (new_uri != "") { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); - if (!DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST)) { + if (!(yield DocumentManager.get_instance ().save_request ( + this, SaveReason.USER_REQUEST))) { + // Revert to original location if save failed or cancelled file = GLib.File.new_for_uri (old_uri); return false; - } else { - // Save may not have completed as it happens async - return true; } + + return true; } else { - warning ("new uri null ignore"); + warning ("Save As: Failed to get new uri"); return false; } } @@ -393,7 +395,7 @@ warning ("save as"); if (!loaded) { return ""; } -warning ("get save as uri"); + var all_files_filter = new Gtk.FileFilter (); all_files_filter.set_filter_name (_("All files")); all_files_filter.add_pattern ("*"); @@ -425,18 +427,18 @@ warning ("get save as uri"); file_chooser.destroy (); - //Check that the location is writable - var new_file = File.new_for_path (new_path); - if (!can_write (new_file)) { - new_path = ""; - } + // //Check that the location is writable + // var new_file = File.new_for_path (new_path); + // if (!can_write (new_file)) { + // new_path = ""; + // } return new_path; } - public bool move (File new_dest) { + public async bool move (File new_dest) { this.file = new_dest; - return DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); + return yield DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); } private void restore_settings () { @@ -634,9 +636,11 @@ warning ("get save as uri"); ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save As…"), () => { - if (save_as ()) { - hide_info_bar (); - } + save_as.begin ((obj, res) => { + if (save_as.end (res)) { + hide_info_bar (); + } + }); }); } else { string message = _( @@ -644,11 +648,11 @@ warning ("get save as uri"); ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { - if (save ()) { - hide_info_bar (); - } else { - //TODO Provide feedback on failure - } + save.begin ((obj, res) => { + if (save.end (res)) { + hide_info_bar (); + } + }); }); } @@ -664,11 +668,11 @@ warning ("get save as uri"); ).printf ("%s".printf (get_basename ())); set_message (Gtk.MessageType.WARNING, message, _("Save changes elsewhere"), () => { - if (save_as ()) { - hide_info_bar (); - } else { - //TODO Provide feedback on failure - } + save_as.begin ((obj, res) => { + if (save_as.end (res)) { + hide_info_bar (); + } + }); }); Utils.action_from_group (MainWindow.ACTION_SAVE, actions).set_enabled (false); @@ -749,7 +753,7 @@ warning ("get save as uri"); public void after_undoable_change () { source_view.set_editable (true); set_saved_status (); - + if (outline != null) { outline.parse_symbols (); } diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index a5ad6c7f50..922049a6bc 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -95,7 +95,7 @@ public class Scratch.Services.DocumentManager : Object { // @force is "true" when tab or app is closing or when user activated "action-save" // Returns "false" if operation cancelled by user - public bool save_request (Document doc, Scratch.SaveReason reason) { + public async bool save_request (Document doc, Scratch.SaveReason reason) { if (doc.inhibit_saving) { return true; } @@ -110,7 +110,8 @@ public class Scratch.Services.DocumentManager : Object { return Source.CONTINUE; } - start_to_save (doc, reason); + // When autosaving should not need to handle errors? + start_to_save.begin (doc, reason); doc_timeout_map.unset (doc); return Source.REMOVE; }); @@ -174,9 +175,7 @@ public class Scratch.Services.DocumentManager : Object { } // Save even when no changes as may need to overwrite external changes - start_to_save (doc, reason); - //Saving was successfully started (but may yet fail asynchronously) - return true; + return yield start_to_save (doc, reason); } private void remove_autosave_for_doc (Document doc) { @@ -186,7 +185,7 @@ public class Scratch.Services.DocumentManager : Object { } } - private void start_to_save (Document doc, SaveReason reason) { + private async bool start_to_save (Document doc, SaveReason reason) { //Assume buffer was editable if a save request was generated doc.working = true; var closing = reason == SaveReason.APP_CLOSING || @@ -203,59 +202,7 @@ public class Scratch.Services.DocumentManager : Object { } } - // Saving to the location given in the doc source file will be attempted - save_doc.begin (doc, reason, (obj, res) => { - try { - if (save_doc.end (res)) { - doc.source_view.buffer.set_modified (false); - doc.last_save_content = doc.source_view.buffer.text; - debug ("File \"%s\" saved successfully", doc.get_basename ()); - if (closing) { - // Delete Backup - var backup_file_path = doc.file.get_path () + "~"; - debug ("Backup file deleting: %s", backup_file_path); - var backup = File.new_for_path (backup_file_path); - if (backup == null || !backup.query_exists ()) { - critical ("Backup file doesn't exists: %s", backup_file_path); - return; - } - - try { - backup.delete (); - debug ("Backup file deleted: %s", backup_file_path); - } catch (Error e) { - critical ("Cannot delete backup \"%s\": %s", backup_file_path, e.message); - } - } - } else { - critical ("saving failed without error thrown"); - } - } catch (Error e) { - if (e.code != 19) { // Not cancelled - critical ( - "Cannot save \"%s\": %s", - doc.get_basename (), - e.message - ); - - // Inform user of failure - doc.check_file_status (); - } - } finally { - doc.working = false; - doc.set_saved_status (); - } - }); - } - - // This is only called once but is split out for clarity - // It is expected that the document buffer will not change during this process - // Any stripping or other automatic change has already taken place - private async bool save_doc (Document doc, SaveReason reason) throws Error { - var save_buffer = new Gtk.SourceBuffer (null); - var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); - save_buffer.text = source_buffer.text; - + //Create backup file var backup = File.new_for_path (doc.file.get_path () + "~"); if (!backup.query_exists ()) { try { @@ -270,32 +217,89 @@ public class Scratch.Services.DocumentManager : Object { } } - // Replace old content with the new one - //TODO Handle cancellables internally - doc.save_cancellable.cancel (); - doc.save_cancellable = new GLib.Cancellable (); - var source_file_saver = new Gtk.SourceFileSaver ( - source_buffer, - doc.source_file - ); - + // Saving to the location given in the doc source file will be attempted + var is_saved = false; + var save_buffer = new Gtk.SourceBuffer (null); + var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); + save_buffer.text = source_buffer.text; + + // Replace old content with the new one + //TODO Handle cancellables internally + doc.save_cancellable.cancel (); + doc.save_cancellable = new GLib.Cancellable (); + var source_file_saver = new Gtk.SourceFileSaver ( + source_buffer, + doc.source_file + ); if (reason == SaveReason.APP_CLOSING) { - GLib.Application.get_default ().hold (); + GLib.Application.get_default ().hold (); } - var success = yield source_file_saver.save_async ( - GLib.Priority.DEFAULT, - doc.save_cancellable, - null - ); + try { - if (reason == SaveReason.APP_CLOSING) { - GLib.Application.get_default ().release (); + is_saved = yield source_file_saver.save_async ( + GLib.Priority.DEFAULT, + doc.save_cancellable, + null + ); + // is_saved = yield save_doc (doc, reason); + } catch (Error e){ + if (e.code != 19) { // Not cancelled + critical ( + "Cannot save \"%s\": %s", + doc.get_basename (), + e.message + ); + + // Inform user of failure + doc.check_file_status (); + } + } finally { + doc.working = false; + doc.set_saved_status (); + + if (reason == SaveReason.APP_CLOSING) { + GLib.Application.get_default ().release (); + } } - return success; + return is_saved; } + // This is only called once but is split out for clarity + // It is expected that the document buffer will not change during this process + // Any stripping or other automatic change has already taken place + // private async bool save_doc (Document doc, SaveReason reason) throws Error { + // var save_buffer = new Gtk.SourceBuffer (null); + // var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); + // save_buffer.text = source_buffer.text; + + // // Replace old content with the new one + // //TODO Handle cancellables internally + // doc.save_cancellable.cancel (); + // doc.save_cancellable = new GLib.Cancellable (); + // var source_file_saver = new Gtk.SourceFileSaver ( + // source_buffer, + // doc.source_file + // ); + + // if (reason == SaveReason.APP_CLOSING) { + // GLib.Application.get_default ().hold (); + // } + + // var success = yield source_file_saver.save_async ( + // GLib.Priority.DEFAULT, + // doc.save_cancellable, + // null + // ); + + // if (reason == SaveReason.APP_CLOSING) { + // GLib.Application.get_default ().release (); + // } + + // return success; + // } + // This is only called once but is split out for clarity private void strip_trailing_spaces_before_save (Document doc) { var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); diff --git a/src/Widgets/DocumentView.vala b/src/Widgets/DocumentView.vala index e5e86fa5f4..5e63122e7e 100644 --- a/src/Widgets/DocumentView.vala +++ b/src/Widgets/DocumentView.vala @@ -69,17 +69,22 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { }); close_tab_requested.connect ((tab) => { - var document = tab as Services.Document; - if (!document.is_file_temporary && document.file != null) { - tab.restore_data = document.get_uri (); - } + var doc = tab as Services.Document; + doc.do_close.begin (false, (obj, res) => { + if (doc.do_close.end (res)) { + if (!doc.is_file_temporary && doc.file != null) { + tab.restore_data = doc.get_uri (); + remove_tab (doc); + } + } + }); - return document.do_close (); + return true; }); tab_switched.connect ((old_tab, new_tab) => { /* The 'document_change' signal is emitted when the document is focused. We do not need to emit it here */ - save_focused_document_uri (new_tab as Services.Document); + remember_focused_document_uri (new_tab as Services.Document); }); tab_restored.connect ((label, restore_data, icon) => { @@ -190,7 +195,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { current_document = doc; doc.focus (); - save_opened_files (); + remember_opened_files (); } catch (Error e) { critical (e.message); } @@ -210,7 +215,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { current_document = doc; doc.focus (); - save_opened_files (); + remember_opened_files (); } catch (Error e) { critical ("Cannot insert clipboard: %s", clipboard); } @@ -248,7 +253,8 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { if (cursor_position > 0) { doc.source_view.cursor_position = cursor_position; } - save_opened_files (); + + remember_opened_files (); }); return false; @@ -300,22 +306,13 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } } - public bool close_document (Services.Document doc) { - if (doc.do_close ()) { - remove_tab (doc); - return true; - } - - return false; + public void close_document (Services.Document doc) { + close_tab_requested (doc); } public void close_current_document () { var doc = current_document; - if (doc != null) { - if (close_tab_requested (doc)) { - remove_tab (doc); - } - } + close_tab_requested (doc); } public void request_placeholder_if_empty () { @@ -381,7 +378,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } if (!is_closing) { - save_opened_files (); + remember_opened_files (); } } @@ -409,7 +406,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { doc.focus (); - save_opened_files (); + remember_opened_files (); } private bool on_focus_in_event () { @@ -443,7 +440,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } } - public void save_opened_files () { + public void remember_opened_files () { if (privacy_settings.get_boolean ("remember-recent-files")) { var vb = new VariantBuilder (new VariantType ("a(si)")); tabs.foreach ((tab) => { @@ -457,7 +454,7 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook { } } - private void save_focused_document_uri (Services.Document? current_document) { + private void remember_focused_document_uri (Services.Document? current_document) { if (privacy_settings.get_boolean ("remember-recent-files")) { var file_uri = ""; From d8e497660869e3f54d3da950d437d895c5f0f2a8 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 20:05:43 +0000 Subject: [PATCH 29/39] Remove debug code --- src/Services/Document.vala | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1806fd1ac6..1757653676 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -52,7 +52,6 @@ namespace Scratch.Services { } private set { - warning ("setting file to %s", value.get_path ()); source_file.set_location (value); source_view.location = value; file_changed (); From 6badb4a36e6cfec3cdbf0368a38702dff3b718aa Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Mon, 30 Jan 2023 20:06:52 +0000 Subject: [PATCH 30/39] Cleanup, delint --- src/Services/DocumentManager.vala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 922049a6bc..e02634a2e9 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -242,8 +242,7 @@ public class Scratch.Services.DocumentManager : Object { doc.save_cancellable, null ); - // is_saved = yield save_doc (doc, reason); - } catch (Error e){ + } catch (Error e) { if (e.code != 19) { // Not cancelled critical ( "Cannot save \"%s\": %s", From 1f3a009a227bb1c74fc658d74229c3b5fe98f9cb Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 10:24:11 +0000 Subject: [PATCH 31/39] Cleanup, reduce some scopes --- src/MainWindow.vala | 16 ------------- src/Services/Document.vala | 37 ++++++++++++++++--------------- src/Services/DocumentManager.vala | 4 +--- 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/src/MainWindow.vala b/src/MainWindow.vala index 0245ebdc76..c1d436c7c0 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -643,19 +643,6 @@ namespace Scratch { document_view.close_document (doc); } - // // Check if there no unsaved changes - // private bool check_unsaved_changes () { - // document_view.is_closing = true; - // foreach (var doc in document_view.docs) { - // if (!doc.do_close (true)) { - // document_view.current_document = doc; - // return false; - // } - // } - - // return true; - // } - // Save session information different from window state private void restore_saved_state_extra () { // Plugin panes size @@ -812,9 +799,6 @@ namespace Scratch { private void action_quit () { handle_quit (); - // if (check_unsaved_changes ()) { - // destroy (); - // } } private void action_open () { diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1757653676..042989452f 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -35,6 +35,8 @@ namespace Scratch.Services { // The parent window's actions public unowned SimpleActionGroup actions { get; set construct; } + public Gtk.SourceFile source_file { get; private set; } + public Scratch.Widgets.SourceView source_view { get; private set; } public bool is_file_temporary { get { @@ -44,8 +46,6 @@ namespace Scratch.Services { } } - public Gtk.SourceFile source_file { get; private set; } - public GLib.File file { get { return source_file.location; @@ -88,31 +88,30 @@ namespace Scratch.Services { } } - public Gtk.Stack main_stack; - public Scratch.Widgets.SourceView source_view; - - - public bool saved = true; public bool delay_autosaving { get; set; } - public bool inhibit_saving { get { return !loaded || completion_shown; } } + public bool content_changed { + get { + return last_save_content != source_view.buffer.text; + } + } - public Scratch.Services.SymbolOutline? outline { get; private set; default = null; } + private Scratch.Services.SymbolOutline? outline = null; private string original_content = ""; // For restoring to original - //TODO Do we need this AND buffer.get_modified ()? - public string last_save_content = ""; // For detecting unsaved content + private string last_save_content = ""; // For detecting unsaved content private bool completion_shown = false; private bool loaded = false; + private Gtk.Stack main_stack; private Gtk.ScrolledWindow scroll; private Gtk.InfoBar info_bar; private Gtk.SourceMap source_map; private Gtk.Paned outline_widget_pane; - + private DocumentManager doc_manager; // Used by DocumentManager public GLib.Cancellable save_cancellable; public GLib.Cancellable load_cancellable; @@ -159,6 +158,8 @@ namespace Scratch.Services { source_map = new Gtk.SourceMap (); outline_widget_pane = new Gtk.Paned (Gtk.Orientation.HORIZONTAL); + doc_manager = DocumentManager.get_instance (); + if (builder_blocks_font != null && builder_font_map != null) { source_map.set_font_map (builder_font_map); source_map.font_desc = builder_blocks_font; @@ -195,11 +196,11 @@ namespace Scratch.Services { check_undoable_actions (); }); - source_view.buffer.changed.connect ((buffer) => { + source_view.buffer.changed.connect (() => { // May need to wait for completion to close // which would otherwise inhibit saving Idle.add (() => { - DocumentManager.get_instance ().save_request.begin ( + doc_manager.save_request.begin ( this, SaveReason.AUTOSAVE ); return Source.REMOVE; @@ -352,7 +353,7 @@ namespace Scratch.Services { return true; } - if (yield DocumentManager.get_instance ().save_request ( + if (yield doc_manager.save_request ( this, app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING )) { @@ -366,7 +367,7 @@ namespace Scratch.Services { public async bool save () { warning ("save"); - return yield DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); + return yield doc_manager.save_request (this, SaveReason.USER_REQUEST); } public async bool save_as () { @@ -375,7 +376,7 @@ warning ("save"); if (new_uri != "") { var old_uri = file.get_uri (); file = GLib.File.new_for_uri (new_uri); - if (!(yield DocumentManager.get_instance ().save_request ( + if (!(yield doc_manager.save_request ( this, SaveReason.USER_REQUEST))) { // Revert to original location if save failed or cancelled file = GLib.File.new_for_uri (old_uri); @@ -437,7 +438,7 @@ warning ("save"); public async bool move (File new_dest) { this.file = new_dest; - return yield DocumentManager.get_instance ().save_request (this, SaveReason.USER_REQUEST); + return yield doc_manager.save_request (this, SaveReason.USER_REQUEST); } private void restore_settings () { diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index e02634a2e9..f175b81356 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -152,9 +152,7 @@ public class Scratch.Services.DocumentManager : Object { } // Only ask user if there are some changes - if (confirm && - doc.source_view.buffer.text != doc.last_save_content) { - + if (confirm && doc.content_changed) { bool save_changes; if (!query_save_changes (doc, out save_changes)) { // User cancelled operation From d5bb6b37441483a1c20069ac988c7c8b9e6b5dc9 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 11:14:55 +0000 Subject: [PATCH 32/39] Fix updating save content after save --- src/Services/Document.vala | 13 +++---- src/Services/DocumentManager.vala | 64 ++++++++----------------------- 2 files changed, 23 insertions(+), 54 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 042989452f..1fe59c7f30 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -102,7 +102,7 @@ namespace Scratch.Services { private Scratch.Services.SymbolOutline? outline = null; private string original_content = ""; // For restoring to original - private string last_save_content = ""; // For detecting unsaved content + public string last_save_content = ""; // For detecting internal and external changes private bool completion_shown = false; private bool loaded = false; @@ -219,6 +219,7 @@ namespace Scratch.Services { ellipsize_mode = Pango.EllipsizeMode.MIDDLE; } + private uint load_timout_id = 0; public async void open (bool force = false) { /* Loading improper files may hang so we cancel after a certain time as a fallback. @@ -347,16 +348,13 @@ namespace Scratch.Services { public async bool do_close (bool app_closing) { debug ("Closing \"%s\"", get_basename ()); - if (!loaded) { load_cancellable.cancel (); return true; } - if (yield doc_manager.save_request ( - this, - app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING - )) { + var reason = app_closing ? SaveReason.APP_CLOSING : SaveReason.TAB_CLOSING; + if (yield doc_manager.save_request (this, reason)) { // DocumentManager will delete any backup doc_closed (); return true; @@ -366,7 +364,6 @@ namespace Scratch.Services { } public async bool save () { -warning ("save"); return yield doc_manager.save_request (this, SaveReason.USER_REQUEST); } @@ -629,6 +626,7 @@ warning ("save"); // Called on focus in and after failed saving public void check_file_status () { // If the file does not exist anymore + assert (!working); if (!exists ()) { if (mounted == false) { string message = _( @@ -699,6 +697,7 @@ warning ("save"); return; } +warning ("last save content not equal to new_buffer.text"); // In case of conflict, either discard current changes and load external changes // or continue. If continuing, the user can later rename this document to keep // external changes or overwrite them by saving with the same name. diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index f175b81356..e2d07519dc 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -217,29 +217,33 @@ public class Scratch.Services.DocumentManager : Object { // Saving to the location given in the doc source file will be attempted var is_saved = false; - var save_buffer = new Gtk.SourceBuffer (null); - var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); - save_buffer.text = source_buffer.text; - - // Replace old content with the new one - //TODO Handle cancellables internally - doc.save_cancellable.cancel (); - doc.save_cancellable = new GLib.Cancellable (); - var source_file_saver = new Gtk.SourceFileSaver ( - source_buffer, - doc.source_file - ); + var save_buffer = new Gtk.SourceBuffer (null); + var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); + save_buffer.text = source_buffer.text; + + // Replace old content with the new one + //TODO Handle cancellables internally + doc.save_cancellable.cancel (); + doc.save_cancellable = new GLib.Cancellable (); + var source_file_saver = new Gtk.SourceFileSaver ( + source_buffer, + doc.source_file + ); + if (reason == SaveReason.APP_CLOSING) { GLib.Application.get_default ().hold (); } try { - is_saved = yield source_file_saver.save_async ( GLib.Priority.DEFAULT, doc.save_cancellable, null ); + + if (is_saved) { + doc.last_save_content = save_buffer.text; + } } catch (Error e) { if (e.code != 19) { // Not cancelled critical ( @@ -263,40 +267,6 @@ public class Scratch.Services.DocumentManager : Object { return is_saved; } - // This is only called once but is split out for clarity - // It is expected that the document buffer will not change during this process - // Any stripping or other automatic change has already taken place - // private async bool save_doc (Document doc, SaveReason reason) throws Error { - // var save_buffer = new Gtk.SourceBuffer (null); - // var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); - // save_buffer.text = source_buffer.text; - - // // Replace old content with the new one - // //TODO Handle cancellables internally - // doc.save_cancellable.cancel (); - // doc.save_cancellable = new GLib.Cancellable (); - // var source_file_saver = new Gtk.SourceFileSaver ( - // source_buffer, - // doc.source_file - // ); - - // if (reason == SaveReason.APP_CLOSING) { - // GLib.Application.get_default ().hold (); - // } - - // var success = yield source_file_saver.save_async ( - // GLib.Priority.DEFAULT, - // doc.save_cancellable, - // null - // ); - - // if (reason == SaveReason.APP_CLOSING) { - // GLib.Application.get_default ().release (); - // } - - // return success; - // } - // This is only called once but is split out for clarity private void strip_trailing_spaces_before_save (Document doc) { var source_buffer = (Gtk.SourceBuffer)(doc.source_view.buffer); From 5d9d107295ff2f23bea7c3558adc4374c1f4a954 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 11:40:53 +0000 Subject: [PATCH 33/39] Restore update saved state on quit; delint --- src/MainWindow.vala | 1 + src/Services/Document.vala | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/MainWindow.vala b/src/MainWindow.vala index c1d436c7c0..9fdcd141f4 100644 --- a/src/MainWindow.vala +++ b/src/MainWindow.vala @@ -697,6 +697,7 @@ namespace Scratch { private void handle_quit () { save_all_documents.begin ((obj, res) => { if (save_all_documents.end (res)) { + update_saved_state (); destroy (); } }); diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 1fe59c7f30..c87c969bc6 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -100,7 +100,7 @@ namespace Scratch.Services { } } - private Scratch.Services.SymbolOutline? outline = null; + private Scratch.Services.SymbolOutline? outline = null; private string original_content = ""; // For restoring to original public string last_save_content = ""; // For detecting internal and external changes private bool completion_shown = false; From c9e744dfc83b1fcb86e5a8ddf354b545db84eaa8 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 13:15:09 +0000 Subject: [PATCH 34/39] Ensure file status checked after loading (avoid race with focus in event) --- src/Services/Document.vala | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index c87c969bc6..f75483bd18 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -246,9 +246,8 @@ namespace Scratch.Services { source_view.sensitive = false; this.working = true; - + // Check whether it is a text file var content_type = ContentType.from_mime_type (mime_type); - if (!force && !(ContentType.is_a (content_type, "text/plain"))) { var title = _("%s Is Not a Text File").printf (get_basename ()); var description = _("Code will not load this type of file."); @@ -271,7 +270,7 @@ namespace Scratch.Services { } var buffer = new Gtk.SourceBuffer (null); /* Faster to load into a separate buffer */ - + // Set time limit on loading the file load_timout_id = Timeout.add_seconds_full (GLib.Priority.HIGH, 5, () => { if (load_cancellable != null && !load_cancellable.is_cancelled ()) { var title = _("Loading File \"%s\" Is Taking a Long Time").printf (get_basename ()); @@ -294,6 +293,7 @@ namespace Scratch.Services { return GLib.Source.REMOVE; }); + //Try to load the file try { var source_file_loader = new Gtk.SourceFileLoader (buffer, source_file); yield source_file_loader.load_async (GLib.Priority.LOW, load_cancellable, null); @@ -319,8 +319,7 @@ namespace Scratch.Services { } } - // Focus in event for SourceView - focus_in_event.connect (on_focus_in); + // Change syntax highlight this.source_view.change_syntax_highlight_from_file (this.file); @@ -340,6 +339,10 @@ namespace Scratch.Services { Idle.add (() => { working = false; loaded = true; + // Check file status etc + on_focus_in (); + // File status rechecked on every focus in + focus_in_event.connect (on_focus_in); return false; }); From ac79f660dc279b67fc6319f023ee8b6c7e044168 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 14:39:17 +0000 Subject: [PATCH 35/39] Dont call check_file_status while working --- src/Services/DocumentManager.vala | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index e2d07519dc..ce09e40c9f 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -251,17 +251,14 @@ public class Scratch.Services.DocumentManager : Object { doc.get_basename (), e.message ); - - // Inform user of failure - doc.check_file_status (); } } finally { doc.working = false; doc.set_saved_status (); - if (reason == SaveReason.APP_CLOSING) { - GLib.Application.get_default ().release (); - } + if (reason == SaveReason.APP_CLOSING) { + GLib.Application.get_default ().release (); + } } return is_saved; From e34db60f1fc9a2b7e339bd4e343b730426764aa0 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 15:14:49 +0000 Subject: [PATCH 36/39] Warn when save fails --- src/Services/Document.vala | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index f75483bd18..63bd21af93 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -380,6 +380,26 @@ namespace Scratch.Services { this, SaveReason.USER_REQUEST))) { // Revert to original location if save failed or cancelled file = GLib.File.new_for_uri (old_uri); + string message = _( + "You cannot save to the file \"%s\".\n Do you want to save the changes somewhere else?" + ).printf ("%s".printf (new_uri)); + + set_message ( + Gtk.MessageType.WARNING, + message, + _("Save changes elsewhere"), + () => { + save_as.begin ((obj, res) => { + if (save_as.end (res)) { + hide_info_bar (); + } + }); + }, + _("Cancel"), () => { + hide_info_bar (); + } + ); + return false; } From 13cb07df9621801f8ffd559863bf595b2294a99d Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 15:25:52 +0000 Subject: [PATCH 37/39] Document: Cleanup unused --- src/Services/Document.vala | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index 63bd21af93..b0db9704fa 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -446,21 +446,9 @@ namespace Scratch.Services { } file_chooser.destroy (); - - // //Check that the location is writable - // var new_file = File.new_for_path (new_path); - // if (!can_write (new_file)) { - // new_path = ""; - // } - return new_path; } - public async bool move (File new_dest) { - this.file = new_dest; - return yield doc_manager.save_request (this, SaveReason.USER_REQUEST); - } - private void restore_settings () { if (Scratch.settings.get_boolean ("show-mini-map")) { source_map.show (); From 606f7836cf1ede3b9c5869a0787bf75e90cc7c82 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 16:47:36 +0000 Subject: [PATCH 38/39] Partial fix backups --- src/Services/Document.vala | 12 ++++++- src/Services/DocumentManager.vala | 56 +++++++++++-------------------- src/Services/FileHandler.vala | 32 +++++++++++++++++- 3 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/Services/Document.vala b/src/Services/Document.vala index b0db9704fa..7f0f3d04bd 100644 --- a/src/Services/Document.vala +++ b/src/Services/Document.vala @@ -375,6 +375,7 @@ namespace Scratch.Services { assert_nonnull (new_uri); if (new_uri != "") { var old_uri = file.get_uri (); + var was_temporary = is_file_temporary; file = GLib.File.new_for_uri (new_uri); if (!(yield doc_manager.save_request ( this, SaveReason.USER_REQUEST))) { @@ -401,8 +402,17 @@ namespace Scratch.Services { ); return false; + } else if (was_temporary) { + //Need to delete old temp file and backup + var temp_file = GLib.File.new_for_uri (old_uri); + //TODO DRY this (also in DocumentManager) + try { + temp_file.delete (); + } catch (Error e) { + //TODO Inform user in UI? + warning ("Cannot delete temporary file \"%s\": %s", old_uri, e.message); + } } - return true; } else { warning ("Save As: Failed to get new uri"); diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index ce09e40c9f..1e6c63e6df 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -130,46 +130,42 @@ public class Scratch.Services.DocumentManager : Object { remove_autosave_for_doc (doc); - bool confirm, closing; + bool confirm; switch (reason) { case USER_REQUEST: + case AUTOSAVE: // Should not come here confirm = false; - closing = false; break; case TAB_CLOSING: - case APP_CLOSING: - if (!doc.is_file_temporary) { - confirm = !autosave_on; - } else { - //Always give opportunity to save as permanent file - confirm = true; - } + confirm = true; + break; + case APP_CLOSING: + confirm = false; // Always just save open docs break; + default: assert_not_reached (); } - // Only ask user if there are some changes - if (confirm && doc.content_changed) { - bool save_changes; + bool save_changes = false; + // Only ask user if there are some changes or file is temporary + if (confirm && (doc.content_changed || doc.is_file_temporary)) { if (!query_save_changes (doc, out save_changes)) { // User cancelled operation return false; } + } - if (!save_changes && doc.is_file_temporary) { - //User chose to discard the temporary file rather than save - try { - doc.file.delete (); - } catch (Error e) { - //TODO Inform user in UI? - warning ("Cannot delete temporary file \"%s\": %s", doc.file.get_uri (), e.message); - } - - return true; + if (!save_changes) { + if (doc.is_file_temporary) { + FileHandler.delete_file_and_backup (doc.file); + } else { + FileHandler.delete_backup (doc.file); } + + return true; } // Save even when no changes as may need to overwrite external changes @@ -200,20 +196,7 @@ public class Scratch.Services.DocumentManager : Object { } } - //Create backup file - var backup = File.new_for_path (doc.file.get_path () + "~"); - if (!backup.query_exists ()) { - try { - doc.file.copy (backup, FileCopyFlags.NONE); - } catch (Error e) { - warning ( - "Cannot create backup copy for file \"%s\": %s", - doc.get_basename (), - e.message - ); - //Should we return fail now? The actual save will probably fail too - } - } + FileHandler.create_backup (doc.file); // Saving to the location given in the doc source file will be attempted var is_saved = false; @@ -243,6 +226,7 @@ public class Scratch.Services.DocumentManager : Object { if (is_saved) { doc.last_save_content = save_buffer.text; + FileHandler.delete_backup (doc.file); } } catch (Error e) { if (e.code != 19) { // Not cancelled diff --git a/src/Services/FileHandler.vala b/src/Services/FileHandler.vala index 1d2b077fd9..a019f4efc4 100644 --- a/src/Services/FileHandler.vala +++ b/src/Services/FileHandler.vala @@ -19,7 +19,6 @@ ***/ namespace Scratch.Services { - public enum FileOption { EXISTS, IS_DIR, @@ -143,6 +142,37 @@ namespace Scratch.Services { else return false; } + + public static void create_backup (GLib.File file) { + //Create backup file + var backup = File.new_for_path (file.get_path () + "~"); + if (!backup.query_exists ()) { + try { + file.copy (backup, FileCopyFlags.NONE); + } catch (Error e) { + warning ( + "Cannot create backup copy for file \"%s\": %s", + file.get_basename (), + e.message + ); + //Should we return fail now? The actual save will probably fail too + } + } + } + + public static void delete_file_and_backup (GLib.File file) { + delete_backup (file); + try { + file.delete (); + } catch (Error e) {} + } + + public static void delete_backup (GLib.File file) { + var backup = File.new_for_path (file.get_path () + "~"); + try { + backup.delete (); + } catch (Error e) {} + } } } From cca16ee29e843a537f0a48e8a66f7a4c67e585b4 Mon Sep 17 00:00:00 2001 From: Jeremy Wootten Date: Tue, 31 Jan 2023 17:40:41 +0000 Subject: [PATCH 39/39] Fix temp and backup file creation/deletion --- src/Services/DocumentManager.vala | 7 ++++--- src/Services/FileHandler.vala | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Services/DocumentManager.vala b/src/Services/DocumentManager.vala index 1e6c63e6df..344cbf50c0 100644 --- a/src/Services/DocumentManager.vala +++ b/src/Services/DocumentManager.vala @@ -149,16 +149,16 @@ public class Scratch.Services.DocumentManager : Object { assert_not_reached (); } - bool save_changes = false; + bool save_required = true; // Only ask user if there are some changes or file is temporary if (confirm && (doc.content_changed || doc.is_file_temporary)) { - if (!query_save_changes (doc, out save_changes)) { + if (!query_save_changes (doc, out save_required)) { // User cancelled operation return false; } } - if (!save_changes) { + if (!save_required) { if (doc.is_file_temporary) { FileHandler.delete_file_and_backup (doc.file); } else { @@ -245,6 +245,7 @@ public class Scratch.Services.DocumentManager : Object { } } + //NOTE If save failed, the backup file remains on disk, but is not used return is_saved; } diff --git a/src/Services/FileHandler.vala b/src/Services/FileHandler.vala index a019f4efc4..64a092d7c7 100644 --- a/src/Services/FileHandler.vala +++ b/src/Services/FileHandler.vala @@ -146,17 +146,17 @@ namespace Scratch.Services { public static void create_backup (GLib.File file) { //Create backup file var backup = File.new_for_path (file.get_path () + "~"); - if (!backup.query_exists ()) { - try { - file.copy (backup, FileCopyFlags.NONE); - } catch (Error e) { - warning ( - "Cannot create backup copy for file \"%s\": %s", - file.get_basename (), - e.message - ); - //Should we return fail now? The actual save will probably fail too - } + // Any existing file will be out of date - delete it. + delete_backup (backup); + try { + file.copy (backup, FileCopyFlags.NONE); + } catch (Error e) { + warning ( + "Cannot create backup copy for file \"%s\": %s", + file.get_basename (), + e.message + ); + //Should we return fail now? The actual save will probably fail too } }