Merge lp:~docky-core/plank/launcher-uri into lp:plank

[Rico Tzschichholz]

[Robert Dyer (psybers)]

diff lines 94-106 -> each individual to_uri call needs its own try/catch block

This is because the first one might be invalid, but the rest may be valid. With 1 try/catch, if the first throws then none will be added to the dock. Individual try/catches will only skip ones that threw individually.

review: Needs Fixing

[Robert Dyer (psybers)]

Have you extensively tested this?

[Rico Tzschichholz (ricotz)]

> diff lines 94-106 -> each individual to_uri call needs its own try/catch block
>
> This is because the first one might be invalid, but the rest may be valid.
> With 1 try/catch, if the first throws then none will be added to the dock.
> Individual try/catches will only skip ones that threw individually.

Yeah I know I was lazy here :\

[Rico Tzschichholz (ricotz)]

> Have you extensively tested this?

As far as I can. It will transition old dockitem's Launcher entries to the new uri scheme. Of course old plank versions can't handle them afterwards.

I was hoping you will give it a spin. Especially in regards to https://bugs.launchpad.net/plank/+bug/1058468

[lp:~docky-core/plank/launcher-uri]

735. By Rico Tzschichholz on 2013-01-29

items: No need for a SeparatorMenuItem if the associated folder is empty

736. By Rico Tzschichholz on 2013-01-30

main: Avoid calling Poxis.exit ()

Change the way we are passing the commandline-arguments through the
initalization methods and return EXIT_SUCCESS or EXIT_FAILURE to the
actual main function

737. By Rico Tzschichholz on 2013-01-30

items: Validate an item before adding it here too

739. By Rico Tzschichholz on 2013-01-29

Handle errors separately in make_default_gnome_items

740. By Rico Tzschichholz on 2013-01-29

Better use string.has_prefix ()

741. By Rico Tzschichholz on 2013-01-30

Handle get_desktop_file () == null

[Sergey "Shnatsel" Davidoff (shnatsel)]

The transition from plain filenames worked fine for me in the latest revision; I have some dangling .dockitem files in launchers folder, they were properly ignored. I didn't notice any regressions. Loading URLs other than file:// doesn't seem to work, but I believe this is out of scope of this branch.

review: Approve

=== modified file 'lib/DockItems.vala'
--- lib/DockItems.vala	2013-01-30 18:16:04 +0000
+++ lib/DockItems.vala	2013-01-30 18:21:19 +0000
@@ -170,12 +170,25 @@
 		
 		ApplicationDockItem? item_for_application (Bamf.Application app)
 		{
+			var app_desktop_file = app.get_desktop_file ();
+			if (app_desktop_file != null && app_desktop_file.has_prefix ("/"))
+				try {
+					app_desktop_file = Filename.to_uri (app_desktop_file);
+				} catch (ConvertError e) {
+					warning (e.message);
+				}
+			
 			foreach (var item in internal_items) {
 				unowned ApplicationDockItem? appitem = (item as ApplicationDockItem);
 				if (appitem == null)
 					continue;
-				if ((appitem.App != null && appitem.App == app) || (appitem.Launcher != null
-					&& appitem.Launcher != "" && appitem.Launcher == app.get_desktop_file ()))
+				
+				var item_app = appitem.App;
+				if (item_app != null && item_app == app)
+					return appitem;
+				
+				var launcher = appitem.Launcher;
+				if (launcher != "" && app_desktop_file != null && launcher == app_desktop_file)
 					return appitem;
 			}
 			
@@ -184,23 +197,22 @@
 		
 		public bool item_exists_for_uri (string uri)
 		{
-			var launcher = uri.replace ("file://", "");
 			foreach (var item in internal_items)
-				if (item.Launcher == launcher)
+				if (item.Launcher == uri)
 					return true;
 			
 			return false;
 		}
 		
-		public void add_item_with_launcher (string launcher, DockItem? target = null)
+		public void add_item_with_uri (string uri, DockItem? target = null)
 		{
-			if (launcher == null || launcher == "")
+			if (uri == null || uri == "")
 				return;
 			
 			// delay automatic add of new dockitems while creating this new one
 			delay_items_monitor ();
 			
-			var dockitem_file = Factory.item_factory.make_dock_item (launcher);
+			var dockitem_file = Factory.item_factory.make_dock_item (uri);
 			if (dockitem_file == null)
 				return;
 			

=== modified file 'lib/DragManager.vala'
--- lib/DragManager.vala	2013-01-21 11:57:33 +0000
+++ lib/DragManager.vala	2013-01-30 18:21:19 +0000
@@ -221,7 +221,7 @@
 			if (DragIsDesktopFile) {
 				var uri = drag_data[0];
 				if (!controller.items.item_exists_for_uri (uri))
-					controller.items.add_item_with_launcher (uri.replace ("file://", ""), item);
+					controller.items.add_item_with_uri (uri, item);
 				
 				ExternalDragActive = false;
 				return true;
@@ -232,7 +232,7 @@
 			} else {
 				foreach (var uri in drag_data) {
 					if (!controller.items.item_exists_for_uri (uri))
-						controller.items.add_item_with_launcher (uri.replace ("file://", ""), item);
+						controller.items.add_item_with_uri (uri, item);
 				}
 			}
 			

=== modified file 'lib/Factories/ItemFactory.vala'
--- lib/Factories/ItemFactory.vala	2013-01-21 11:57:33 +0000
+++ lib/Factories/ItemFactory.vala	2013-01-30 18:21:19 +0000
@@ -87,13 +87,29 @@
 				return false;
 			
 			if (browser != null)
-				make_dock_item (new DesktopAppInfo (browser.get_id ()).get_filename ());
+				try {
+					make_dock_item (Filename.to_uri (new DesktopAppInfo (browser.get_id ()).get_filename ()));
+				} catch (ConvertError e) {
+					warning (e.message);
+				}
 			if (terminal != null)
-				make_dock_item (new DesktopAppInfo (terminal.get_id ()).get_filename ());
+				try {
+					make_dock_item (Filename.to_uri (new DesktopAppInfo (terminal.get_id ()).get_filename ()));
+				} catch (ConvertError e) {
+					warning (e.message);
+				}
 			if (calendar != null)
-				make_dock_item (new DesktopAppInfo (calendar.get_id ()).get_filename ());
+				try {
+					make_dock_item (Filename.to_uri (new DesktopAppInfo (calendar.get_id ()).get_filename ()));
+				} catch (ConvertError e) {
+					warning (e.message);
+				}
 			if (media != null)
-				make_dock_item (new DesktopAppInfo (media.get_id ()).get_filename ());
+				try {
+					make_dock_item (Filename.to_uri (new DesktopAppInfo (media.get_id ()).get_filename ()));
+				} catch (ConvertError e) {
+					warning (e.message);
+				}
 			
 			return true;
 		}
@@ -104,50 +120,50 @@
 		public void make_default_items ()
 		{
 			// add plank item!
-			make_dock_item ((Paths.DataFolder.get_parent ().get_path () ?? "") + "/applications/" + Factory.main.app_launcher);
+			make_dock_item (Paths.DataFolder.get_parent ().get_child ("applications").get_child (Factory.main.app_launcher).get_uri ());
 			
 			if (make_default_gnome_items ())
 				return;
 			
 			// add browser
-			if (make_dock_item ("/usr/share/applications/chromium-browser.desktop") == null)
-				if (make_dock_item ("/usr/local/share/applications/google-chrome.desktop") == null)
-					if (make_dock_item ("/usr/share/applications/firefox.desktop") == null)
-						if (make_dock_item ("/usr/share/applications/epiphany.desktop") == null)
-							make_dock_item ("/usr/share/applications/kde4/konqbrowser.desktop");
+			if (make_dock_item ("file:///usr/share/applications/chromium-browser.desktop") == null)
+				if (make_dock_item ("file:///usr/local/share/applications/google-chrome.desktop") == null)
+					if (make_dock_item ("file:///usr/share/applications/firefox.desktop") == null)
+						if (make_dock_item ("file:///usr/share/applications/epiphany.desktop") == null)
+							make_dock_item ("file:///usr/share/applications/kde4/konqbrowser.desktop");
 			
 			// add terminal
-			if (make_dock_item ("/usr/share/applications/terminator.desktop") == null)
-				if (make_dock_item ("/usr/share/applications/gnome-terminal.desktop") == null)
-					make_dock_item ("/usr/share/applications/kde4/konsole.desktop");
+			if (make_dock_item ("file:///usr/share/applications/terminator.desktop") == null)
+				if (make_dock_item ("file:///usr/share/applications/gnome-terminal.desktop") == null)
+					make_dock_item ("file:///usr/share/applications/kde4/konsole.desktop");
 			
 			// add music player
-			if (make_dock_item ("/usr/share/applications/exaile.desktop") == null)
-				if (make_dock_item ("/usr/share/applications/songbird.desktop") == null)
-					if (make_dock_item ("/usr/share/applications/rhythmbox.desktop") == null)
-						if (make_dock_item ("/usr/share/applications/banshee-1.desktop") == null)
-							make_dock_item ("/usr/share/applications/kde4/amarok.desktop");
+			if (make_dock_item ("file:///usr/share/applications/exaile.desktop") == null)
+				if (make_dock_item ("file:///usr/share/applications/songbird.desktop") == null)
+					if (make_dock_item ("file:///usr/share/applications/rhythmbox.desktop") == null)
+						if (make_dock_item ("file:///usr/share/applications/banshee-1.desktop") == null)
+							make_dock_item ("file:///usr/share/applications/kde4/amarok.desktop");
 			
 			// add IM client
-			if (make_dock_item ("/usr/share/applications/pidgin.desktop") == null)
-				make_dock_item ("/usr/share/applications/empathy.desktop");
+			if (make_dock_item ("file:///usr/share/applications/pidgin.desktop") == null)
+				make_dock_item ("file:///usr/share/applications/empathy.desktop");
 		}
 		
 		/**
-		 * Creates a new .dockitem for a launcher.
+		 * Creates a new .dockitem for a uri.
 		 *
-		 * @param launcher the launcher to create a .dockitem for
+		 * @param uri the uri or path to create a .dockitem for
 		 * @param sort the Sort value in the new .dockitem
 		 * @return the new {@link GLib.File} of the new .dockitem created
 		 */
-		public GLib.File? make_dock_item (string launcher)
+		public GLib.File? make_dock_item (string uri)
 		{
-			var launcher_file = File.new_for_path (launcher);
+			var launcher_file = File.new_for_uri (uri);
 			
 			if (launcher_file.query_exists ()) {
 				var file = new KeyFile ();
 				
-				file.set_string (typeof (Items.DockItemPreferences).name (), "Launcher", launcher);
+				file.set_string (typeof (Items.DockItemPreferences).name (), "Launcher", uri);
 				
 				try {
 					// find a unique file name, based on the name of the launcher
@@ -166,7 +182,7 @@
 					stream.put_string (file.to_data ());
 					stream.close ();
 					
-					debug ("Created dock item '%s' for launcher '%s'", dockitem_file.get_path (), launcher);
+					debug ("Created dock item '%s' for launcher '%s'", dockitem_file.get_path (), uri);
 					return dockitem_file;
 				} catch { }
 			}

=== modified file 'lib/Items/ApplicationDockItem.vala'
--- lib/Items/ApplicationDockItem.vala	2012-12-15 20:36:01 +0000
+++ lib/Items/ApplicationDockItem.vala	2013-01-30 18:21:19 +0000
@@ -186,7 +186,7 @@
 		
 		void handle_launcher_changed ()
 		{
-			App = Matcher.get_default ().app_for_launcher (Prefs.Launcher);
+			App = Matcher.get_default ().app_for_uri (Prefs.Launcher);
 			
 			launcher_changed ();
 		}
@@ -284,7 +284,7 @@
 		
 		void launch ()
 		{
-			Services.System.launch (File.new_for_path (Prefs.Launcher));
+			Services.System.launch (File.new_for_uri (Prefs.Launcher));
 		}
 		
 		/**
@@ -459,7 +459,7 @@
 			foreach (var uri in uris)
 				files.add (File.new_for_uri (uri));
 			
-			Services.System.launch_with_files (File.new_for_path (Prefs.Launcher), files.to_array ());
+			Services.System.launch_with_files (File.new_for_uri (Prefs.Launcher), files.to_array ());
 			
 			return true;
 		}
@@ -504,7 +504,7 @@
 			
 			try {
 				var file = new KeyFile ();
-				file.load_from_file (launcher, 0);
+				file.load_from_file (Filename.from_uri (launcher), 0);
 				
 				icon = file.get_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_ICON);
 				text = file.get_locale_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_NAME);
@@ -607,7 +607,7 @@
 				return;
 			
 			try {
-				monitor = File.new_for_path (Prefs.Launcher).monitor (0);
+				monitor = File.new_for_uri (Prefs.Launcher).monitor (0);
 				monitor.changed.connect (monitor_changed);
 			} catch {
 				warning ("Unable to watch the launcher file '%s'", Prefs.Launcher);

=== modified file 'lib/Items/DockItem.vala'
--- lib/Items/DockItem.vala	2013-01-21 11:57:33 +0000
+++ lib/Items/DockItem.vala	2013-01-30 18:21:19 +0000
@@ -245,7 +245,7 @@
 		 * Whether or not this item is valid for the .dockitem given.
 		 */
 		public virtual bool ValidItem {
-			get { return File.new_for_path (Prefs.Launcher).query_exists (); }
+			get { return File.new_for_uri (Prefs.Launcher).query_exists (); }
 		}
 		
 		/**

=== modified file 'lib/Items/DockItemPreferences.vala'
--- lib/Items/DockItemPreferences.vala	2013-01-21 11:57:33 +0000
+++ lib/Items/DockItemPreferences.vala	2013-01-30 18:21:19 +0000
@@ -24,7 +24,7 @@
 	 */
 	public class DockItemPreferences : Preferences
 	{
-		[Description(nick = "launcher", blurb = "The path to the launcher for this item.")]
+		[Description(nick = "launcher", blurb = "The uri for this item.")]
 		public string Launcher { get; set; }
 		
 		/**
@@ -50,5 +50,22 @@
 		{
 			Launcher = "";
 		}
+		
+		/**
+		 * {@inheritDoc}
+		 */
+		protected override void verify (string prop)
+		{
+			switch (prop) {
+			case "Launcher":
+				if (Launcher.has_prefix ("/"))
+					try {
+						Launcher = Filename.to_uri (Launcher);
+					} catch (ConvertError e) {
+						warning (e.message);
+					}
+				break;
+			}
+		}
 	}
 }

=== modified file 'lib/Items/FileDockItem.vala'
--- lib/Items/FileDockItem.vala	2013-01-29 08:32:24 +0000
+++ lib/Items/FileDockItem.vala	2013-01-30 18:21:19 +0000
@@ -59,9 +59,15 @@
 			if (!ValidItem)
 				return;
 			
-			OwnedFile = File.new_for_path (Prefs.Launcher);
+			OwnedFile = File.new_for_uri (Prefs.Launcher);
 			
 			Icon = DrawingService.get_icon_from_file (OwnedFile) ?? DEFAULT_ICON;
+			
+			if (!OwnedFile.is_native ()) {
+				Text = OwnedFile.get_uri ();
+				return;
+			}
+			
 			Text = OwnedFile.get_basename () ?? "";
 			
 			// pop up the dir contents on a left click too
@@ -163,7 +169,7 @@
 		
 		void handle_launcher_changed ()
 		{
-			OwnedFile = File.new_for_path (Prefs.Launcher);
+			OwnedFile = File.new_for_uri (Prefs.Launcher);
 			
 			launcher_changed ();
 		}

=== modified file 'lib/Services/Matcher.vala'
--- lib/Services/Matcher.vala	2012-08-14 14:48:47 +0000
+++ lib/Services/Matcher.vala	2013-01-30 18:21:19 +0000
@@ -122,8 +122,16 @@
 			return list;
 		}
 		
-		public Bamf.Application? app_for_launcher (string launcher)
+		public Bamf.Application? app_for_uri (string uri)
 		{
+			string launcher;
+			try {
+				launcher = Filename.from_uri (uri);
+			} catch (ConvertError e) {
+				warning (e.message);
+				return null;
+			}
+			
 			unowned Bamf.Application app = bamf_matcher.get_application_for_desktop_file (launcher, false);
 			
 			warn_if_fail (app != null);

=== modified file 'lib/libplank.symbols'
--- lib/libplank.symbols	2013-01-21 12:10:22 +0000
+++ lib/libplank.symbols	2013-01-30 18:21:19 +0000
@@ -2,7 +2,7 @@
 plank_dock_controller_get_type
 plank_dock_controller_new
 plank_dock_items_add_item
-plank_dock_items_add_item_with_launcher
+plank_dock_items_add_item_with_uri
 plank_dock_items_construct
 plank_dock_items_get_Items
 plank_dock_items_get_type