Cleanup: Simplify open document

plugins/FuzzySearch/FuzzySearch.vala

@@ -127,10 +127,7 @@ public class Scratch.Plugins.FuzzySearch: Peas.ExtensionBase, Scratch.Services.A
 
         var popover = new Scratch.FuzzySearchPopover (indexer, window);
         popover.open_file.connect ((filepath) => {
-            var file = new Scratch.FolderManager.File (filepath);
-            var doc = new Scratch.Services.Document (window.actions, file.file);
-
-            window.open_document.begin (doc);
+            window.document_view.open_document.begin (filepath);
             popover.popdown ();
         });
 

src/Application.vala

@@ -194,16 +194,16 @@ namespace Scratch {
             var window = get_last_window ();
             foreach (var file in files) {
                 bool is_folder;
+                string path = file.get_uri ();
                 if (Scratch.Services.FileHandler.can_open_file (file, out is_folder)) {
                     if (is_folder) {
                         window.open_folder (file);
                     } else {
                         debug ("Files length: %d\n", files.length);
-                        var doc = new Scratch.Services.Document (window.actions, file);
                         if (location_jump_manager.has_selection_range != null && files.length == 1) {
-                            window.open_document_at_selected_range.begin (doc, true, location_jump_manager.range);
+                            window.document_view.open_document.begin (path, true, -2, location_jump_manager.range);
                         } else {
-                            window.open_document.begin (doc);
+                            window.document_view.open_document.begin (path);
                         }
                     }
                 }

src/MainWindow.vala

@@ -41,6 +41,8 @@ public class Scratch.MainWindow : Hdy.Window {
     }
 
     public Code.Sidebar sidebar { get; private set; }
+    // Temporary fix to make get_project_for_file function accessible.
+    public FolderManager.FileView folder_manager_view;
     public Scratch.Application app { get; private set; }
     public Scratch.Widgets.DocumentView document_view { get; private set; }
     public SimpleActionGroup actions { get; private set; }
@@ -160,7 +162,6 @@ public class Scratch.MainWindow : Hdy.Window {
 
     private Code.Terminal terminal;
     private Code.WelcomeView welcome_view;
-    private FolderManager.FileView folder_manager_view;
     private Gtk.Clipboard clipboard;
     private Gtk.EventControllerKey key_controller;
     private Gtk.Paned hp1;
@@ -445,8 +446,7 @@ public class Scratch.MainWindow : Hdy.Window {
                     bool is_folder;
                     //TODO Handle folders dropped here
                     if (Scratch.Services.FileHandler.can_open_file (file, out is_folder) && !is_folder) {
-                        Scratch.Services.Document doc = new Scratch.Services.Document (actions, file);
-                        document_view.open_document.begin (doc);
+                        document_view.open_document.begin (filename);
                     }
                 }
 
@@ -461,12 +461,10 @@ public class Scratch.MainWindow : Hdy.Window {
         sidebar.add_tab (folder_manager_view);
         folder_manager_view.show_all ();
 
-        folder_manager_view.activate.connect ((a) => {
-            var file = new Scratch.FolderManager.File (a);
-            var doc = new Scratch.Services.Document (actions, file.file);
-
+        folder_manager_view.activate.connect ((path) => {
+            var file = new Scratch.FolderManager.File (path);
             if (file.is_valid_textfile) {
-                open_document.begin (doc);
+                document_view.open_document.begin (path);
             } else {
                 open_binary (file.file);
             }
@@ -683,14 +681,22 @@ public class Scratch.MainWindow : Hdy.Window {
                             focused_file = file;
                         }
                         //TODO Check files valid (settings could have been manually altered)
-                        var doc = new Scratch.Services.Document (actions, file);
-                        if (doc.exists () || !doc.is_file_temporary) {
-                            if (restore_override != null && (file.get_path () == restore_override.file.get_path ())) {
-                                yield open_document_at_selected_range (doc, true, restore_override.range, true);
-                                was_restore_overriden = true;
-                            } else {
-                                yield open_document (doc, was_restore_overriden ? false : is_focused, pos);
-                            }
+                        if (restore_override != null &&
+                            (file.get_path () == restore_override.file.get_path ())) {
+
+                            yield document_view.open_document (
+                                uri,
+                                true,
+                                -2,
+                                restore_override.range
+                            );
+                            was_restore_overriden = true;
+                        } else {
+                            yield document_view.open_document (
+                                uri,
+                                was_restore_overriden ? false : is_focused,
+                                pos
+                            );
                         }
                     }
                 }
@@ -783,27 +789,6 @@ public class Scratch.MainWindow : Hdy.Window {
         folder_manager_view.open_folder (foldermanager_file);
     }
 
-    public async void open_document (Scratch.Services.Document doc,
-                               bool focus = true,
-                               int cursor_position = 0) {
-
-        FolderManager.ProjectFolderItem? project = folder_manager_view.get_project_for_file (doc.file);
-        doc.source_view.project = project;
-        yield document_view.open_document (doc, focus, cursor_position);
-    }
-
-    public async void open_document_at_selected_range (Scratch.Services.Document doc,
-                                                 bool focus = true,
-                                                 SelectionRange range = SelectionRange.EMPTY,
-                                                 bool is_override = false) {
-        if (restore_override != null && is_override == false) {
-            return;
-        }
-
-        doc.source_view.project = folder_manager_view.get_project_for_file (doc.file);
-        yield document_view.open_document (doc, focus, 0, range);
-    }
-
     // Close a document
     public void close_document (Scratch.Services.Document doc) {
         document_view.close_document (doc);
@@ -1003,9 +988,7 @@ public class Scratch.MainWindow : Hdy.Window {
                     // Update last visited path
                     Utils.last_path = Path.get_dirname (uri);
                     // Open the file
-                    var file = File.new_for_uri (uri);
-                    var doc = new Scratch.Services.Document (actions, file);
-                    open_document.begin (doc);
+                    document_view.open_document.begin (uri);
                 }
             }
         });
@@ -1020,10 +1003,7 @@ public class Scratch.MainWindow : Hdy.Window {
         }
 
         var new_window = new MainWindow (false);
-        var file = File.new_for_path (path);
-        var doc = new Scratch.Services.Document (new_window.actions, file);
-
-        new_window.open_document.begin (doc, true);
+        new_window.document_view.open_document.begin (path, true);
     }
 
 
@@ -1278,8 +1258,7 @@ public class Scratch.MainWindow : Hdy.Window {
 
     private void restore_project_docs (string project_path) {
         document_manager.take_restorable_paths (project_path).@foreach ((doc_path) => {
-            var doc = new Scratch.Services.Document (actions, File.new_for_path (doc_path));
-            open_document.begin (doc); // Use this to reassociate project and document.
+            document_view.open_document.begin (doc_path); // Use this to reassociate project and document.
             return true;
         });
     }

src/Widgets/DocumentView.vala

@@ -293,78 +293,97 @@ public class Scratch.Widgets.DocumentView : Gtk.Box {
     }
 
     public void new_document () {
-        var file = File.new_for_path (unsaved_file_path_builder ());
+        var new_doc_path = unsaved_file_path_builder ();
+        var file = File.new_for_path (new_doc_path);
         try {
             file.create (FileCreateFlags.PRIVATE);
-
-            var doc = new Services.Document (window.actions, file);
             // Must open document in order to unlock it.
-            open_document.begin (doc);
+            open_document.begin (new_doc_path);
         } catch (Error e) {
             critical (e.message);
         }
     }
 
     public void new_document_from_clipboard (string clipboard) {
-        var file = File.new_for_path (unsaved_file_path_builder ());
+        var new_doc_path = unsaved_file_path_builder ();
+        var file = File.new_for_path (new_doc_path);
 
         // Set clipboard content
         try {
             file.create (FileCreateFlags.PRIVATE);
             file.replace_contents (clipboard.data, null, false, 0, null);
-            var doc = new Services.Document (window.actions, file);
-
-            open_document.begin (doc);
-
-
+            open_document.begin (new_doc_path);
         } catch (Error e) {
             critical ("Cannot insert clipboard: %s", clipboard);
         }
     }
 
-    public async void open_document (Services.Document doc, bool focus = true, int cursor_position = 0, SelectionRange range = SelectionRange.EMPTY) {
-       for (int n = 0; n <= docs.length (); n++) {
-            var nth_doc = docs.nth_data (n);
-            if (nth_doc == null) {
+    // Open document from path to avoid unnecessarily creating Document objects and
+    // to confine Document creation to this class.
+    // Documents are treated the same regarding the focus, cursor_position and range parameters
+    // whether already open or not.
+    // Cursor position may have any value > -2 to set cursor. Lower values are ignored (default)
+    // Specifying a valid non-empty selection range overrides the cursor position
+    public async void open_document (
+        string doc_path,
+        bool focus = true,
+        int cursor_position = -2,
+        SelectionRange range = SelectionRange.EMPTY
+    ) {
+        Scratch.Services.Document? doc = null; // The document to be created and/or focused
+        // Check whether document already open
+        bool found = false;
+        for (int n = 0; n < docs.length (); n++) {
+            doc = docs.nth_data (n);
+            //TODO Is this check necessary?
+            if (doc == null || doc.file == null) {
+                critical ("Invalid document at position %i. %s", n, doc == null ? "Null document" : "Null document file");
                 continue;
             }
 
-            if (nth_doc.file != null && nth_doc.file.get_uri () == doc.file.get_uri ()) {
-                if (focus) {
-                    current_document = nth_doc;
-                }
-
-                debug ("This Document was already opened! Not opening a duplicate!");
-                if (range != SelectionRange.EMPTY) {
-                    Idle.add_full (GLib.Priority.LOW, () => { // This helps ensures new tab is drawn before opening document.
-                        current_document.source_view.select_range (range);
-                        update_opened_files_setting ();
-
-                        return false;
-                    });
-                }
-
-                return;
+            if (doc.file.get_uri () == doc_path) {
+                found = true;
+                break;
             }
         }
 
-        insert_document (doc, (int) docs.length ());
-        if (focus) {
-            current_document = doc;
+        if (!found) {
+            // Document not already open, create and insert it now
+            doc = new Scratch.Services.Document (
+                window.actions,
+                GLib.File.new_for_commandline_arg (doc_path)
+            );
+            // Temporary fix, the get_project_for_file function is intended to be moved
+            // to GitManager instance.
+            var project = window.folder_manager_view.get_project_for_file (doc.file);
+            doc.source_view.project = project;
+            insert_document (doc, (int) docs.length ());
+            // Load contents before proceeding
+            yield doc.open (false);
+        } else {
+            debug ("This Document was already opened! Not opening a duplicate!");
         }
 
-        yield doc.open (false);
-
-        if (focus && doc == current_document) {
+        // @doc must have been assigned at this point
+        assert (doc != null);
+        // If we need to select a range we have to focus document anyway
+        if (focus || range != SelectionRange.EMPTY) {
             doc.focus ();
+            current_document = doc;
         }
 
-        if (range != SelectionRange.EMPTY) {
-            doc.source_view.select_range (range);
-        } else if (cursor_position > 0) {
+        if (cursor_position > -2) {
             doc.source_view.cursor_position = cursor_position;
         }
 
+        if (range != SelectionRange.EMPTY) {
+            Idle.add_full (GLib.Priority.LOW, () => { // This helps ensures new tab is drawn before opening document.
+                current_document.source_view.select_range (range);
+                update_opened_files_setting (); //Records changed cursor position
+                return false;
+            });
+        }
+
         update_opened_files_setting ();
     }
 
@@ -517,9 +536,7 @@ public class Scratch.Widgets.DocumentView : Gtk.Box {
     }
 
     public void restore_closed_tab (string path) {
-        var file = File.new_for_path (path);
-        var doc = new Services.Document (window.actions, file);
-        open_document.begin (doc);
+        open_document.begin (path);
 
         var menu = (Menu) tab_history_button.menu_model;
         for (var i = 0; i < menu.get_n_items (); i++) {
@@ -634,9 +651,7 @@ public class Scratch.Widgets.DocumentView : Gtk.Box {
         if (info == TargetType.URI_LIST) {
             var uris = sel.get_uris ();
             foreach (var filename in uris) {
-                var file = File.new_for_uri (filename);
-                var doc = new Services.Document (window.actions, file);
-                open_document.begin (doc);
+                open_document.begin (filename);
             }
 
             Gtk.drag_finish (ctx, true, false, time);

src/Widgets/SourceView.vala

@@ -67,9 +67,16 @@ namespace Scratch.Widgets {
             }
 
             set {
+                // Values of -1 and greater than number of characters
+                // in the buffer place the cursor at the end of the buffer.
+                // Ignore lower values
+                if (value <= -2) {
+                    return;
+                }
+
                 Gtk.TextIter iter;
                 buffer.get_iter_at_offset (out iter, value);
-                buffer.place_cursor (iter); //Assume invalid offset handled correctly for now
+                buffer.place_cursor (iter);
                 Idle.add (() => {
                     scroll_to_iter (iter, 0.25, false, 0, 0);
                     return Source.REMOVE;