Merge lp:~jeremywootten/pantheon-files/fix-1176471-folder-properties into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Show additional property information for folder selections (lp: 1176471)

This branch shows another information line when the selection consists only of folders. This line shows the how many subfolders and ordinary files were found on a deep count of the selected folder (s).

[Felipe Escoto (philip.scott)]

Empty folders show

"Contains: "

review: Needs Fixing

[lp:~jeremywootten/pantheon-files/fix-1176471-folder-properties]

2052. By Jeremy Wootten on 2016-04-06

Merge trunk to r2101

2053. By Jeremy Wootten on 2016-04-06

Deal better with empty folders

2054. By Jeremy Wootten on 2016-04-06

Hide 'Contains:' line for empty folders

[Felipe Escoto (philip.scott)]

I made some comments on code style, but now it's not appearing on my end for some reason :P

[lp:~jeremywootten/pantheon-files/fix-1176471-folder-properties]

2055. By Jeremy Wootten on 2016-04-09

Fix formatting; ensure contains label shows when appropriate

[Jeremy Wootten (jeremywootten)]

I have fixed the code format issues identified and also a previously unidentified bug causing the "Contains" line to sometimes be hidden inappropriately.

[Felipe Escoto (philip.scott)]

Perfect! Thank you :)

review: Approve

=== modified file 'src/View/PropertiesWindow.vala'
--- src/View/PropertiesWindow.vala	2016-02-28 13:08:42 +0000
+++ src/View/PropertiesWindow.vala	2016-04-09 09:23:32 +0000
@@ -254,6 +254,8 @@
     private Gtk.Widget header_title;
     private Gtk.Label type_label;
     private Gtk.Label size_label;
+    private Gtk.Label contains_label;
+    private Gtk.Label contains_key_label;
     private Gtk.Widget type_key_label;
     private string ftype; /* common type */
     private Gtk.Spinner spinner;
@@ -265,21 +267,24 @@
 
     private bool files_contain_a_directory;
 
-    private uint _folder_count;
-    private signal void folder_count_changed ();
+    private uint _uncounted_folders = 0;
+    private uint selected_folders = 0;
+    private uint selected_files = 0;
+    private signal void uncounted_folders_changed ();
 
-    private uint folder_count {
+    private uint uncounted_folders {
         get {
-            return _folder_count;
+            return _uncounted_folders;
         }
 
         set {
-            _folder_count = value;
-            folder_count_changed ();
+            _uncounted_folders = value;
+            uncounted_folders_changed ();
         }
     }
 
-    private uint file_count;
+    private uint folder_count = 0; /* Count of folders current NOT including top level (selected) folders (to match OverlayBar)*/
+    private uint file_count; /* Count of files current including top level (selected) files other than folders */
 
     public PropertiesWindow (GLib.List<GOF.File> _files, FM.AbstractDirectoryView _view, Gtk.Window parent) {
         base (_("Properties"), parent);
@@ -296,6 +301,12 @@
 
         view = _view;
 
+        /* Connect signal before creating any DeepCount directories */
+        this.destroy.connect (() => {
+            foreach (var dir in deep_count_directories)
+                dir.cancel ();
+        });
+
         /* The properties window may outlive the passed-in file object
            lifetimes. The objects must be referenced as a precaution.
 
@@ -327,9 +338,11 @@
                 critical ("Properties Window constructor called with invalid file data (2)");
                 return;
             }
+
             var ftype = gof.get_ftype ();
             if (ftype != null)
                 mimes.add (ftype);
+
             if (gof.is_directory)
                 files_contain_a_directory = true;
         }
@@ -380,25 +393,12 @@
             add_section (stack, _("Preview"), PanelType.PREVIEW.to_string (), preview_box);
         }
 
-        if (count == 1 && !view.is_in_recent ()) {
-            int start_offset= 0, end_offset = -1;
-
-            Marlin.get_rename_region (goffile.info.get_name (), out start_offset, out end_offset, goffile.is_folder ());
-            (header_title as Gtk.Entry).select_region (start_offset, end_offset);
-        }
-
-        if (folder_count == 0)
-            spinner.hide ();
-
-        if (size_warning < 1)
-            size_warning_image.hide ();
-
-        if (file_count > 1) {
-            type_key_label.hide ();
-            type_label.hide ();
-        }
-
         show_all ();
+
+        /* There is a race condition between reaching update_header_desc () or here first
+         * so call update_widgets_state in both places.
+         */
+        update_widgets_state ();
         present ();
     }
 
@@ -408,12 +408,6 @@
         string header_desc_str;
 
         header_desc_str = format_size ((int64) total_size);
-        if (ftype != null)
-            type_label.label = goffile.formated_type;
-        else {
-            type_key_label.hide ();
-            type_label.hide ();
-        }
 
         if (size_warning > 0) {
             string file_plural = _("file");
@@ -424,6 +418,8 @@
         }
 
         size_label.label = header_desc_str;
+        contains_label.label = get_contains_label (folder_count, file_count);
+        update_widgets_state ();
     }
 
     private Mutex mutex;
@@ -431,37 +427,47 @@
 
     private void selection_size_update () {
         total_size = 0;
-        deep_count_directories = null;
+        uncounted_folders = 0;
+        selected_folders = 0;
+        selected_files = 0;
         folder_count = 0;
         file_count = 0;
         size_warning = 0;
         size_warning_image.hide ();
 
+        deep_count_directories = null;
+
         foreach (GOF.File gof in files) {
             if (gof.is_root_network_folder ()) {
                 size_label.label = _("unknown");
                 continue;
             }
             if (gof.is_directory) {
-                folder_count++;
-                var d = new Marlin.DeepCount (gof.location);
+                mutex.lock ();
+                uncounted_folders++; /* this gets decremented by DeepCount*/
+                mutex.unlock ();
+
+                selected_folders++;
+                var d = new Marlin.DeepCount (gof.location); /* Starts counting on creation */
                 deep_count_directories.prepend (d);
+
                 d.finished.connect (() => {
-                                    mutex.lock ();
-                                    deep_count_directories.remove (d);
-                                    total_size += d.total_size;
-                                    size_warning = d.file_not_read;
-                                    update_header_desc ();
-                                    if (file_count + folder_count == size_warning)
-                                        size_label.label = _("unknown");
-
-                                    folder_count--;
-                                    if (!size_label.visible)
-                                        size_label.show ();
-                                    mutex.unlock ();
-                                    });
+                    mutex.lock ();
+                    deep_count_directories.remove (d);
+
+                    total_size += d.total_size;
+                    size_warning = d.file_not_read;
+                    if (file_count + uncounted_folders == size_warning)
+                        size_label.label = _("unknown");
+
+                    folder_count += d.dirs_count;
+                    file_count += d.files_count;
+                    uncounted_folders--; /* triggers signal which updates description when reaches zero */
+                    mutex.unlock ();
+                });
+
             } else {
-                file_count++;
+                selected_files++;
             }
 
             mutex.lock ();
@@ -469,24 +475,18 @@
             mutex.unlock ();
         }
 
-        if (file_count > 0)
-            update_header_desc ();
-
-        if (folder_count > 0) {
+        if (uncounted_folders > 0) {/* possible race condition - uncounted_folders could have been decremented? */
             spinner.start ();
-
-            folder_count_changed.connect (() => {
-                if (folder_count == 0) {
+            uncounted_folders_changed.connect (() => {
+                if (uncounted_folders == 0) {
                     spinner.hide ();
                     spinner.stop ();
+                    update_header_desc ();
                 }
             });
+        } else {
+            update_header_desc ();
         }
-
-        this.destroy.connect (() => {
-            foreach (var dir in deep_count_directories)
-                dir.cancel ();
-        });
     }
 
     private void rename_file (GOF.File file, string new_name) {
@@ -520,12 +520,36 @@
     }
 
     private void build_header_box (Gtk.Box content) {
+        /* create some widgets first (may be hidden by selection_size_update ()) */
         var file_pix = goffile.get_icon_pixbuf (48, false, GOF.FileIconFlags.NONE);
         var file_img = new Gtk.Image.from_pixbuf (overlay_emblems (file_pix, goffile.emblems_list));
 
+        spinner = new Gtk.Spinner ();
+        spinner.set_hexpand (false);
+        spinner.halign = Gtk.Align.START;
+
+        size_warning_image = new Gtk.Image.from_icon_name ("help-info-symbolic", Gtk.IconSize.MENU);
+        size_warning_image.halign = Gtk.Align.START;
+        size_warning_image.no_show_all = true;
+        size_label = new Gtk.Label ("");
+        size_label.set_hexpand (false);
+
+        type_label = new Gtk.Label ("");
+        type_label.set_halign (Gtk.Align.START);
+        type_key_label = new Gtk.Label (_("Type:"));
+        type_key_label.halign = Gtk.Align.END;
+
+        contains_label = new Gtk.Label ("");
+        contains_label.set_halign (Gtk.Align.START);
+        contains_key_label = new Gtk.Label (_("Contains:"));
+        contains_key_label.set_halign (Gtk.Align.END);
+
+        selection_size_update (); /* Start counting first to get number of selected files and folders */
+
+        /* Build header box */
         if (count > 1 || (count == 1 && !goffile.is_writable ())) {
             var label = new Gtk.Label ("");
-            label.set_markup ("<span>" + _("%u selected items").printf(count) + "</span>");
+            label.set_markup ("<span>" + get_selected_label (selected_folders, selected_files) + "</span>");
             label.set_halign (Gtk.Align.START);
             header_title = label;
         } else if (count == 1 && goffile.is_writable ()) {
@@ -545,28 +569,6 @@
         }
 
         pack_header_box (file_img, header_title);
-
-        /* The header box is ready, now let's build some widgets that are going
-         * to be updated by selection_size_update() while the rest of the UI is
-         * being built. */
-        type_label = new Gtk.Label ("");
-        type_label.set_halign (Gtk.Align.START);
-
-        size_label = new Gtk.Label ("");
-        size_label.set_hexpand (false);
-
-        type_key_label = new Gtk.Label (_("Type:"));
-        type_key_label.halign = Gtk.Align.END;
-
-        spinner = new Gtk.Spinner ();
-        spinner.set_hexpand (false);
-        spinner.halign = Gtk.Align.START;
-
-        size_warning_image = new Gtk.Image.from_icon_name ("help-info-symbolic", Gtk.IconSize.MENU);
-        size_warning_image.halign = Gtk.Align.START;
-        size_warning_image.no_show_all = true;
-
-        selection_size_update ();
     }
 
     private string? get_common_ftype () {
@@ -652,9 +654,23 @@
                     info.add (new Pair<string, string>(_("Deleted:"), deletion_date));
             }
         }
+
         ftype = get_common_ftype ();
         if (ftype != null) {
             info.add (new Pair<string, string>(_("MimeType:"), ftype));
+            /* get image size in pixels using an asynchronous method to stop the interface blocking on
+             * large images. */
+            if ("image" in ftype) {
+                string resolution_value = "";
+                if (file.width > 0) { /* resolution has already been determined */
+                    resolution_value = goffile.width.to_string () +" × " + goffile.height.to_string () + " px";
+                } else {
+                    resolution_value = _("loading ...");
+                    /* Async function will update info when resolution determined */
+                    get_resolution.begin (file, info);
+                }
+                info.add (new Pair<string, string> (resolution_key, resolution_value));
+            }
         } else {
             /* show list of mimetypes only if we got a default application in common */
             if (view.get_default_app () != null && !goffile.is_directory) {
@@ -666,19 +682,7 @@
             }
         }
 
-        /* get image size in pixels using an asynchronous method to stop the interface blocking on
-         * large images. */
-        if ("image" in ftype) {
-            string resolution_value = "";
-            if (file.width > 0) { /* resolution has already been determined */
-                resolution_value = goffile.width.to_string () +" × " + goffile.height.to_string () + " px";
-            } else {
-                resolution_value = _("loading ...");
-                /* Async function will update info when resolution determined */
-                get_resolution.begin (file, info);
-            }
-            info.add (new Pair<string, string> (resolution_key, resolution_value));
-        }
+
 
         if (got_common_location ()) {
             if (view.is_in_recent ()) {
@@ -756,12 +760,13 @@
         size_key_label.halign = Gtk.Align.END;
 
         var size_box = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 4);
+        size_box.pack_start (spinner, false, false);
         size_box.pack_start (size_label, false, true);
-        size_box.pack_start (spinner, false, false);
         size_box.pack_start (size_warning_image);
 
         create_info_line (size_key_label, size_label, information, ref n, size_box);
         create_info_line (type_key_label, type_label, information, ref n);
+        create_info_line (contains_key_label, contains_label, information, ref n);
 
         foreach (var pair in item_info) {
             var value_label = new Gtk.Label (pair.value);
@@ -1465,6 +1470,119 @@
         }
         return file_size;
     }
+
+    private string get_contains_label (uint folders, uint files) {
+        string txt = "";
+        if (folders > 0) {
+            if (folders > 1) {
+                if (files > 0) {
+                    if (files > 1) {
+                        txt = _("%u subfolders and %u files").printf (folders, files);
+                    } else {
+                        txt = _("%u subfolders and %u file").printf (folders,files);
+                    }
+                } else {
+                    txt = _("%u subfolders").printf (folders);
+                }
+            } else {
+                if (files > 0) {
+                    if (files > 1) {
+                        txt = _("%u subfolder and %u files").printf (folders, files);
+                    } else {
+                        txt = _("%u subfolder and %u file").printf (folders, files);
+                    }
+                } else {
+                    txt = _("%u folder").printf (folders);
+                }
+            }
+        } else {
+            if (files > 0) {
+                if (files > 1) {
+                    txt = _("%u files").printf (files);
+                } else {
+                    txt = _("%u file").printf (files);
+                }
+            }
+        }
+
+        return txt;
+    }
+
+    private string get_selected_label (uint folders, uint files) {
+        string txt = "";
+        uint total = folders + files;
+
+        if (folders > 0) {
+            if (folders > 1) {
+                if (files > 0) {
+                    if (files > 1) {
+                        txt = _("%u selected items (%u folders and %u files)").printf (total, folders, files);
+                    } else {
+                        txt = _("%u selected items (%u folders and %u file)").printf (total, folders, files);
+                    }
+                } else {
+                    txt = _("%u folders").printf (folders);
+                }
+            } else {
+                if (files > 0) {
+                    if (files > 1) {
+                        txt = _("%u selected items (%u folder and %u files)").printf (total, folders,files);
+                    } else {
+                        txt = _("%u selected items (%u folder and %u file)").printf (total, folders,files);
+                    }
+                } else {
+                    txt = _("%u folder").printf (folders); /* displayed for background folder*/
+                }
+            }
+        } else {
+            if (files > 0) {
+                if (files > 1) {
+                    txt = _("%u files").printf (files);
+                } else {
+                    txt = _("%u file").printf (files); /* should not be displayed - entry instead */
+                }
+            }
+        }
+
+        /* The selection should never be empty */
+        return txt;
+    }
+
+    /** Hide certain widgets under certain conditions **/
+    private void update_widgets_state () {
+        if (uncounted_folders == 0)
+            spinner.hide ();
+
+        if (size_warning < 1)
+            size_warning_image.hide ();
+
+        if (ftype != null) {
+            type_label.label = goffile.formated_type;
+        }
+
+        if (count > 1) {
+            type_key_label.hide ();
+            type_label.hide ();
+        }
+
+        if ((header_title is Gtk.Entry) && !view.is_in_recent ()) {
+            int start_offset= 0, end_offset = -1;
+
+            Marlin.get_rename_region (goffile.info.get_name (), out start_offset, out end_offset, goffile.is_folder ());
+            (header_title as Gtk.Entry).select_region (start_offset, end_offset);
+        }
+
+        /* Only show 'contains' label when only folders selected - otherwise could be ambiguous whether
+         * the "contained files" counted are only in the subfolders or not.*/
+        /* Only show 'contains' label when folders selected are not empty */
+        if (count > selected_folders || contains_label.get_text ().length < 1) {
+            contains_key_label.hide ();
+            contains_label.hide ();
+        } else { /* Make sure it shows otherwise (may have been hidden by previous call)*/
+            contains_key_label.show ();
+            contains_label.show ();
+        }
+    }
 }
 
 public class Marlin.View.VolumePropertiesWindow : Marlin.View.PropertiesWindowBase {