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

Proposed by Mikkel Kamstrup Erlandsen
Status: Superseded
Proposed branch: lp:~angeloc/unity-lens-files/fix-for-773841
Merge into: lp:unity-lens-files
Diff against target: 201 lines (+73/-35)
3 files modified
src/daemon.vala (+33/-15)
src/folder.vala (+1/-1)
src/url-checker.vala (+39/-19)
To merge this branch: bzr merge lp:~angeloc/unity-lens-files/fix-for-773841
Reviewer Review Type Date Requested Status
Michal Hruby (community) Needs Fixing
Unity Bugs Pending
Review via email: mp+95591@code.launchpad.net

This proposal has been superseded by a proposal from 2012-03-06.

Description of the change

Fixes activation of remotely mountable volumes.

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

To post a comment you must log in.
Revision history for this message
Michal Hruby (mhr3) wrote :

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
Revision history for this message
Angelo Compagnucci (angeloc) wrote :

+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.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

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?

214. By Angelo Compagnucci

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

Revision history for this message
Angelo Compagnucci (angeloc) wrote :

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

Revision history for this message
Michal Hruby (mhr3) wrote :

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

215. By Angelo Compagnucci

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.

Revision history for this message
Angelo Compagnucci (angeloc) wrote :

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?

216. By Angelo Compagnucci

Merging changes for bug 921665 because of commons.

217. By Angelo Compagnucci

Fixed indentation
Fixed wrong enum type name

218. By Angelo Compagnucci

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

Revision history for this message
Angelo Compagnucci (angeloc) wrote :

Done!

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

219. By Angelo Compagnucci

Code cleanup

220. By Angelo Compagnucci

Added UrlType.UNKNOWN

Revision history for this message
Angelo Compagnucci (angeloc) wrote :

Added UrlType.UNKNOWN as requested.

221. By Angelo Compagnucci

Code cleanup, fixing codestyle.

222. By Angelo Compagnucci

Merged with trunk, fix for bug 773841

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *15 *
16 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>16 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>
17 * Modified by Angelo Compagnucci <angelo.compagnucci@gmail.com>
17 *18 *
18 */19 */
19using Zeitgeist;20using Zeitgeist;
@@ -33,6 +34,7 @@
3334
34 private Bookmarks bookmarks;35 private Bookmarks bookmarks;
35 private UrlChecker urls;36 private UrlChecker urls;
37 private UrlType url_type;
3638
37 private Unity.Lens lens;39 private Unity.Lens lens;
38 private Unity.Scope scope;40 private Unity.Scope scope;
@@ -376,14 +378,16 @@
376378
377 results_model.clear ();379 results_model.clear ();
378380
379 /* check if the thing typed isn't a url (like facebook.com) */381 /* check if the thing typed isn't a url (like facebook.com)
380 var checked_url = urls.check_url (search.search_string);382 * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
383 var url_type = UrlType.WEB;
384 var checked_url = urls.check_url (search.search_string, out url_type);
381 if (checked_url != null)385 if (checked_url != null)
382 {386 {
383 results_model.append (checked_url, urls.icon,387 results_model.append (checked_url, urls.get_icon_for_type(url_type),
384 Categories.FILES_AND_FOLDERS,388 Categories.FILES_AND_FOLDERS,
385 "text/html", search.search_string,389 "text/html", search.search_string,
386 checked_url, checked_url);390 checked_url, checked_url);
387 }391 }
388392
389 var category_id = has_search ?393 var category_id = has_search ?
@@ -438,13 +442,16 @@
438442
439 txn.clear ();443 txn.clear ();
440444
441 /* check if the thing typed isn't a url (like facebook.com) */445 /* check if the thing typed isn't a url (like facebook.com)
442 var checked_url = urls.check_url (search.search_string);446 * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
447 var url_type = UrlType.WEB;
448 var checked_url = urls.check_url (search.search_string, out url_type);
443 if (checked_url != null)449 if (checked_url != null)
444 {450 {
445 txn.append (checked_url, urls.icon, Categories.RECENT,451 txn.append (checked_url, urls.get_icon_for_type(url_type),
446 "text/html", search.search_string,452 Categories.RECENT,
447 checked_url, checked_url);453 "text/html", search.search_string,
454 checked_url, checked_url);
448 }455 }
449456
450 /* apply filters to results found by zeitgeist */457 /* apply filters to results found by zeitgeist */
@@ -757,13 +764,24 @@
757 {764 {
758 debug (@"Activating: $uri");765 debug (@"Activating: $uri");
759 try {766 try {
760 if (!bookmarks.launch_if_bookmark (uri))767 if (bookmarks.launch_if_bookmark (uri))
761 AppInfo.launch_default_for_uri (uri, null);768 return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
762 return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);769
770 /* this code ensures that a file manager will be used
771 * * if uri it's a remote location that should be mounted */
772 var url_type = UrlType.WEB;
773 var checked_url = urls.check_url (uri, out url_type);
774 if (checked_url != null && url_type == UrlType.MOUNTABLE) {
775 var muris = new GLib.List<string>();
776 muris.prepend (uri);
777 var file_manager = AppInfo.get_default_for_type("inode/directory", true);
778 file_manager.launch_uris(muris,null);
779 return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
780 }
763 } catch (GLib.Error error) {781 } catch (GLib.Error error) {
764 warning ("Failed to launch URI %s", uri);782 warning ("Failed to launch URI %s", uri);
765 return new Unity.ActivationResponse(Unity.HandledType.NOT_HANDLED);
766 }783 }
784 return new Unity.ActivationResponse (Unity.HandledType.NOT_HANDLED);
767 }785 }
768786
769 private void on_zeitgeist_changed ()787 private void on_zeitgeist_changed ()
770788
=== 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 @@
187 var uris = new List<string> ();187 var uris = new List<string> ();
188 uris.append (uri);188 uris.append (uri);
189 189
190 launcher.launch_uris (uris, new AppLaunchContext());190 launcher.launch_uris (uris, null);
191 191
192 return true;192 return true;
193 }193 }
194194
=== 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 @@
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *15 *
16 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>16 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>
17 * Modified by Angelo Compagnucci <angelo.compagnucci@gmail.com>
17 *18 *
18 */19 */
1920
20namespace Unity.FilesLens {21namespace Unity.FilesLens {
22 public enum UrlType {
23 WEB,
24 MOUNTABLE;
25 }
2126
22 public class UrlChecker : Object27 public class UrlChecker : Object
23 {28 {
24 /* A string serialized GIcon */29 /* A string serialized GIcon */
25 public string icon { get; private set; }30 public string web_icon { get; private set; }
31 public string mountable_icon { get; private set; }
26 32
27 /* Regexes URLs must match */33 /* Regexes URLs must match */
28 private List<Regex> regexes;34 private Regex web_regex;
35 private Regex mountable_regex;
29 36
30 public UrlChecker ()37 public UrlChecker ()
31 {38 {
32 icon = new ThemedIcon ("web-browser").to_string ();39 web_icon = new ThemedIcon ("web-browser").to_string ();
33 regexes = new List<Regex> ();40 mountable_icon = new ThemedIcon ("folder-remote").to_string ();
34 41
35 try {42 try {
36 regexes.prepend (new Regex (".+\\...+")); // contains a dot + >= 2 chars43 web_regex = new Regex ("([a-zA-Z0-9\\-]+\\...+|http[s]{0,1}://.+)");
37 regexes.prepend (new Regex ("www.*"));44 mountable_regex = new Regex ("(\\\\|(ftp|ssh|sftp|smb|dav)://).+");
38 regexes.prepend (new Regex ("http://.+"));
39 regexes.prepend (new Regex ("https://.+"));
40 } catch (RegexError e) {45 } catch (RegexError e) {
41 warning ("Error compiling regular expressions for URL matching. URL launching will not work: %s", e.message);46 warning ("Error compiling regular expressions for URL matching. URL launching will not work: %s", e.message);
42 }47 }
43 }48 }
44 49
45 /* Returns a valid HTTP/HTTPS URL if the input looks like it,50 /* Returns a valid URL if the input looks like it or null otherwise,
46 * or null otherwise */51 * returns also url type which can after be used
47 public string? check_url (string sample)52 * to retrive corresponding icon
53 */
54 public string? check_url (string sample, out UrlType url_type)
48 {55 {
49 if (sample.strip () == "") return null;56 if (sample.strip () == "") return null;
50
51 foreach (var regex in regexes)
52 {
53 if (regex.match (sample))
54 {
55 return sample.has_prefix ("http") ? sample : ("http://" + sample);
56 }
57 }
58 57
58 if (mountable_regex.match (sample))
59 {
60 url_type = UrlType.MOUNTABLE;
61 return sample.replace("\\\\","smb://");
62 }
63 else if (web_regex.match (sample))
64 {
65 url_type = UrlType.WEB;
66 return sample.has_prefix ("http") ? sample : ("http://" + sample);
67 }
59 return null;68 return null;
60 }69 }
61 70
71 public string get_icon_for_type (UrlType url_type)
72 {
73 switch (url_type){
74 case UrlType.WEB:
75 return web_icon;
76 case UrlType.MOUNTABLE:
77 return mountable_icon;
78 }
79 return web_icon;
80 }
81
62 } /* end: class UrlChecker */82 } /* end: class UrlChecker */
6383
64} /* end: namespace */84} /* end: namespace */

Subscribers

People subscribed via source and target branches