Merge lp:~mitya57/indicator-sound/lp1502480 into lp:indicator-sound/15.10

[Dmitry Shachnev]

Support multiple desktop names in $XDG_CURRENT_DESKTOP.

According to the Desktop Entry specification:

If $XDG_CURRENT_DESKTOP is set then it contains a colon-separated list of strings. In order, each string is considered.

This branch adds support for proper detecting of Unity-derived desktops. It is mainly needed for GNOME Flashback session, which uses unity-control-center and XDG_DESKTOP_NAME=GNOME-Flashback:Unity.

[Alberts Muktupāvels (muktupavels)]

review: Approve

[Mystic-Mirage (mystic-mirage)]

review: Approve

[Alkis Georgopoulos (alkisg)]

It looks fine to me.

In shell, to avoid the loop, I do:
if echo ":$XDG_CURRENT_DESKTOP:" | grep -q ":Unity:"; then
    echo yes
else
    echo no
fi

Performance-wise it's fine, it needs only one malloc (for strcat) instead of many (for split), but I'm not sure how readable it is.

review: Approve

[Charles Kerr (charlesk)]

Not a blocker, but I think the standalone ``is_unity()'' function makes sense here as well as in indicator-datetime.

Two reasons:
1. readability: (is_unity() && (Environment.find_program_in_path ("unity-control-center") != null))
2. the ability of is_unity() to cache its value s.t. it doesn't have to re-evaluate each time it's called

review: Approve

[lp:~mitya57/indicator-sound/lp1502480]

537. By Dmitry Shachnev on 2016-04-05

Move Unity detection into a separate function, and add caching

[Dmitry Shachnev (mitya57)]

Splitted the code into a separate function and added caching, please check. I'm not fluent in Vala, so if that could be done simplier, please let me know.

[Charles Kerr (charlesk)]

LGTM.

review: Approve

=== modified file 'src/service.vala'
--- src/service.vala	2016-03-16 14:00:47 +0000
+++ src/service.vala	2016-04-05 21:55:32 +0000
@@ -206,6 +206,7 @@
 	private VolumeWarning _volume_warning;
 	private IndicatorSound.InfoNotification _info_notification = new IndicatorSound.InfoNotification();
 	private AccountsServiceAccess _accounts_service_access;
+	bool? is_unity = null;
 
 	const double volume_step_percentage = 0.06;
 
@@ -224,6 +225,24 @@
 		}
 	}
 
+	private bool desktop_is_unity() {
+		if (is_unity != null) {
+			return is_unity;
+		}
+
+		is_unity = false;
+		unowned string desktop = Environment.get_variable ("XDG_CURRENT_DESKTOP");
+
+		foreach (var name in desktop.split(":")) {
+			if (name == "Unity") {
+				is_unity = true;
+				break;
+			}
+		}
+
+		return is_unity;
+	}
+
 	void activate_desktop_settings (SimpleAction action, Variant? param) {
 		unowned string env = Environment.get_variable ("DESKTOP_SESSION");
 		string cmd;
@@ -238,13 +257,10 @@
 			cmd = "pavucontrol";
 		else if (env == "mate")
 			cmd = "mate-volume-control";
+		else if (desktop_is_unity() && Environment.find_program_in_path ("unity-control-center") != null)
+			cmd = "unity-control-center sound";
 		else
-		{
-			if (Environment.get_variable ("XDG_CURRENT_DESKTOP") == "Unity" && Environment.find_program_in_path ("unity-control-center") != null)
-				cmd = "unity-control-center sound";
-			else
-				cmd = "gnome-control-center sound";
-		}
+			cmd = "gnome-control-center sound";
 
 		try {
 			Process.spawn_command_line_async (cmd);