Merge lp:~jeremywootten/pantheon-files/fix-1086929-remake into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Implement auto-sizing of columns in column view.

This is a remake of my previous branch to implement column auto-sizing in column view (currently owned by elementary Developer Community), which included a lot of extraneous formatting changes. In this branch, any changes to existing code have been minimised and it is based on a later revision of trunk. The functional code is the same as the previous branch.

[David Gomes (davidgomes)]

Newline after diff line 58, 82.

167-169, align those correctly.

Good branch, I can confirm the core code is the same, so we can merge this. Plus, it's much much less code, which is good.

review: Needs Fixing

[David Gomes (davidgomes)]

Newline after diff line 37 too.

[Jeremy Wootten (jeremywootten)]

OK. So the rule is new line after one-line if statements unless followed
by a brace?

What is the preferred way of aligning 167-169? Align under "g_strconcat"
or the opening paren or elsewhere?

On 27 September 2013 10:52, David Gomes <email address hidden> wrote:

> Review: Needs Fixing
>
> Newline after diff line 58, 82.
>
> 167-169, align those correctly.
>
> Good branch, I can confirm the core code is the same, so we can merge
> this. Plus, it's much much less code, which is good.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-files/fix-1086929-remake/+merge/188004
> You are the owner of lp:~jeremywootten/pantheon-files/fix-1086929-remake.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Jeremy Wootten (jeremywootten)]

Re aligning; looking again I guess you mean under the first quote (")?

On 27 September 2013 10:52, David Gomes <email address hidden> wrote:

> Review: Needs Fixing
>
> Newline after diff line 58, 82.
>
> 167-169, align those correctly.
>
> Good branch, I can confirm the core code is the same, so we can merge
> this. Plus, it's much much less code, which is good.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-files/fix-1086929-remake/+merge/188004
> You are the owner of lp:~jeremywootten/pantheon-files/fix-1086929-remake.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[David Gomes (davidgomes)]

Regarding aligning, yes I mean under the first quote.

The rule concerning new lines is after any if, for/while/foreach/function block, except if followed by another "}" (for example in the end of a class when you close the class and the namespace and the last function in the class). Be it one-line or multiple-ĺines.

[Jeremy Wootten (jeremywootten)]

OK, fixed accordingly.

On 27 September 2013 11:50, David Gomes <email address hidden> wrote:

> Regarding aligning, yes I mean under the first quote.
>
> The rule concerning new lines is after any if, for/while/foreach/function
> block, except if followed by another "}" (for example in the end of a class
> when you close the class and the namespace and the last function in the
> class). Be it one-line or multiple-ĺines.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-files/fix-1086929-remake/+merge/188004
> You are the owner of lp:~jeremywootten/pantheon-files/fix-1086929-remake.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Cody Garver (codygarver)]

I think I found a bug with this. When Trash has some amount of files in it (more than 1, or a certain size, but not sure how much), emptying causes pantheon-files to lock up.

I have experienced this twice, still trying to figure out how to reproduce.

[Jeremy Wootten (jeremywootten)]

Just emptied my trash of 1000+ files successfully but it was fairly slow.
 It could be that the longest filename (which detemines the column width)
is being updated after every deletion. I'll look into it.

On 2 October 2013 18:02, Cody Garver <email address hidden> wrote:

> I think I found a bug with this. When Trash has some amount of files in it
> (more than 1, or a certain size, but not sure how much), emptying causes
> pantheon-files to lock up.
>
> I have experienced this twice, still trying to figure out how to reproduce.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-files/fix-1086929-remake/+merge/188004
> You are the owner of lp:~jeremywootten/pantheon-files/fix-1086929-remake.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[lp:~jeremywootten/pantheon-files/fix-1086929-remake]

1346. By Jeremy Wootten on 2013-10-06

Merge changes from trunk rev 1351

1347. By Jeremy Wootten on 2013-10-09

Only autosize after all changes to directory done

[Jeremy Wootten (jeremywootten)]

Hello Cody, There does seem to be an issue with deleting or copying large
numbers of files (especially image) files - it can be slow and there is no
progress information so the application appears frozen. I have pushed
another revision to this branch that only autosizes the column after all
deletions are done rather than after each deletion. Please review this
revision, thanks.

There are some other efficiencies that can be made but they are not
connected to autosizing so I have not put them in this branch.

On 2 October 2013 18:02, Cody Garver <email address hidden> wrote:

> I think I found a bug with this. When Trash has some amount of files in it
> (more than 1, or a certain size, but not sure how much), emptying causes
> pantheon-files to lock up.
>
> I have experienced this twice, still trying to figure out how to reproduce.
> --
>
> https://code.launchpad.net/~jeremywootten/pantheon-files/fix-1086929-remake/+merge/188004
> You are the owner of lp:~jeremywootten/pantheon-files/fix-1086929-remake.
>

--
Jeremy Wootten
GPG Key ID CB585BCD
Key Fingerprint 37C0 3C2A A6D4 E45B BA7C 4328 2DF2 1882 CB58 5BCD

[Cody Garver (codygarver)]

I gathered a big bunch of Trash to do delete tests with and found the problems I previously had with this branch are resolved.

I want to get lp:~jeremywootten/pantheon-files/fix-1178869 merged, do a point release, then merge this branch so it gets some time in trunk for extended testing.

review: Approve

=== modified file 'libcore/gof-directory-async.vala'
--- libcore/gof-directory-async.vala	2013-09-11 19:36:05 +0000
+++ libcore/gof-directory-async.vala	2013-10-09 18:52:53 +0000
@@ -28,6 +28,8 @@
     /* we're looking for particular path keywords like *\/icons* .icons ... */
     public bool uri_contain_keypath_icons;
     public int uri_keypath_size = 0;
+    public string longest_file_name = ""; //for auto-sizing Miller columns
+    public bool track_longest_name;
 
     public enum State {
         NOT_LOADED,
@@ -75,7 +77,7 @@
         file = GOF.File.get (location);
         file.exists = true;
         cancellable = new Cancellable ();
-
+        track_longest_name = false;
         //query_info_async (file, file_info_available);
         if (file.info == null)
             file.query_update ();
@@ -139,6 +141,7 @@
 
     public void load () {
         cancellable.reset ();
+        longest_file_name = "";
         if (state != State.LOADED) {
             /* clear directory info if it's not fully loaded */
             if (state == State.LOADING)
@@ -170,22 +173,36 @@
 
             bool show_hidden = Preferences.get_default ().pref_show_hidden_files;
             foreach (GOF.File gof in file_hash.get_values ()) {
-                if (gof.info != null && (!gof.is_hidden || show_hidden))
+                if (gof.info != null && (!gof.is_hidden || show_hidden)) {
+                    if (track_longest_name)
+                        update_longest_file_name (gof);
+
                     file_loaded (gof);
+                }
             }
             done_loading ();
         }
     }
 
+    private void update_longest_file_name (GOF.File gof) {
+        if (longest_file_name.length < gof.basename.length)
+            longest_file_name = gof.basename;
+    }
+
     public void load_hiddens () {
         if (state != State.LOADED) {
             load ();
         } else {
             foreach (GOF.File gof in file_hash.get_values ()) {
-                if (gof != null  && gof.info != null && gof.is_hidden)
+                if (gof != null && gof.info != null && gof.is_hidden) {
+                    if (track_longest_name)
+                        update_longest_file_name (gof);
+
                     file_loaded (gof);
+                }
             }
         }
+        done_loading ();
     }
 
     public void update_desktop_files () {
@@ -232,6 +249,7 @@
                 if (files == null)
                     break;
 
+                bool show_hidden =  Preferences.get_default ().pref_show_hidden_files;
                 foreach (var file_info in files) {
                     GLib.File loc = location.get_child ((string) file_info.get_name ());
                     //GOF.File gof;
@@ -244,8 +262,12 @@
                     //debug ("file: %s", gof.name);
 
                     add_to_hash_cache (gof);
-                    if (!gof.is_hidden || Preferences.get_default ().pref_show_hidden_files)
+                    if (!gof.is_hidden || show_hidden) {
+                        if (track_longest_name)
+                            update_longest_file_name (gof);
+
                         file_loaded (gof);
+                    }
 
                     //mutex.lock ();
                     files_count++;
@@ -318,6 +340,11 @@
             /* add to sorted_dirs */
             sorted_dirs.insert_sorted (gof, GOF.File.compare_by_display_name);
         }
+
+        if (track_longest_name && gof.basename.length > longest_file_name.length) {
+            longest_file_name = gof.basename;
+            done_loading ();
+        }
     }
 
     private void file_info_available (GOF.File gof) {
@@ -408,8 +435,16 @@
                     need_reload ();
                 } else {
                     list_fchanges.reverse ();
+                    bool tln = track_longest_name;
+                    /* do not autosize during multiple changes */
+                    track_longest_name = false;
                     foreach (var fchange in list_fchanges)
                         real_directory_changed (fchange.file, null, fchange.event);
+
+                    if (tln) {
+                        track_longest_name = true;
+                        load ();
+                    }
                 }
             }
             list_fchanges_count = 0;
@@ -438,6 +473,9 @@
     }
 
     public static void notify_files_removed (List<GLib.File> files) {
+        List<Async> dirs = null;
+        bool found;
+
         foreach (var loc in files) {
             GOF.File gof = GOF.File.get (loc);
             Async? dir = cache_lookup (gof.directory);
@@ -445,8 +483,20 @@
             if (dir != null) {
                 //message ("notify removed %s", gof.uri);
                 dir.notify_file_removed (gof);
+                found = false;
+                foreach (var d in dirs) {
+                    if (d == dir)
+                        found = true;
+                }
+                if (!found)
+                    dirs.append (dir);
             }
         }
+
+        foreach (var d in dirs) {
+            if (d.track_longest_name)
+                d.load();
+        }
     }
 
     public static Async from_gfile (GLib.File file) {
@@ -578,7 +628,7 @@
         return null;
     }
 
-    private int icon_size;
+    public int icon_size;
     public void threaded_load_thumbnails (int size) {
         try {
             icon_size = size;

=== modified file 'libcore/gof-file.c'
--- libcore/gof-file.c	2013-08-10 20:20:23 +0000
+++ libcore/gof-file.c	2013-10-09 18:52:53 +0000
@@ -829,7 +829,7 @@
     }
 
     file->is_gone = TRUE;
-    g_warning ("end %s", G_STRFUNC);
+    //g_warning ("end %s", G_STRFUNC);
 }
 
 static void gof_file_init (GOFFile *file) {

=== modified file 'src/View/ViewContainer.vala'
--- src/View/ViewContainer.vala	2013-07-21 04:44:33 +0000
+++ src/View/ViewContainer.vala	2013-10-09 18:52:53 +0000
@@ -207,6 +207,7 @@
             }
             if (slot != null && slot.directory != null && slot.directory.file.exists) {
                 slot.directory.cancel ();
+                slot.directory.track_longest_name = false;
             }
 
             if (nview == ViewMode.MILLER) {

=== modified file 'src/fm-columns-view.c'
--- src/fm-columns-view.c	2013-09-09 03:08:16 +0000
+++ src/fm-columns-view.c	2013-10-09 18:52:53 +0000
@@ -79,9 +79,7 @@
     if (gof_directory_async_is_empty (dir))
     {
         PangoLayout* layout = gtk_widget_create_pango_layout(GTK_WIDGET(view), NULL);
-        gchar *str = g_strconcat("<span size='x-large'>", _("This folder is empty."), "</span>", NULL);
-        pango_layout_set_markup (layout, str, -1);
-
+        pango_layout_set_markup (layout, FM_DIRECTORY_VIEW (view)->empty_message, -1);
         PangoRectangle extents;
         /* Get hayout height and width */
         pango_layout_get_extents(layout, NULL, &extents);

=== modified file 'src/fm-directory-view.c'
--- src/fm-directory-view.c	2013-09-29 17:01:20 +0000
+++ src/fm-directory-view.c	2013-10-09 18:52:53 +0000
@@ -316,6 +316,12 @@
     g_object_get (view, "zoom-level", &zoom, NULL);
     int size = marlin_zoom_level_to_icon_size (zoom);
     gof_directory_async_threaded_load_thumbnails (view->details->slot->directory, size);
+    /* If in Miller view, autosize the column */
+    if (view->details->slot->ready_to_autosize)
+        autosize_slot (view->details->slot);
+    else
+        view->details->slot->ready_to_autosize = TRUE;
+
     //g_signal_emit (view, signals[DIRECTORY_LOADED], 0, directory);
 }
 
@@ -501,6 +507,10 @@
     view->details->dir_merge_id = 0;
     view->details->open_with_action_group = NULL;
     view->details->open_with_merge_id = 0;
+    view->empty_message = g_strconcat ("<span size='x-large'>",
+                                       _("This folder is empty."),
+                                       "</span>",
+                                       NULL);
 }
 
 static GObject*
@@ -2272,7 +2282,7 @@
     g_return_if_fail (FM_IS_DIRECTORY_VIEW (view));
     g_return_if_fail (view->model == model);
 
-    g_message ("%s", G_STRFUNC);
+    g_debug ("%s", G_STRFUNC);
     /* Get tree paths of selected files */
     selected_paths = (*FM_DIRECTORY_VIEW_GET_CLASS (view)->get_selected_paths) (view);
 
@@ -2307,7 +2317,7 @@
     g_return_if_fail (FM_IS_DIRECTORY_VIEW (view));
     g_return_if_fail (view->model == model);
 
-    g_message ("%s", G_STRFUNC);
+    g_debug ("%s", G_STRFUNC);
     /* Check if there was only one file selected before the row was deleted. The
      * path is set by thunar_standard_view_row_deleted() if this is the case */
     if (G_LIKELY (view->details->selection_before_delete != NULL))
@@ -3115,7 +3125,7 @@
 void
 fm_directory_view_set_active_slot (FMDirectoryView *view)
 {
-    g_warning ("%s %s %s", G_STRFUNC,
+    g_debug ("%s %s %s", G_STRFUNC,
                view->details->slot->mwcols->active_slot->directory->file->uri,
                view->details->slot->directory->file->uri
                );
@@ -3124,7 +3134,7 @@
     if (view->details->slot->mwcols->active_slot == view->details->slot)
         return;
 
-    g_warning ("%s", G_STRFUNC);
+    //g_warning ("%s", G_STRFUNC);
     gof_window_slot_active (view->details->slot);
 
     /* make sure to grab focus as right click menus don't automaticly get it */

=== modified file 'src/fm-directory-view.h'
--- src/fm-directory-view.h	2013-08-10 20:15:26 +0000
+++ src/fm-directory-view.h	2013-10-09 18:52:53 +0000
@@ -65,6 +65,7 @@
     GtkCellRenderer         *name_renderer;
     MarlinZoomLevel         zoom_level;
     gboolean                updates_frozen;
+    const gchar* empty_message;
 
     FMDirectoryViewDetails  *details;
 };

=== modified file 'src/gof-window-slot.c'
--- src/gof-window-slot.c	2013-08-10 20:15:26 +0000
+++ src/gof-window-slot.c	2013-10-09 18:52:53 +0000
@@ -46,6 +46,7 @@
 {
     slot->content_box = gtk_box_new (GTK_ORIENTATION_VERTICAL, 0);
     slot->width = 0;
+    slot->ready_to_autosize = FALSE;
     GOF_ABSTRACT_SLOT (slot)->extra_location_widgets = gtk_box_new (GTK_ORIENTATION_VERTICAL, 0);
     gtk_box_pack_start (GTK_BOX (slot->content_box), GOF_ABSTRACT_SLOT(slot)->extra_location_widgets, FALSE, FALSE, 0);
 }
@@ -89,6 +90,7 @@
     //load_dir_async_cancel(slot->directory);
     g_debug ("%s %s\n", G_STRFUNC, slot->directory->file->uri);
     //g_warning ("%s %s %u\n", G_STRFUNC, slot->directory->file->uri, G_OBJECT (slot->directory)->ref_count);
+    g_signal_handlers_disconnect_by_data (slot->directory, slot);
     g_object_unref(slot->directory);
     g_object_unref(slot->location);
     G_OBJECT_CLASS (parent_class)->finalize (object);
@@ -131,6 +133,68 @@
 
     gtk_paned_pack1 (GTK_PANED (hpane), column, FALSE, FALSE);
     gtk_paned_pack2 (GTK_PANED (hpane), box1, TRUE, FALSE);
+
+    /* If the directory finished loading before slot was ready then autosize the slot now.
+    * Otherwise the slot will be autosized by the directory_done_loading callback
+    * This is necessary because the directory loads faster from the cache than from disk
+    * On first use the directory loads from disk and we reach here before the directory
+    * has finished loading.
+    * On subsequent uses, the directory loads from cache before the slot is ready.
+    * Whichever finishes first sets slot->ready_to_autosize = TRUE
+    * Whichever finds slot->ready_to_autosize = TRUE does the autosizing.
+    */
+
+    if (slot->ready_to_autosize)
+        autosize_slot (slot);
+    else
+        slot->ready_to_autosize = TRUE;
+}
+
+void autosize_slot (GOFWindowSlot *slot)
+{
+    g_return_if_fail (GOF_IS_WINDOW_SLOT (slot));
+    g_return_if_fail (slot->view_box != NULL);
+    g_return_if_fail (GTK_IS_WIDGET (slot->view_box));
+    g_return_if_fail (GOF_DIRECTORY_IS_ASYNC (slot->directory));
+    g_return_if_fail (slot->mwcols != NULL);
+
+    PangoLayout* layout = gtk_widget_create_pango_layout (GTK_WIDGET (slot->view_box), NULL);
+
+    if (gof_directory_async_is_empty (slot->directory))
+        pango_layout_set_markup (layout, FM_DIRECTORY_VIEW (slot->view_box)->empty_message, -1);
+    else
+        pango_layout_set_markup (layout,
+                                 g_markup_escape_text (slot->directory->longest_file_name, -1),
+                                 -1);
+
+    PangoRectangle extents;
+    pango_layout_get_extents (layout, NULL, &extents);
+
+    gint column_width = (int) pango_units_to_double(extents.width)
+                        + 2 * slot->directory->icon_size
+                        + 2 * slot->mwcols->handle_size
+                        + 12;
+
+    gint min_width = slot->mwcols->preferred_column_width / 2;
+
+    if (column_width < min_width)
+        column_width = min_width;
+    else {
+        //TODO make max_width a setting
+        gint max_width = 2 * slot->mwcols->preferred_column_width;
+        if (column_width > max_width)
+            column_width = max_width;
+    }
+
+    if (slot->slot_number == 0)
+        gtk_widget_set_size_request (gtk_paned_get_child1 (GTK_PANED (slot->hpane)), column_width, -1);
+    else
+        gtk_paned_set_position (GTK_PANED (slot->hpane), column_width);
+
+    slot->width = column_width;
+    gtk_widget_show_all (slot->mwcols->colpane);
+    gtk_widget_queue_draw (slot->mwcols->colpane);
+
 }
 
 void
@@ -157,9 +221,11 @@
         g_list_free (slot->mwcols->slot);
         slot->mwcols->slot = l;
     }
-    slot->mwcols->total_width += slot->mwcols->preferred_column_width + 100;
+
+    marlin_window_columns_add (slot->mwcols, location);
+
+    slot->mwcols->total_width += slot->width + 100;
     gtk_widget_set_size_request (slot->mwcols->colpane, slot->mwcols->total_width, -1);
-    marlin_window_columns_add (slot->mwcols, location);
 }
 
 GOFWindowSlot *
@@ -193,6 +259,7 @@
     gtk_box_pack_start(GTK_BOX (slot->content_box), slot->view_box, TRUE, TRUE, 0);
 
     marlin_view_view_container_set_content ((MarlinViewViewContainer *) slot->ctab, slot->content_box);
+    slot->directory->track_longest_name = FALSE;
     gof_directory_async_load (slot->directory);
 }
 
@@ -207,6 +274,7 @@
                                                "window-slot", slot, NULL));
     gtk_box_pack_start (GTK_BOX (slot->content_box), slot->view_box, TRUE, TRUE, 0);
     marlin_view_view_container_set_content ((MarlinViewViewContainer *) slot->ctab, slot->content_box);
+    slot->directory->track_longest_name = FALSE;
     gof_directory_async_load (slot->directory);
 }
 
@@ -224,6 +292,7 @@
 {
     slot->view_box = GTK_WIDGET (g_object_new (FM_TYPE_COLUMNS_VIEW,
                                                "window-slot", slot, NULL));
+    slot->directory->track_longest_name = TRUE;
     gof_directory_async_load (slot->directory);
 }
 
@@ -233,8 +302,10 @@
 {
     g_return_if_fail (GOF_IS_WINDOW_SLOT (slot));
 
-    if (slot->mwcols)
-        marlin_window_columns_active_slot (slot->mwcols, slot);
+    if (slot->mwcols != NULL) {
+         marlin_window_columns_active_slot (slot->mwcols, slot);
+        autosize_slot (slot);
+    }
 }
 
 void

=== modified file 'src/gof-window-slot.h'
--- src/gof-window-slot.h	2013-06-27 19:34:48 +0000
+++ src/gof-window-slot.h	2013-10-09 18:52:53 +0000
@@ -55,6 +55,8 @@
 
     MarlinWindowColumns *mwcols;
     guint width;
+    guint slot_number;
+    gboolean ready_to_autosize;
 };
 
 struct GOFWindowSlotClass {
@@ -81,5 +83,6 @@
 
 void            gof_window_slot_freeze_updates (GOFWindowSlot *slot);
 void            gof_window_slot_unfreeze_updates (GOFWindowSlot *slot);
+void            autosize_slot (GOFWindowSlot *slot);
 
 #endif /* GOF_WINDOW_SLOT_H */

=== modified file 'src/marlin-window-columns.c'
--- src/marlin-window-columns.c	2013-07-05 18:20:51 +0000
+++ src/marlin-window-columns.c	2013-10-09 18:52:53 +0000
@@ -110,6 +110,7 @@
     GOFWindowSlot *slot = mwcols->active_slot;
 
     gof_window_slot_make_column_view (slot);
+    slot->slot_number = 0;
 
     mwcols->colpane = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 0);
     slot->colpane = mwcols->colpane;
@@ -156,7 +157,9 @@
 marlin_window_columns_add (MarlinWindowColumns *mwcols, GFile *location)
 {
     GOFWindowSlot *slot = gof_window_slot_new (location, mwcols->ctab);
+    slot->width = mwcols->preferred_column_width;
     gof_window_slot_make_column_view (slot);
+    slot->slot_number = mwcols->active_slot->slot_number + 1;
     slot->mwcols = mwcols;
     slot->colpane = mwcols->active_slot->colpane;
     gof_window_column_add (slot, slot->view_box);