Merge lp:~azzar1/unity-lens-files/add-filesystem-icons into lp:unity-lens-files

[Andrea Azzarone]

Add filesystem icons in the "Folders" category of the files lens.

Manual test added. It's my first time with Vala so forgive me for my errors :)

[lp:~azzar1/unity-lens-files/add-filesystem-icons]

234. By Andrea Azzarone on 2012-08-18

Change uri prefix.

[Paweł Stołowski (stolowski)]

Hi Andrea, nice job, it works well and looks good, however it's missing support for recently introduced previews - if you right-click on a device icon in the dash, an empty preview is displayed and an error is printed to the stderr:

(process:3255): unity-files-daemon-WARNING **: daemon.vala:1042: Couldn't stat file: 'device://099A-74A7'

Could you please check with design what would they like to see as a device preview and implement the missing bits in the preview(..) method?

Manual tests should now follow the format requested by QA - see TEST_TEMPLATE.txt in unity source code. I know existing tests don't follow it (and need to be fixed), but when we introduce new tests file we should stick to the new format.

Also some minor comments:

This can fail and throw GLib.Error:
415 + AppInfo.launch_default_for_uri (get_volume_uri (), null);

Formatting issues (spaces missing):
363 + public Volume volume {get; set construct; }

378 + Object (volume:volume, uri:"device://"+uuid, icon:icon_name, display_name:name, dnd_uri:"device://"+uuid);

review: Needs Fixing

[Andrea Azzarone (azzar1)]

> Hi Andrea, nice job, it works well and looks good, however it's missing
> support for recently introduced previews - if you right-click on a device icon
> in the dash, an empty preview is displayed and an error is printed to the
> stderr:
>
> (process:3255): unity-files-daemon-WARNING **: daemon.vala:1042: Couldn't stat
> file: 'device://099A-74A7'
>
> Could you please check with design what would they like to see as a device
> preview and implement the missing bits in the preview(..) method?

Hey thank you for the review and sorry for the delay (I was quite busy).
I'll add preview support (already talked with John Lea about it).

>
> Manual tests should now follow the format requested by QA - see
> TEST_TEMPLATE.txt in unity source code. I know existing tests don't follow it
> (and need to be fixed), but when we introduce new tests file we should stick
> to the new format.
>

Will do.

> Also some minor comments:
>
> This can fail and throw GLib.Error:
> 415 + AppInfo.launch_default_for_uri (get_volume_uri (), null);
>
>
> Formatting issues (spaces missing):
> 363 + public Volume volume {get; set construct; }
>
> 378 + Object (volume:volume, uri:"device://"+uuid, icon:icon_name,
> display_name:name, dnd_uri:"device://"+uuid);

Will fix.

[lp:~azzar1/unity-lens-files/add-filesystem-icons]

235. By Andrea Azzarone on 2012-09-05

Merge trunk.

236. By Andrea Azzarone on 2012-09-05

Add space.

237. By Andrea Azzarone on 2012-09-05

Fix manual test.

238. By Andrea Azzarone on 2012-09-06

Add preview support for file system icons.

239. By Andrea Azzarone on 2012-09-06

Fix warnings.

[Andrea Azzarone (azzar1)]

*poke*

[Paweł Stołowski (stolowski)]

The preview looks fine, but I found a general problem with how uri for device item is constructed: if you plug a device with more than 1 partition, you get same uris for all partitions. I've a usb stick with 2 partitions (each partition has a different name though), and get 2 "device://0012-D687" uris for them. Left clicking on such item in the dash will always activate the same partition; also preview navigation (left/right arrow in the preview) is misbehaving in such case.

review: Needs Fixing

[Andrea Azzarone (azzar1)]

Maybe we can use mount point to build the uri. It should be easy enough to change.

[lp:~azzar1/unity-lens-files/add-filesystem-icons]

240. By Andrea Azzarone on 2012-09-17

Use mount point as IDs.

[Andrea Azzarone (azzar1)]

Ok I've updated the branch to use mount point as IDs. I need to update unity trunk too.

[Paweł Stołowski (stolowski)]

yes, that sounds like a good fix.

[Paweł Stołowski (stolowski)]

Yay, it works now! Thanks!

review: Approve

[Andrea Azzarone (azzar1)]

MMM we cannot use the mount point because if the volume is not mounted volume.get_activation_root() always return null (it shouldn't, but it does here). Btw I've a pen drive with two partitions and they have two different uris.

[Paweł Stołowski (stolowski)]

Uh, how about falling back to volume name or something else?
My pendrive may be a special case as it has two *factory* made partitions that you cannot remove (it's a MySQL-branded usb stick I got during last UDS).

[Andrea Azzarone (azzar1)]

Let's try with label :)

=== modified file 'src/daemon.vala'
--- src/daemon.vala	2012-09-13 15:01:52 +0000
+++ src/daemon.vala	2012-09-17 19:57:19 +0000
@@ -34,6 +34,7 @@
     private Zeitgeist.Monitor monitor;
 
     private Bookmarks bookmarks;
+    private Devices devices;
     private UrlChecker urls;
 
     private Unity.Lens lens;
@@ -94,6 +95,13 @@
 
       bookmarks = new Bookmarks ();
       urls = new UrlChecker ();
+ 
+      devices = new Devices ();
+      devices.changed.connect (() => {
+        /* make sure our results are fresh */
+        scope.queue_search_changed (SearchType.DEFAULT);
+        scope.queue_search_changed (SearchType.GLOBAL);
+      });
 
       /* Listen for filter changes */
       scope.filters_changed.connect (() => {
@@ -453,6 +461,18 @@
                                               0, int64.MAX, result_flags,
                                               bookmark_uris, category_id);
 
+        if (has_search)
+        {
+          GLib.List<Device> matching_devices;
+
+          if (has_search)
+            matching_devices = devices.search (search.search_string);
+          else
+            matching_devices = devices.list ();
+
+          append_devices (matching_devices, results_model, Categories.FILES_AND_FOLDERS);
+        }
+
         /* Add downloads catagory if we don't have a search */
         if (has_search == false)
         {
@@ -603,6 +623,16 @@
                                                 min_size, max_size,
                                                 ResultFlags.USE_ORIGIN,
                                                 bookmark_uris);
+
+          GLib.List<Device> matching_devices;
+ 
+          if (has_search)
+            matching_devices = devices.search (search.search_string);
+          else
+            matching_devices = devices.list ();
+
+          append_devices (matching_devices, results_model,
+                          Categories.FOLDERS);
         }
         else
         {
@@ -799,6 +829,18 @@
       }
     }
 
+    private void append_devices (GLib.List<Device> devices,
+                                 Dee.Model results_model,
+                                 Categories category)
+    {
+      foreach (var device in devices)
+      {
+        results_model.append (device.uri, device.icon_name, category,
+                              "", device.display_name,
+                              device.dnd_uri);
+      }
+    }
+
     private async void perform_locate (uint timeout,
                                        string query,
                                        Dee.Model results_model,
@@ -861,7 +903,7 @@
       }
     }
 
-    private void count_directory_items (string top_path, ref int count, ref int64 total_size, bool recurse=false) throws GLib.Error
+    private void count_directory_items (string top_path, ref int count, ref uint64 total_size, bool recurse=false) throws GLib.Error
     {
       var dir = GLib.File.new_for_path (top_path);
       var file_iter = dir.enumerate_children (GLib.FileAttribute.STANDARD_SIZE, GLib.FileQueryInfoFlags.NONE); //follow symlinks
@@ -913,6 +955,11 @@
       {
         real_uri = Bookmark.uri_from_bookmark (uri);
       }
+      else if (Device.is_device_uri (uri))
+      {
+        preview = create_device_preview (uri);
+        return preview;
+      }
       else
       {
         real_uri = uri;
@@ -959,7 +1006,7 @@
           open_action.activated.connect (on_preview_open);
           preview.add_action (open_action);
 
-          int64 total_size = 0;
+          uint64 total_size = 0;
           if (finfo.get_file_type () == GLib.FileType.DIRECTORY)
           {
             if (file.get_path () != null)
@@ -1010,6 +1057,50 @@
       return preview;
     }
 
+
+    private Unity.GenericPreview? create_device_preview (string uri)
+    {
+      var device = devices.get_device_from_uri (uri);
+
+      if (device == null)
+        return null;
+
+      Unity.GenericPreview preview = new Unity.GenericPreview (device.display_name, "", device.icon);
+
+      int contents_count = 0;
+      uint64 contents_size = 0;
+      string filesystem_type = "";
+      uint64 total_capacity = 0;
+      uint64 free_capacity = 0;
+
+      try {
+        var file = device.get_root_file (device.volume);
+        var file_info = file.query_filesystem_info (GLib.FileAttribute.FILESYSTEM_SIZE + "," +
+                                                    GLib.FileAttribute.FILESYSTEM_TYPE + "," +
+                                                    GLib.FileAttribute.FILESYSTEM_FREE, null);
+
+        count_directory_items (file.get_path (), ref contents_count, ref contents_size, false);
+        filesystem_type = file_info.get_attribute_as_string (GLib.FileAttribute.FILESYSTEM_TYPE);
+        total_capacity = file_info.get_attribute_uint64 (GLib.FileAttribute.FILESYSTEM_SIZE);
+        free_capacity = file_info.get_attribute_uint64 (GLib.FileAttribute.FILESYSTEM_FREE);
+
+        contents_size = total_capacity - free_capacity;
+        
+      } catch (Error e) {
+        warning ("Failed to query filesystem info: %s", e.message);
+      }
+
+      preview.subtitle = _("Total capacity %s").printf (GLib.format_size(total_capacity));
+      preview.add_info (new InfoHint ("filesytem_type", _("Filesystem type"), null, filesystem_type));
+      preview.add_info (new InfoHint ("contents", _("Contents"), null, _("%s, %u items").printf (GLib.format_size (contents_size), contents_count)));
+
+      var open_action = new Unity.PreviewAction ("open", _("Open"), null);
+      open_action.activated.connect (on_preview_open);
+      preview.add_action (open_action);
+
+      return preview;
+    }
+
     public Unity.ActivationResponse on_email_file (string uri)
     {
       debug (@"Emailing file: $uri");
@@ -1033,7 +1124,7 @@
     {
       try
       {
-        if (bookmarks.launch_if_bookmark (uri) || GLib.AppInfo.launch_default_for_uri (uri, null))
+        if (bookmarks.launch_if_bookmark (uri) || devices.launch_if_device (uri) || GLib.AppInfo.launch_default_for_uri (uri, null))
         {
           return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
         }
@@ -1066,7 +1157,7 @@
     {
       debug (@"Activating: $uri");
       try {
-        if (bookmarks.launch_if_bookmark (uri)) 
+        if (bookmarks.launch_if_bookmark (uri) || devices.launch_if_device (uri)) 
           return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
           
         /* this code ensures that a file manager will be used

=== modified file 'src/folder.vala'
--- src/folder.vala	2012-09-03 07:07:56 +0000
+++ src/folder.vala	2012-09-17 19:57:19 +0000
@@ -21,18 +21,18 @@
 
   public class Bookmarks : Object
   {
-    
+
     private List<Bookmark> bookmarks;
     private string bookmarks_file;
     private FileMonitor monitor;
-    
+
     public signal void changed ();
-    
+
     public Bookmarks ()
     {
       bookmarks_file = @"$(Environment.get_home_dir())/.gtk-bookmarks";
       update();
-      
+
       /* Update the bookmarks list whener the bookmarks file changes */
       try {
         monitor = File.new_for_path (bookmarks_file).monitor (FileMonitorFlags.NONE);
@@ -47,7 +47,7 @@
                  bookmarks_file, e.message);
       }
     }
-    
+
     private void update ()
     {
       bookmarks = new List<Bookmark> ();
@@ -57,31 +57,32 @@
 
       File desktop_file = File.new_for_path (Environment.get_user_special_dir (UserDirectory.DESKTOP));
       desktop_uri = desktop_file.get_uri ();
-      
+
       try {
         FileUtils.get_contents (bookmarks_file, out contents);
-        
+
       } catch (FileError e) {
         warning ("Failed to read favorites: %s", e.message);
         return;
       }
-      
+
       string[] favorites = contents.split ("\n");
       string mimetype = "inode/directory";
-      
+
       foreach (var uri in favorites)
       {
         if (uri == "")
           continue;
+
         // Filter out useless bookmark that is created by Nautilus.
         // https://bugs.launchpad.net/unity-lens-files/+bug/1044309
         if (uri.has_prefix("x-nautilus-desktop:")) continue;
-          
+
         if (uri == desktop_uri) has_desktop_in_favourites = true;
-              
+
         string[] parts = uri.split (" ", 2);
         string display_name;
-        
+
         if (parts.length == 1)
           {
             display_name = Uri.unescape_string (uri);
@@ -98,11 +99,11 @@
                      uri);
             display_name = uri;
           }
-        
+
         var bookmark = new Bookmark (uri, mimetype, display_name);
         bookmarks.append (bookmark);
       }
-      
+
       /* Add desktop statically */
       if (!has_desktop_in_favourites)
       {
@@ -164,12 +165,12 @@
     {
       return filter_bookmarks (bookmarks);
     }
-    
+
     public List<Bookmark> prefix_search (string search)
     {
       var prefix = Utils.normalize_string (search);
       var matches = new List<Bookmark> ();
-      
+
       foreach (var bookmark in bookmarks)
       {
         foreach (var term in bookmark.get_index_terms ())
@@ -182,35 +183,35 @@
             }
         }
       }
-      
+
       return filter_bookmarks (matches);
     }
-      
+
     public bool launch_if_bookmark (string uri) throws Error
     {
       if (!uri.has_prefix ("bookmark:"))
         return false;
-      
+
       var launcher = AppInfo.get_default_for_type ("inode/directory", true);
-      
+
       uri = uri.offset (9); // Remove "bookmark:" prefix from uri
-      
+
       if (launcher == null)
         {
           warning ("No default handler for inode/directory. Unable to open bookmark '%s'", uri);
           throw new IOError.FAILED ("No default handler for inode/directory. Unable to open bookmark '%s'", uri);
         }
-      
+
       var uris = new List<string> ();
       uris.append (uri);
-      
+
       launcher.launch_uris (uris, null);
-      
+
       return true;
     }
-  
+
   }
-  
+
   public class Bookmark : Object
   {
     public string uri { get; set construct; }
@@ -218,19 +219,19 @@
     public string mimetype { get; set construct; }
     public string display_name { get; set construct; }
     public string dnd_uri { get; set construct; }
-    
+
     private List<string> index_terms;
-  
+
     public Bookmark (string uri, string mimetype, string display_name)
     {
       Object (uri:"bookmark:"+uri, icon:Utils.get_icon_for_uri (uri, mimetype),
               mimetype:mimetype, display_name:display_name, dnd_uri:uri);
-      
+
       index_terms = new List<string> ();
       index_terms.append (Utils.normalize_string (Path.get_basename (uri)));
       index_terms.append (Utils.normalize_string (display_name));
     }
-    
+
     public unowned List<string> get_index_terms ()
     {
       return index_terms;
@@ -246,6 +247,210 @@
 	  return uri.substring (9);
 	}
   }
-  
+
+
+  public class Devices : Object
+  {
+    private List<Device> devices;
+    private VolumeMonitor volume_monitor;
+
+    public signal void changed ();
+
+    public Devices ()
+    {
+      volume_monitor = VolumeMonitor.get ();
+
+      update();
+
+      volume_monitor.volume_added.connect ((mon, volume) => {
+          update ();
+          changed ();
+       });
+
+      volume_monitor.volume_removed.connect ((mon, volume) => {
+          update ();
+          changed ();
+       });
+
+      volume_monitor.volume_changed.connect ((mon, volume) => {
+          update ();
+          changed ();
+       });
+    }
+
+    private void update ()
+    {
+      devices = new List<Device> ();
+
+      foreach ( Volume volume in volume_monitor.get_volumes ())
+      { 
+        var device = new Device (volume);
+        devices.append (device);
+      }
+    }
+
+    public List<Device> list ()
+    {
+      var result = new GLib.List<Device> ();
+
+      foreach (var device in devices)
+      {
+        result.append (device);
+      }
+
+      return result;
+    }
+
+    public List<Device> search (string search)
+    {
+      var normalized_search = Utils.normalize_string (search);
+      var matches = new List<Device> ();
+
+      foreach (var device in devices)
+      {
+        foreach (var term in device.get_index_terms ())
+        {
+          if (term.contains (normalized_search))
+          {
+            matches.append (device);
+            break;
+          }
+        }
+      }
+
+      return matches;
+    }
+
+    public bool launch_if_device (string uri) throws Error
+    {
+      if (!Device.is_device_uri (uri))
+        return false;
+
+      foreach (var device in devices)
+      {
+        if (device.uri == uri)
+        {
+          device.mount_and_open ();
+          return true;
+        }
+      }
+
+      return false;
+    }
+ 
+    public Device? get_device_from_uri (string uri)
+    {
+      if (!Device.is_device_uri (uri))
+        return null;
+
+      foreach (var device in devices)
+      {
+        if (device.uri == uri)
+          return device;
+      }
+
+      return null;
+    }
+  }
+
+
+  public class Device : Object
+  {
+    public Volume volume { get; set construct; }
+    public string name { get; set construct; }
+    public string uri { get; set construct; }
+    public Icon icon { get; set construct; }
+    public string icon_name { get; set construct; }
+    public string display_name { get; set construct; }
+    public string dnd_uri { get; set construct; }
+
+    private List<string> index_terms_;
+
+    public Device (Volume volume)
+    {
+      var name = volume.get_name ();
+      var icon = volume.get_icon ();
+      var icon_name = icon.to_string ();
+
+      var identifier = get_volume_uri (volume); 
+      if (identifier != null && identifier.has_prefix ("file://"))
+        identifier = identifier.substring(7);
+ 
+      Object (volume:volume, name:name, uri:"device://"+identifier, icon:icon,icon_name:icon_name, display_name:name, dnd_uri:"device://"+"ciao");
+
+      index_terms_ = new List<string> ();
+      index_terms_.append (Utils.normalize_string (name));
+    }
+
+    public unowned List<string> get_index_terms ()
+    {
+      return index_terms_;
+    }
+
+    public static bool is_device_uri (string uri)
+    {
+      return uri.has_prefix ("device://");
+    }
+
+    public void mount_and_open () throws Error
+    {
+      if (is_mounted (volume))
+      {
+        open_in_file_manager ();
+      }
+      else
+      {
+        volume.mount.begin (MountMountFlags.NONE, null, null, (obj, res) =>  {
+          try {
+            if (volume.mount.end (res))
+              open_in_file_manager ();
+          } catch (Error e) {
+            warning ("Failed to mount %s: %s", display_name, e.message);
+          }
+        });
+      } 
+    }
+
+    private static bool is_mounted (Volume volume)
+    {
+      var mount = volume.get_mount ();
+      return mount != null;
+    }
+
+    private void open_in_file_manager () throws Error
+    {
+      if (!is_mounted (volume))
+        return;
+
+      AppInfo.launch_default_for_uri (get_volume_uri (volume), null);
+    }
+
+    private static string? get_volume_uri (Volume volume)
+    {
+      var root = get_root_file (volume);
+
+      if (root == null)
+        return null;
+
+      return root.get_uri ();
+    }
+
+    public static File? get_root_file (Volume volume)
+    {
+      if (is_mounted(volume))
+      {
+        var mount = volume.get_mount ();
+
+        if (mount == null)
+          return null;
+
+        return mount.get_root ();
+      }
+      else
+      {
+        return volume.get_activation_root ();
+      }
+    }
+  }
 
 } /* end: namespace */

=== modified file 'tests/manual/folders.txt'
--- tests/manual/folders.txt	2012-04-05 14:58:27 +0000
+++ tests/manual/folders.txt	2012-09-17 19:57:19 +0000
@@ -8,3 +8,44 @@
 Outcome
   You should see "Files and Folders" category with both folders starting with 'd' (including "Desktop"), followed by files containing 'd' or residing in a folder with 'd' in name.
 
+
+File system icons - Plug
+----------------------------------------------------
+Tests that file system icons are shown in the "Folder" category.
+
+Actions:
+#. Plug a USB flash drive.
+#. Open files lens.
+#. Expand "Folder" category.
+
+Expected Result:
+You should see the icon of the USB flash drive in the "Folders" category.
+
+
+File system icons - Click
+----------------------------------------------------
+Test that file system icons can be mounted and are opened in the file manager.
+
+Actions:
+#. Plug a USB flash drive.
+#. Open files lens.
+#. Expand "Folder" category.
+#. Click on the icon of the USB flash drive.
+
+Expected Result:
+The file manager should be opened showing the content of the USB flash drive.
+
+  
+File system icons - Unplug
+----------------------------------------------------
+Test that file system icons are removed from the "Folder" category.
+
+Actions:
+#. Plug a USB flash drive.
+#. Open files lens.
+#. Expand "Folder" category.
+#. Make sure you see the icon of the USB flash drive.
+#. Unplug the USB flash drive.
+
+Expected Result:
+Make sure the icon is correctly removed from the "Folder" category.