Merge lp:~mitya57/unity-gtk-module/lp1383382 into lp:unity-gtk-module

[Dmitry Shachnev]

Support multiple desktop names in $XDG_CURRENT_DESKTOP.

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

For example, on GNOME Flashback session, $XDG_CURRENT_DESKTOP is now ‘GNOME-Flashback;Unity’,
and we still want to load unity-gtk-module there.

[William Hua (attente)]

The change seems to break under Unity. What's the proper dash syntax is to make it also match when XDG_CURRENT_DESKTOP is just "Unity"?

review: Needs Fixing

[Alberts Muktupāvels (muktupavels)]

Would not it be better to split string into array?

DESKTOPS=array from $XDG_CURRENT_DESKTOP...
for DESKTOP in $DESKTOPS; do
  if [ "x$DESKTOP" != "xUnity" ] then
    stop
    exit 0
  fi
done

[Dmitry Shachnev (mitya57)]

Oh, what I was trying to use is a bashism. Should be better now.

[Dmitry Shachnev (mitya57)]

What Alberts suggests is also fine, but involves sed:

for DESKTOP in $(echo "$XDG_CURRENT_DESKTOP" | sed 's/;/ /')
do
  if [ "x$DESKTOP" = "xUnity" ] then
    exit 0
  fi
done
stop

William, if you prefer this, then I will use this instead.

[Alberts Muktupāvels (muktupavels)]

XDG_CURRENT_DESKTOP separator is ':' not ';'.

[Dmitry Shachnev (mitya57)]

Indeed. Will make that change if William prefers your solution to mine.

[William Hua (attente)]

Don't we want the opposite effect? The script should exit if all of the tokens are not Unity (i.e. it should not exit if any is Unity)? Also, the sed should end with /g in case there's more than one ':'.

[William Hua (attente)]

Side note, the case pattern match does seem to work for me.

[Dmitry Shachnev (mitya57)]

exit = "finish the pre-start script, go ahead"
stop = "stop the upstart job, bad desktop"

So, the script gets to the "stop" line only when none of the parts matched Unity.

[William Hua (attente)]

> for DESKTOP in $(echo "$XDG_CURRENT_DESKTOP" | sed 's/;/ /')
> do
> if [ "x$DESKTOP" = "xUnity" ] then
> exit 0
> fi
> done
> stop

I just tried this, replacing the expression with 's/:/ /g', but it doesn't seem to work with Unity either.

[Dmitry Shachnev (mitya57)]

Sorry, I forgot a semicolon before then:

for DESKTOP in $(echo "$XDG_CURRENT_DESKTOP" | sed 's/:/ /')
do
  if [ "x$DESKTOP" = "xUnity" ]; then
    exit 0
  fi
done
stop

Do you prefer this variant?

[William Hua (attente)]

Ah, thank you! It works now! Can you test it again under GNOME Fallback? I can't seem to get a GNOME Fallback session working. But I tested it under Unity and XFCE.

I don't have a strong preference, either is fine.

review: Approve

[Dmitry Shachnev (mitya57)]

The Flashback session is currently not working because of <https://<email address hidden>>.

With the proposed upstart patch (and r334 from this branch) applied, everything works fine.

Feel free to merge either r333 or r334 :)

[Dmitry Shachnev (mitya57)]

Err, the correct link should be <http://<email address hidden>>.

[William Hua (attente)]

Just one minor nitpick, could you please add the trailing 'g' to the sed expression in case there's more than one colon? I can confirm that this works on Unity, GNOME Flashback and XFCE. Thanks Dmitry!

[Dmitry Shachnev (mitya57)]

Done (amended the previous commit).

[lp:~mitya57/unity-gtk-module/lp1383382]

333. By Dmitry Shachnev on 2015-01-15

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

For example, on GNOME Flashback session, $XDG_CURRENT_DESKTOP is now
‘GNOME-Flashback;Unity’, and we still want to load unity-gtk-module there.

=== modified file 'data/unity-gtk-module.conf'
--- data/unity-gtk-module.conf	2014-04-15 12:41:08 +0000
+++ data/unity-gtk-module.conf	2015-01-15 11:26:26 +0000
@@ -3,11 +3,13 @@
 start on starting dbus
 
 pre-start script
-    if [ "x$XDG_CURRENT_DESKTOP" != "xUnity" ]
-    then
-      stop
-      exit 0
-    fi
+    for DESKTOP in $(echo "$XDG_CURRENT_DESKTOP" | sed 's/:/ /g')
+    do
+      if [ "x$DESKTOP" = "xUnity" ]; then
+        exit 0
+      fi
+    done
+    stop
 end script
 
 script