Merge lp:~andersk/unity-lens-applications/lp778202 into lp:unity-lens-applications

Thanks Anders for your patch and your try to fix this.
However, we don't run to spaw a shell to launch everything from alt + F2 as we are then loosing some nice capability like GIO integration and perfect and reliably matching of the application in the launcher.

What I would suggest (slightly more complicated feature) is a "Run in terminal" checkbox which can help to get there and interpret that.

Thanks a lot for your contribution. Do not hesitate to hop on #ayatana on the freenode channel to get some help about it. (not that and next week however, as most of the unity team is travelling at UDS right now).

Didier: I think you misunderstand the patch. sh is not a terminal. I’m only using sh -c to interpret the command string correctly, so that you can use, a quoted argument containing spaces (see bug #778202). This is the same way that system() and popen() interpret the command string.

This should have no effect on GIO integration or application matching. The patch only modifies the array passed to Process.spawn_async; no other state is affected.

Oops, by removing exec_or_dir I broke Alt+F2 with a directory name. Fixed now.

@Anders: Totally agree, but you spaw a shell which will confuse bamf (we really need to use gio to launch the application directly to get a perfect match between the application launched and the launcher).

Launching it in dash (default shell in ubuntu) will just add on level of indirection (sh will be the parent application for every applications you launch from alt + F2 and the dash).

I think the correct solution is rather having a look at Process.spawn_async. There should be a FLAG to interpret those characters. What do you think about it?

I haven’t seen any such problems when testing my patch. For example, I can do Alt+F2 gcalctool, and I get a gcalctool that’s a child of sh -c gcalctool that’s a child of /sbin/init. The appropriate icon shows up in the launcher. I can middle-click the icon to launch another gcalctool. The new one is a direct child of /sbin/init. The two gcalctools are grouped in the same launcher icon, and clicking the icon raises both of them, etc. What else should I be testing?

Also, if BAMF gets confused by init → sh → gcalctool, why wouldn’t it be confused by init → gnome-terminal → bash → gcalctool, or by any of the myriad other applications that launch other applications via sh? If so, is there a bug report for this?

None of the spawn_async flags are relevant. There is a spawn_command_line_async that takes a string and maybe interprets shell quotes correctly, but it doesn’t accept a working_directory, so that would regress bug #736471. There is GLib.Shell.parse_argv, which does only the parsing part of spawn_command_line_async, but still it only supports a small subset of shell functionality (for example, you couldn’t Alt+F2 ‘sleep 5; do something else’).

> (we
> really need to use gio to launch the application directly to get a perfect
> match between the application launched and the launcher).

I think this isn’t a concern because the patch only affects applications launched directly using Process.spawn_async. It does not affect any applications launched using GIO.

I'm request Jason's view on bamf, but I think you just trigger by that the fallback of the bamf integration, which is really awesome (we didn't see for a while in both maverick and natty that for a long period of time, we were in the fallback mode as Jason has done a perfect job on that), but with trickier application like java, python and such, this becomes untrue.

I really prefer the "GLib.Shell.parse_arg" path if you can implement that than the hack of sh -c. Seems more robust and clean code-wise.

For others cases like ‘sleep 5; do something else’ I think that "run in a terminal" checkbox is a better way to fix that problem as it's clearly explicit. I'm not even sure that gnome-panel alt + F2 supported that in fact.

I'm adding Jason to this review so that he can help on the bamf side.

Thanks for your further investigation :)

I think this is too old to enter anyway ;) Cleaning up MP list