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

Proposed by Mikkel Kamstrup Erlandsen on 2012-03-02
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) 2012-03-02 Needs Fixing on 2012-03-02
Unity Bugs 2012-03-02 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.
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
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.

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 on 2012-03-02

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

Angelo Compagnucci (angeloc) wrote :

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

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 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) 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 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) wrote :

Done!

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

219. By Angelo Compagnucci on 2012-03-06

Code cleanup

220. By Angelo Compagnucci on 2012-03-06

Added UrlType.UNKNOWN

Angelo Compagnucci (angeloc) wrote :

Added UrlType.UNKNOWN as requested.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/daemon.vala'
2--- src/daemon.vala 2012-02-14 13:44:37 +0000
3+++ src/daemon.vala 2012-03-06 10:35:04 +0000
4@@ -14,6 +14,7 @@
5 * along with this program. If not, see <http://www.gnu.org/licenses/>.
6 *
7 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>
8+ * Modified by Angelo Compagnucci <angelo.compagnucci@gmail.com>
9 *
10 */
11 using Zeitgeist;
12@@ -33,6 +34,7 @@
13
14 private Bookmarks bookmarks;
15 private UrlChecker urls;
16+ private UrlType url_type;
17
18 private Unity.Lens lens;
19 private Unity.Scope scope;
20@@ -376,14 +378,16 @@
21
22 results_model.clear ();
23
24- /* check if the thing typed isn't a url (like facebook.com) */
25- var checked_url = urls.check_url (search.search_string);
26+ /* check if the thing typed isn't a url (like facebook.com)
27+ * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
28+ var url_type = UrlType.WEB;
29+ var checked_url = urls.check_url (search.search_string, out url_type);
30 if (checked_url != null)
31 {
32- results_model.append (checked_url, urls.icon,
33- Categories.FILES_AND_FOLDERS,
34- "text/html", search.search_string,
35- checked_url, checked_url);
36+ results_model.append (checked_url, urls.get_icon_for_type(url_type),
37+ Categories.FILES_AND_FOLDERS,
38+ "text/html", search.search_string,
39+ checked_url, checked_url);
40 }
41
42 var category_id = has_search ?
43@@ -438,13 +442,16 @@
44
45 txn.clear ();
46
47- /* check if the thing typed isn't a url (like facebook.com) */
48- var checked_url = urls.check_url (search.search_string);
49+ /* check if the thing typed isn't a url (like facebook.com)
50+ * or isn't a remote mountable url (like ftp://ftp.ubuntu.com)*/
51+ var url_type = UrlType.WEB;
52+ var checked_url = urls.check_url (search.search_string, out url_type);
53 if (checked_url != null)
54 {
55- txn.append (checked_url, urls.icon, Categories.RECENT,
56- "text/html", search.search_string,
57- checked_url, checked_url);
58+ txn.append (checked_url, urls.get_icon_for_type(url_type),
59+ Categories.RECENT,
60+ "text/html", search.search_string,
61+ checked_url, checked_url);
62 }
63
64 /* apply filters to results found by zeitgeist */
65@@ -757,13 +764,24 @@
66 {
67 debug (@"Activating: $uri");
68 try {
69- if (!bookmarks.launch_if_bookmark (uri))
70- AppInfo.launch_default_for_uri (uri, null);
71- return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
72+ if (bookmarks.launch_if_bookmark (uri))
73+ return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
74+
75+ /* this code ensures that a file manager will be used
76+ * * if uri it's a remote location that should be mounted */
77+ var url_type = UrlType.WEB;
78+ var checked_url = urls.check_url (uri, out url_type);
79+ if (checked_url != null && url_type == UrlType.MOUNTABLE) {
80+ var muris = new GLib.List<string>();
81+ muris.prepend (uri);
82+ var file_manager = AppInfo.get_default_for_type("inode/directory", true);
83+ file_manager.launch_uris(muris,null);
84+ return new Unity.ActivationResponse(Unity.HandledType.HIDE_DASH);
85+ }
86 } catch (GLib.Error error) {
87 warning ("Failed to launch URI %s", uri);
88- return new Unity.ActivationResponse(Unity.HandledType.NOT_HANDLED);
89 }
90+ return new Unity.ActivationResponse (Unity.HandledType.NOT_HANDLED);
91 }
92
93 private void on_zeitgeist_changed ()
94
95=== modified file 'src/folder.vala'
96--- src/folder.vala 2011-09-29 12:06:45 +0000
97+++ src/folder.vala 2012-03-06 10:35:04 +0000
98@@ -187,7 +187,7 @@
99 var uris = new List<string> ();
100 uris.append (uri);
101
102- launcher.launch_uris (uris, new AppLaunchContext());
103+ launcher.launch_uris (uris, null);
104
105 return true;
106 }
107
108=== modified file 'src/url-checker.vala'
109--- src/url-checker.vala 2012-01-30 11:26:40 +0000
110+++ src/url-checker.vala 2012-03-06 10:35:04 +0000
111@@ -14,51 +14,71 @@
112 * along with this program. If not, see <http://www.gnu.org/licenses/>.
113 *
114 * Authored by Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com>
115+ * Modified by Angelo Compagnucci <angelo.compagnucci@gmail.com>
116 *
117 */
118
119 namespace Unity.FilesLens {
120+ public enum UrlType {
121+ WEB,
122+ MOUNTABLE;
123+ }
124
125 public class UrlChecker : Object
126 {
127 /* A string serialized GIcon */
128- public string icon { get; private set; }
129+ public string web_icon { get; private set; }
130+ public string mountable_icon { get; private set; }
131
132 /* Regexes URLs must match */
133- private List<Regex> regexes;
134+ private Regex web_regex;
135+ private Regex mountable_regex;
136
137 public UrlChecker ()
138 {
139- icon = new ThemedIcon ("web-browser").to_string ();
140- regexes = new List<Regex> ();
141+ web_icon = new ThemedIcon ("web-browser").to_string ();
142+ mountable_icon = new ThemedIcon ("folder-remote").to_string ();
143
144 try {
145- regexes.prepend (new Regex (".+\\...+")); // contains a dot + >= 2 chars
146- regexes.prepend (new Regex ("www.*"));
147- regexes.prepend (new Regex ("http://.+"));
148- regexes.prepend (new Regex ("https://.+"));
149+ web_regex = new Regex ("([a-zA-Z0-9\\-]+\\...+|http[s]{0,1}://.+)");
150+ mountable_regex = new Regex ("(\\\\|(ftp|ssh|sftp|smb|dav)://).+");
151 } catch (RegexError e) {
152 warning ("Error compiling regular expressions for URL matching. URL launching will not work: %s", e.message);
153 }
154 }
155
156- /* Returns a valid HTTP/HTTPS URL if the input looks like it,
157- * or null otherwise */
158- public string? check_url (string sample)
159+ /* Returns a valid URL if the input looks like it or null otherwise,
160+ * returns also url type which can after be used
161+ * to retrive corresponding icon
162+ */
163+ public string? check_url (string sample, out UrlType url_type)
164 {
165 if (sample.strip () == "") return null;
166-
167- foreach (var regex in regexes)
168- {
169- if (regex.match (sample))
170- {
171- return sample.has_prefix ("http") ? sample : ("http://" + sample);
172- }
173- }
174
175+ if (mountable_regex.match (sample))
176+ {
177+ url_type = UrlType.MOUNTABLE;
178+ return sample.replace("\\\\","smb://");
179+ }
180+ else if (web_regex.match (sample))
181+ {
182+ url_type = UrlType.WEB;
183+ return sample.has_prefix ("http") ? sample : ("http://" + sample);
184+ }
185 return null;
186 }
187
188+ public string get_icon_for_type (UrlType url_type)
189+ {
190+ switch (url_type){
191+ case UrlType.WEB:
192+ return web_icon;
193+ case UrlType.MOUNTABLE:
194+ return mountable_icon;
195+ }
196+ return web_icon;
197+ }
198+
199 } /* end: class UrlChecker */
200
201 } /* end: namespace */

Subscribers

People subscribed via source and target branches