Merge lp:~angeloc/unity-lens-files/fix-for-773841 into lp:unity-lens-files

[Mikkel Kamstrup Erlandsen]

Fixes activation of remotely mountable volumes.

I am merge proposing this on behalf of Angelo since there appears to be some LP hiccups...

[Michal Hruby (mhr3)]

Nice work, thanks for the contribution!

A couple of comments:

7 private UrlChecker urls;
8 + private UrlMountChecker murls;

Wouldn't it make sense to extend the UrlChecker class to do this instead of adding yet another class that has almost the same semantics?

130 + regexes.prepend (new Regex ("\\\\.+"));
131 + regexes.prepend (new Regex ("ftp://.+"));
132 + regexes.prepend (new Regex ("ssh://.+"));
133 + regexes.prepend (new Regex ("sftp://.+"));
134 + regexes.prepend (new Regex ("smb://.+"));
135 + regexes.prepend (new Regex ("dav://.+"));

There's no need to use this many regexes - instead you can first preprocess the query by replacing \\ with smb:// (if it has \\ prefix), and then you can use a single regex "(ftp|ssh|sftp|smb|dav)://.+".

review: Needs Fixing

[Angelo Compagnucci (angeloc)]

+1 for grouping regexes, I'll make it!

Unifying UrlChecker and UrlMountChecker could be coumbersone, because we need to switch icon dinamically in case you have an url or a mountable volume. We have to add some sort of side effect and a discrete amount of ifs.

Furthermore we should have a way to distinguish between url or remote location in activate method and I thought using different check_urls method be the most straightforward way to go.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Btw Angelo, have you signed the contributor agreement? We need that in order to be able to take your branch http://www.canonical.com/contributors

On merging of the url checker classes I think Michal meant to have two different instances of the same class, but being able to define which regexes they use in the constructor or something. Michal?

[lp:~angeloc/unity-lens-files/fix-for-773841]

214. By Angelo Compagnucci on 2012-03-02

Collapsed wasteful regexes.
Added url-mount-checker to makefile.

[Angelo Compagnucci (angeloc)]

OK, like adding a flag that switch between url and remotes, think it could be done!
Please wait!

[Michal Hruby (mhr3)]

We cleared that on IRC already, I wanted the same instance and an enum defining what is what.

[lp:~angeloc/unity-lens-files/fix-for-773841]

215. By Angelo Compagnucci on 2012-03-05

Unified url-checker, with url-mount-checker, url-mount-checker removed.
Collapsed regexes as much as possible.
Using an enum to identify url_types.
Chenged check_url to return the url type.

[Angelo Compagnucci (angeloc)]

Done!

Unified url-checker, with url-mount-checker, url-mount-checker removed.
Collapsed regexes as much as possible.
Using an enum to identify url_types.
Chenged check_url to return the url type.

It's ok?

[lp:~angeloc/unity-lens-files/fix-for-773841]

216. By Angelo Compagnucci on 2012-03-05

Merging changes for bug 921665 because of commons.

217. By Angelo Compagnucci on 2012-03-06

Fixed indentation
Fixed wrong enum type name

218. By Angelo Compagnucci on 2012-03-06

Changed NORMAL -> WEB for url enum type.
Fixed wrong parenthesis position.
Fixed code style.

[Angelo Compagnucci (angeloc)]

Done!

Merged with fix for bug 921665.
Changed code to naming conventions requirements and code style.

[lp:~angeloc/unity-lens-files/fix-for-773841]

219. By Angelo Compagnucci on 2012-03-06

Code cleanup

220. By Angelo Compagnucci on 2012-03-06

Added UrlType.UNKNOWN

[Angelo Compagnucci (angeloc)]

Added UrlType.UNKNOWN as requested.

[lp:~angeloc/unity-lens-files/fix-for-773841]

221. By Angelo Compagnucci on 2012-03-07

Code cleanup, fixing codestyle.

222. By Angelo Compagnucci on 2012-03-12

Merged with trunk, fix for bug 773841

Unmerged revisions

=== modified file 'src/daemon.vala'
--- src/daemon.vala	2012-02-14 13:44:37 +0000
+++ src/daemon.vala	2012-03-06 10:35:04 +0000
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by Mikkel Kamstrup Erlandsen <[e-mail]>
+ * Modified by Angelo Compagnucci <[e-mail]>
  *
  */
 using Zeitgeist;
@@ -33,6 +34,7 @@
 
     private Bookmarks bookmarks;
     private UrlChecker urls;
+    private UrlType url_type;
 
     private Unity.Lens lens;
     private Unity.Scope scope;
@@ -376,14 +378,16 @@
 
         results_model.clear ();
 
-        /* check if the thing typed isn't a url (like facebook.com) */
-        var checked_url = urls.check_url (search.search_string);
+        /* check if the thing typed isn't a url (like facebook.com) 
+         * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
+        var url_type = UrlType.WEB;
+        var checked_url = urls.check_url (search.search_string, out url_type);
         if (checked_url != null)
         {
-          results_model.append (checked_url, urls.icon,
-                                Categories.FILES_AND_FOLDERS,
-                                "text/html", search.search_string,
-                                checked_url, checked_url);
+          results_model.append (checked_url, urls.get_icon_for_type(url_type),
+                Categories.FILES_AND_FOLDERS,
+                "text/html", search.search_string,
+                checked_url, checked_url);
         }
 
         var category_id = has_search ?
@@ -438,13 +442,16 @@
 
         txn.clear ();
 
-        /* check if the thing typed isn't a url (like facebook.com) */
-        var checked_url = urls.check_url (search.search_string);
+        /* check if the thing typed isn't a url (like facebook.com) 
+         * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
+        var url_type = UrlType.WEB; 
+        var checked_url = urls.check_url (search.search_string, out url_type);
         if (checked_url != null)
         {
-          txn.append (checked_url, urls.icon, Categories.RECENT,
-                      "text/html", search.search_string,
-                      checked_url, checked_url);
+          txn.append (checked_url, urls.get_icon_for_type(url_type), 
+                Categories.RECENT,
+                "text/html", search.search_string,
+                checked_url, checked_url);
         }
 
         /* apply filters to results found by zeitgeist */
@@ -757,13 +764,24 @@
     {
       debug (@"Activating: $uri");
       try {
-        if (!bookmarks.launch_if_bookmark (uri))
-          AppInfo.launch_default_for_uri (uri, null);
-        return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
+        if (bookmarks.launch_if_bookmark (uri)) 
+          return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
+          
+        /* this code ensures that a file manager will be used
+         * * if uri it's a remote location that should be mounted */
+        var url_type = UrlType.WEB;
+        var checked_url = urls.check_url (uri, out url_type);
+        if (checked_url != null && url_type == UrlType.MOUNTABLE) {
+          var muris = new GLib.List<string>();
+          muris.prepend (uri);
+          var file_manager = AppInfo.get_default_for_type("inode/directory", true);
+          file_manager.launch_uris(muris,null);
+          return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
+        }
       } catch (GLib.Error error) {
         warning ("Failed to launch URI %s", uri);
-        return new Unity.ActivationResponse(Unity.HandledType.NOT_HANDLED);
       }
+      return new Unity.ActivationResponse (Unity.HandledType.NOT_HANDLED);
     }
 
     private void on_zeitgeist_changed ()

=== modified file 'src/folder.vala'
--- src/folder.vala	2011-09-29 12:06:45 +0000
+++ src/folder.vala	2012-03-06 10:35:04 +0000
@@ -187,7 +187,7 @@
       var uris = new List<string> ();
       uris.append (uri);
       
-      launcher.launch_uris (uris, new AppLaunchContext());
+      launcher.launch_uris (uris, null);
       
       return true;
     }

=== modified file 'src/url-checker.vala'
--- src/url-checker.vala	2012-01-30 11:26:40 +0000
+++ src/url-checker.vala	2012-03-06 10:35:04 +0000
@@ -14,51 +14,71 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by Mikkel Kamstrup Erlandsen <[e-mail]>
+ * Modified by Angelo Compagnucci <[e-mail]>
  *
  */
 
 namespace Unity.FilesLens {
+  public enum UrlType {
+  WEB,
+  MOUNTABLE;
+  }
 
   public class UrlChecker : Object
   {
     /* A string serialized GIcon */
-    public string icon { get; private set; }
+    public string web_icon { get; private set; }
+    public string mountable_icon { get; private set; }
     
     /* Regexes URLs must match */
-    private List<Regex> regexes;
+    private Regex web_regex;
+    private Regex mountable_regex;
     
     public UrlChecker ()
     {
-      icon = new ThemedIcon ("web-browser").to_string ();
-      regexes = new List<Regex> ();
+      web_icon = new ThemedIcon ("web-browser").to_string ();
+      mountable_icon = new ThemedIcon ("folder-remote").to_string ();
       
       try {
-        regexes.prepend (new Regex (".+\\...+")); // contains a dot + >= 2 chars
-        regexes.prepend (new Regex ("www.*"));
-        regexes.prepend (new Regex ("http://.+"));
-        regexes.prepend (new Regex ("https://.+"));
+        web_regex = new Regex ("([a-zA-Z0-9\\-]+\\...+|http[s]{0,1}://.+)");
+        mountable_regex = new Regex ("(\\\\|(ftp|ssh|sftp|smb|dav)://).+");
       } catch (RegexError e) {
         warning ("Error compiling regular expressions for URL matching. URL launching will not work: %s", e.message);
       }
     }
     
-    /* Returns a valid HTTP/HTTPS URL if the input looks like it,
-     * or null otherwise */
-    public string? check_url (string sample)
+    /* Returns a valid URL if the input looks like it or null otherwise,
+     * returns also url type which can after be used
+     * to retrive corresponding icon
+     */
+    public string? check_url (string sample, out UrlType url_type)
     {
       if (sample.strip () == "") return null;
-
-      foreach (var regex in regexes)
-      {
-        if (regex.match (sample))
-          {
-            return sample.has_prefix ("http") ? sample : ("http://" + sample);
-          }
-      }
       
+      if (mountable_regex.match (sample))
+      {
+        url_type = UrlType.MOUNTABLE;
+        return sample.replace("\\\\","smb://");
+      }
+      else if (web_regex.match (sample))
+      {
+        url_type = UrlType.WEB;
+        return sample.has_prefix ("http") ? sample : ("http://" + sample);
+      }
       return null;
     }
     
+    public string get_icon_for_type (UrlType url_type)
+    {
+      switch (url_type){
+        case UrlType.WEB:
+      return web_icon;
+        case UrlType.MOUNTABLE:
+      return mountable_icon;
+      }
+      return web_icon;
+  }
+    
   } /* end: class UrlChecker */
 
 } /* end: namespace */