Use dialog for external changes

src/MainWindow.vala

@@ -896,7 +896,7 @@ namespace Scratch {
                 if (doc.is_file_temporary == true) {
                     action_save_as ();
                 } else {
-                    doc.save_with_hold.begin (true);
+                    doc.save_request ();
                 }
             }
         }

src/Services/Document.vala

@@ -92,6 +92,7 @@ namespace Scratch.Services {
 
         // Locked documents can be edited but cannot be (auto)saved to the current file.
         // Locked documents can be saved to a different file (when they will be unlocked)
+        // Create as locked so focus events ingb=nored. Unlock when content is loaded
         private bool _locked = true;
         public bool locked {
             get {
@@ -115,8 +116,8 @@ namespace Scratch.Services {
         public Gtk.Stack main_stack;
         public Scratch.Widgets.SourceView source_view;
         private Scratch.Services.SymbolOutline? outline = null;
-        public string original_content;
-        private string last_save_content;
+        public string original_content = "";
+        private string last_save_content = "";
         public bool saved = true;
         private bool completion_shown = false;
 
@@ -208,6 +209,17 @@ namespace Scratch.Services {
 
             this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null);
 
+            // Focus in event for SourceView
+            // Check if file changed externally or permissions changed
+            this.source_view.focus_in_event.connect (() => {
+                if (!locked && !is_file_temporary) {
+                    check_file_status ();
+                    check_undoable_actions ();
+                }
+
+                return false;
+            });
+
             // Focus out event for SourceView
             this.source_view.focus_out_event.connect (() => {
                 if (!locked && Scratch.settings.get_boolean ("autosave")) {
@@ -237,9 +249,7 @@ namespace Scratch.Services {
                 completion_shown = false;
             });
 
-            // /* Create as loaded and unlocked - could be new document */
             loaded = file == null;
-            locked = false;
             ellipsize_mode = Pango.EllipsizeMode.MIDDLE;
         }
 
@@ -261,8 +271,10 @@ namespace Scratch.Services {
                                 Source.remove (timeout_saving);
                                 timeout_saving = 0;
                             }
+
                             timeout_saving = Timeout.add (1000, () => {
-                                save.begin ();
+                                check_file_status ();
+                                save.begin (); // Not forced
                                 timeout_saving = 0;
                                 return false;
                             });
@@ -371,16 +383,6 @@ namespace Scratch.Services {
                 }
             }
 
-            // Focus in event for SourceView
-            this.source_view.focus_in_event.connect (() => {
-                if (!working && !locked) {
-                    check_file_status ();
-                    check_undoable_actions ();
-                }
-
-                return false;
-            });
-
             // Change syntax highlight
             this.source_view.change_syntax_highlight_from_file (this.file);
 
@@ -474,6 +476,13 @@ namespace Scratch.Services {
             return ret_value;
         }
 
+        // Handle save action (only use for user interaction)
+        public void save_request () {
+            check_undoable_actions ();
+            check_file_status (); // Need to check for external changes before forcing save
+            save_with_hold.begin (true);
+        }
+
         private bool is_saving = false;
         public async bool save_with_hold (bool force = false, bool saving_as = false) {
             // Prevent reentry which could result in mismatched holds on Application
@@ -571,6 +580,7 @@ namespace Scratch.Services {
 
             this.set_saved_status (true);
             last_save_content = source_view.buffer.text;
+            hide_info_bar ();
 
             debug ("File “%s” saved successfully", get_basename ());
 
@@ -619,6 +629,7 @@ namespace Scratch.Services {
                 // Should not set "modified" state of the buffer to true - this is automatic
                 is_saved = yield save (true, true);
                 if (is_saved) {
+                    source_view.buffer.set_modified (false);
                     if (is_current_file_temporary) {
                         try {
                             // Delete temporary file
@@ -861,23 +872,37 @@ namespace Scratch.Services {
         private void check_file_status () {
             // If the file does not exist anymore
             if (!exists ()) {
-                locked = true;
-                string details;
-                if (mounted == false) {
-                    details = _("The location containing the file “%s” was unmounted.");
+                if (source_view.buffer.get_modified ()) {
+                    locked = true;
+                    string details;
+                    if (mounted == false) {
+                        details = _("The location containing the file “%s” was unmounted and there are unsaved changes.");
+                    } else {
+                        details = _("File “%s” was deleted and there are unsaved changes.");
+                    }
+
+                    ask_save_location (details.printf ("<b>%s</b>".printf (get_basename ())));
                 } else {
-                    details = _("File “%s” was deleted.");
+                    var close_tab_action = Utils.action_from_group (MainWindow.ACTION_CLOSE_TAB, actions);
+                    close_tab_action.set_enabled (true);
+                    this.saved = true; //Do not try to save
+                    close_tab_action.activate (new Variant ("s", file.get_path ()));
                 }
-
-                ask_save_location (details.printf ("<b>%s</b>".printf (get_basename ())));
-            } else if (loaded) { // Check external changes after loading
+            } else if (loaded && !is_saving) { // Check external changes after loading
                 if (!locked && !can_write () && source_view.buffer.get_modified ()) {
-                // The file has become unwritable while changes are pending
+                    // The file has become unwritable while changes are pending
                     locked = true;
                     var details = _("File “%s” does not have write permission.");
                     ask_save_location (details.printf ("<b>%s</b>".printf (get_basename ())));
                 } else {
-                // Check for external changes (can load even if locked or unwritable)
+                    // Detect external changes by comparing file content with buffer content.
+                    // Only done when no unsaved internal changes else difference from saved
+                    // file are to be expected.
+
+                    //TODO Check required behaviour on continue
+                    // If user selects to continue regardless then no further
+                    // check made for this document
+                    // External changes will be overwritten on next (auto) save
                     var new_buffer = new Gtk.SourceBuffer (null);
                     var source_file_loader = new Gtk.SourceFileLoader (
                         new_buffer,
@@ -896,36 +921,34 @@ namespace Scratch.Services {
                                 return;
                             }
 
-                            if (source_view.buffer.text == new_buffer.text) {
+                            if (last_save_content == new_buffer.text) {
                                 return;
                             }
 
-                            if (!source_view.buffer.get_modified ()) {
-                                //FIXME Should block editing until responded?
-                                if (Scratch.settings.get_boolean ("autosave")) {
-                                    source_view.set_text (new_buffer.text, false);
-                                } else {
-                                    string message = _(
-                                        "File “%s” was modified by an external application."
-                                    ).printf ("<b>%s</b>".printf (get_uri ()));
-
-                                    set_message (
-                                        Gtk.MessageType.WARNING,
-                                        message,
-                                         _("Reload"), () => {
-                                            this.source_view.set_text (
-                                                new_buffer.text, false
-                                            );
-                                            hide_info_bar ();
-                                        },
-                                        _("Continue"), () => {
-                                            hide_info_bar ();
-                                        })
-                                    ;
-                                }
+                            if (last_save_content == source_view.buffer.text) {
+                                // There are no unsaved internal edits so just load the external changes
+                                //TODO Indicate to the user external changes loaded?
+                                source_view.buffer.text = new_buffer.text;
+                                last_save_content = source_view.buffer.text;
+                                // We know the content and file are now in sync so set unmodified
+                                source_view.buffer.set_modified (false);
+                                return;
+                            }
+
+                            var primary_text = _("File “%s” was modified by an external application").printf (file.get_uri ());
+                            string secondary_text;
+
+                            if (source_view.buffer.get_modified ()) {
+                                secondary_text = _(
+                "There are also unsaved changes. Reloading the document will overwrite the unsaved changes."
+                                    );
                             } else {
-                                //TODO Handle conflicting changes (dialog?)
+                                secondary_text = _(
+                "The document changed externally since you last saved it."
+                                    );
                             }
+
+                            ask_external_changes (primary_text, secondary_text, new_buffer.text);
                         }
                     );
                 }
@@ -981,6 +1004,77 @@ namespace Scratch.Services {
             dialog.present ();
         }
 
+        private void ask_external_changes (
+            string primary_text,
+            string secondary_text,
+            string external_content
+        ) {
+            locked = true;
+
+            var app_instance = (Gtk.Application) GLib.Application.get_default ();
+            var dialog = new Granite.MessageDialog (
+                    primary_text,
+                    secondary_text,
+                    new ThemedIcon ("dialog-warning"),
+                    Gtk.ButtonsType.NONE
+                ) {
+                transient_for = app_instance.active_window
+
+            };
+
+            dialog.add_button (_("Continue"), Gtk.ResponseType.REJECT);
+
+            var reload_button = (Gtk.Button) (dialog.add_button (_("Reload"), 0));
+            reload_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION);
+
+            var overwrite_button = (Gtk.Button) (dialog.add_button (_("Overwrite"), 1));
+            overwrite_button.get_style_context ().add_class (Gtk.STYLE_CLASS_DESTRUCTIVE_ACTION);
+
+            var saveas_button = (Gtk.Button) (dialog.add_button (_("Save Document elsewhere"), Gtk.ResponseType.ACCEPT));
+            saveas_button.get_style_context ().add_class (Gtk.STYLE_CLASS_SUGGESTED_ACTION);
+
+            dialog.response.connect ((id) => {
+                dialog.destroy ();
+                Idle.add (() => {
+                    switch (id) {
+                        case Gtk.ResponseType.ACCEPT: // Save as
+                            save_as_with_hold.begin ((obj, res) => {
+                                if (save_as_with_hold.end (res)) {
+                                    locked = false;
+                                }
+                            });
+                            break;
+                        case Gtk.ResponseType.REJECT: // Ignore
+                            // Document remains locked while conflicts exist
+                            // The user must resolve some other way.  To overwrite
+                            // external changes use "Save As" with same name
+                            break;
+                        case 0: // Reload
+                            source_view.buffer.text = external_content;
+                            source_view.buffer.set_modified (false);
+                            last_save_content = source_view.buffer.text;
+                            set_saved_status (true);
+                            locked = false;
+                            break;
+                        case 1: // Overwrite
+                            // Force save, unlock to allow saving to same location
+                            locked = false;
+                            save_with_hold.begin (true, false, (obj, res) => {
+                                if (!save_with_hold.end (res)) {
+                                    locked = true;
+                                }
+                            });
+                            break;
+                        default:
+                            assert_not_reached ();
+                    }
+
+                    return Source.REMOVE;
+                });
+            });
+
+            dialog.present ();
+        }
         // Set Undo/Redo action sensitive property
         public void check_undoable_actions () {
             var source_buffer = (Gtk.SourceBuffer) source_view.buffer;

src/Widgets/DocumentView.vala

@@ -187,12 +187,8 @@ public class Scratch.Widgets.DocumentView : Granite.Widgets.DynamicNotebook {
             file.create (FileCreateFlags.PRIVATE);
 
             var doc = new Services.Document (window.actions, file);
-
-            insert_document (doc, -1);
-            current_document = doc;
-
-            doc.focus ();
-            save_opened_files ();
+            // Must open document in order to unlock it.
+            open_document (doc);
         } catch (Error e) {
             critical (e.message);
         }