Ensure folder items in sidebar always expandable

src/FolderManager/File.vala

@@ -102,6 +102,10 @@ namespace Scratch.FolderManager {
         // checks if we're dealing with a textfile
         public bool is_valid_textfile {
             get {
+                if (path.has_prefix (".goutputstream")) {
+                    return false;
+                }
+
                 if (info.get_is_backup ()) {
                     return false;
                 }
@@ -146,7 +150,10 @@ namespace Scratch.FolderManager {
                     var file_info = new FileInfo ();
                     while ((file_info = enumerator.next_file ()) != null) {
                         var child = file.get_child (file_info.get_name ());
-                        _children.add (new File (child.get_path ()));
+                        var child_file = new File (child.get_path ());
+                        if (child_file.is_valid_directory () || child_file.is_valid_textfile) {
+                            _children.add (child_file);
+                        }
                     }
 
                     children_valid = true;

src/FolderManager/FolderItem.vala

@@ -26,6 +26,7 @@ namespace Scratch.FolderManager {
     public class FolderItem : Item {
         private GLib.FileMonitor monitor;
         private bool children_loaded = false;
+        private bool has_dummy;
         private Granite.Widgets.SourceList.Item dummy; /* Blank item for expanded empty folders */
 
         public FolderItem (File file, FileView view) requires (file.is_valid_directory) {
@@ -40,22 +41,11 @@ namespace Scratch.FolderManager {
             selectable = false;
 
             dummy = new Granite.Widgets.SourceList.Item ("");
-            add (dummy);
-
-            toggled.connect (() => {
-                var root = get_root_folder ();
-                if (!children_loaded && expanded && n_children <= 1 && file.children.size > 0) {
-                    clear ();
-                    add_children ();
-                    if (root != null) {
-                        root.child_folder_loaded (this);
-                    }
+            // Must add dummy on unexpanded folders else expander will not show
+            ((Granite.Widgets.SourceList.ExpandableItem)this).add (dummy);
+            has_dummy = true;
 
-                    children_loaded = true;
-                } else if (!expanded && root != null) {
-                    root.update_item_status (this); //When toggled closed, update status to reflect hidden contents
-                }
-            });
+            toggled.connect (on_toggled);
 
             try {
                 monitor = file.file.monitor_directory (GLib.FileMonitorFlags.NONE);
@@ -65,6 +55,38 @@ namespace Scratch.FolderManager {
             }
         }
 
+        private void on_toggled () {
+            var root = get_root_folder ();
+            if (!children_loaded &&
+                 expanded &&
+                 n_children <= 1 &&
+                 file.children.size > 0) {
+
+                foreach (var child in file.children) {
+                    Granite.Widgets.SourceList.Item item = null;
+                    if (child.is_valid_directory ()) {
+                        item = new FolderItem (child, view);
+                    } else if (child.is_valid_textfile) {
+                        item = new FileItem (child, view);
+                    }
+
+                    if (item != null) {
+                        add (item);
+                    }
+                }
+
+                children_loaded = true;
+                if (root != null) {
+                    root.child_folder_loaded (this);
+                }
+            } else if (!expanded &&
+                       root != null &&
+                       root.monitored_repo != null) {
+                //When toggled closed, update status to reflect hidden contents
+                root.update_item_status (this);
+            }
+        }
+
         public override Gtk.Menu? get_context_menu () {
             var contractor_menu = new Gtk.Menu ();
 
@@ -186,79 +208,70 @@ namespace Scratch.FolderManager {
             return new_item;
         }
 
-        private void add_children () {
-            foreach (var child in file.children) {
-                Granite.Widgets.SourceList.Item item = null;
-                if (child.is_valid_directory ()) {
-                    item = new FolderItem (child, view);
-                } else if (child.is_valid_textfile) {
-                    item = new FileItem (child, view);
-                }
-
-                if (item != null) {
-                    add (item);
-                }
-            }
-        }
-
-        private void remove_all_children () {
+        public void remove_all_badges () {
             foreach (var child in children) {
-                remove (child);
+                remove_badge (child);
             }
         }
 
-        private new void remove (Granite.Widgets.SourceList.Item item) {
+        private void remove_badge (Granite.Widgets.SourceList.Item item) {
             if (item is FolderItem) {
-                ((FolderItem) item).remove_all_children ();
+                ((FolderItem) item).remove_all_badges ();
             }
 
-            base.remove (item);
+            item.badge = "";
         }
 
-        public void remove_all_badges () {
-            foreach (var child in children) {
-                remove_badge (child);
+        public new void add (Granite.Widgets.SourceList.Item item) {
+            if (has_dummy && n_children == 1) {
+                ((Granite.Widgets.SourceList.ExpandableItem)this).remove (dummy);
+                has_dummy = false;
             }
+
+            ((Granite.Widgets.SourceList.ExpandableItem)this).add (item);
         }
 
-        private void remove_badge (Granite.Widgets.SourceList.Item item) {
+        public new void remove (Granite.Widgets.SourceList.Item item) {
             if (item is FolderItem) {
-                ((FolderItem) item).remove_all_badges ();
+                var folder = (FolderItem)item;
+                foreach (var child in folder.children) {
+                    folder.remove (child);
+                }
             }
 
-            item.badge = "";
+            view.ignore_next_select = true;
+            ((Granite.Widgets.SourceList.ExpandableItem)this).remove (item);
+            // Add back dummy if empty unless we are removing a rename item
+            if (!(item is RenameItem || has_dummy || n_children > 0)) {
+                ((Granite.Widgets.SourceList.ExpandableItem)this).add (dummy);
+                has_dummy = true;
+            }
+        }
+
+        public new void clear () {
+            ((Granite.Widgets.SourceList.ExpandableItem)this).clear ();
+            has_dummy = false;
         }
 
         private void on_changed (GLib.File source, GLib.File? dest, GLib.FileMonitorEvent event) {
-            if (!children_loaded) {
+            if (source.get_basename ().has_prefix (".goutputstream")) {
+                return; // Ignore changes due to temp files and streams
+            }
+
+            if (!children_loaded) { // No child items except dummy, child never expanded
                 /* Empty folder with dummy item will come here even if expanded */
                 switch (event) {
                     case GLib.FileMonitorEvent.DELETED:
-                        // This is a pretty intensive operation. For each file deleted, the cache will be
-                        // invalidated and recreated again, from disk. If it turns out users are seeing
-                        // slugishness or slowness when deleting a lot of files, then it might be worth
-                        // storing file.children.size in a variable and subtracting from it with every
-                        // delete
-                        file.invalidate_cache ();
-
-                        if (file.children.size == 0) {
-                            clear ();
+                        file.invalidate_cache (); //TODO Throttle if required
+                        if (expanded) {
+                            toggled ();
                         }
-
                         break;
                     case GLib.FileMonitorEvent.CREATED:
-                        if (source.query_exists () == false) {
-                            return;
-                        }
-
-                        /* Fix adding new file to expanded empty folder */
-                        if (expanded && file.children.size == 0) {
-                            file.invalidate_cache ();
-                            clear ();
-                            add_children ();
-                            children_loaded = true;
+                        file.invalidate_cache ();  //TODO Throttle if required
+                        if (expanded) {
+                            toggled ();
                         }
-
                         break;
                     case FileMonitorEvent.RENAMED:
                     case FileMonitorEvent.PRE_UNMOUNT:
@@ -272,28 +285,15 @@ namespace Scratch.FolderManager {
 
                         break;
                 }
-            } else {
+            } else { // Child has been expanded ( but could be closed now) and items loaded (or dummy)
                 // No cache invalidation is needed here because the entire state is kept in the tree
                 switch (event) {
                     case GLib.FileMonitorEvent.DELETED:
-                        var children_tmp = new Gee.ArrayList<Granite.Widgets.SourceList.Item> ();
-                        children_tmp.add_all (children);
-                        foreach (var item in children_tmp) {
-                            if (((Item) item).path == source.get_path ()) {
-                                // This is a workaround for SourceList silliness: you cannot remove an item
-                                // without it automatically selecting another one.
-
-                                view.ignore_next_select = true;
-                                remove (item);
-                                if (file.children.size == 0) {
-                                    clear ();
-                                    add (dummy);
-                                    expanded = false;
-                                    children_loaded = false;
-                                }
-
-                                view.selected = null;
-                            }
+                        // Find item corresponding to deleted file
+                        // Note may not be found if deleted file is not valid for display
+                        var path_item = find_item_for_path (source.get_path ());
+                        if (path_item != null) {
+                            remove (path_item);
                         }
 
                         break;
@@ -302,32 +302,16 @@ namespace Scratch.FolderManager {
                             return;
                         }
 
-                        // Temporary files from GLib that are present when saving a file
-                        if (source.get_basename ().has_prefix (".goutputstream")) {
-                            return;
-                        }
-
-                        var file = new File (source.get_path ());
-                        var exists = false;
-                        foreach (var item in children) {
-                            if (((Item) item).path == file.path) {
-                                exists = true;
-                                break;
-                            }
-                        }
-
-                        Item? item = null;
-
-                        if (!exists) {
+                        var path_item = find_item_for_path (source.get_path ());
+                        if (path_item == null) {
+                            var file = new File (source.get_path ());
                             if (file.is_valid_directory ()) {
-                                item = new FolderItem (file, view);
+                                path_item = new FolderItem (file, view);
                             } else if (!file.is_temporary) {
-                                item = new FileItem (file, view);
+                                path_item = new FileItem (file, view);
                             }
-                        }
 
-                        if (item != null) {
-                            add (item);
+                            add (path_item);
                         }
 
                         break;
@@ -340,7 +324,6 @@ namespace Scratch.FolderManager {
                     case FileMonitorEvent.MOVED_IN:
                     case FileMonitorEvent.MOVED_OUT:
                     case FileMonitorEvent.ATTRIBUTE_CHANGED:
-
                         break;
                 }
             }
@@ -356,6 +339,17 @@ namespace Scratch.FolderManager {
             }
         }
 
+        private FolderManager.Item? find_item_for_path (string path) {
+            foreach (var item in children) {
+                // Item could be dummy
+                if ((item is FolderManager.Item) && ((FolderManager.Item) item).path == path) {
+                    return (FolderManager.Item)item;
+                }
+            }
+
+            return null;
+        }
+
         private void on_add_new (bool is_folder) {
             if (!file.is_executable) {
                 // This is necessary to avoid infinite loop below
@@ -371,27 +365,19 @@ namespace Scratch.FolderManager {
                 new_file = file.file.get_child (("%s %d").printf (name, n));
                 n++;
             }
-
             expanded = true;
             var rename_item = new RenameItem (new_file.get_basename (), is_folder);
-            if (file.children.size == 0) {
-                clear ();  /* Remove dummy item */
-            }
-
             add (rename_item);
-
             /* Start editing after finishing signal handler */
             GLib.Idle.add (() => {
                 view.start_editing_item (rename_item);
-
                 /* Need to poll view editing as no signal is generated when canceled (Granite bug) */
                 Timeout.add (200, () => {
                     if (view.editing) {
                         return Source.CONTINUE;
                     } else {
                         var new_name = rename_item.name;
                         view.ignore_next_select = true;
-                        remove (rename_item);
                         try {
                             var gfile = file.file.get_child_for_display_name (new_name);
                             if (is_folder) {
@@ -402,10 +388,8 @@ namespace Scratch.FolderManager {
                             }
                         } catch (Error e) {
                             warning (e.message);
-                            /* Replace dummy if file creation fails */
-                            if (file.children.size == 0) {
-                                add (dummy);
-                            }
+                        } finally {
+                            remove (rename_item);
                         }
                     }
 

src/FolderManager/Item.vala

@@ -68,6 +68,8 @@ namespace Scratch.FolderManager {
                 return 1;
             }
 
+            assert (a is Item && b is Item); //Ensure more informative error message
+
             return File.compare (((Item)a).file, ((Item)b).file);
         }