After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 503203 - GAppLaunchContext support
GAppLaunchContext support
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-12-12 06:43 UTC by Matthias Clasen
Modified: 2007-12-13 05:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GdkAppLaunchContext patch (20.91 KB, patch)
2007-12-12 06:43 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2007-12-12 06:43:17 UTC
Here is a patch that is a pretty straight port of EelAppLaunchContext from the eel-gio branch. The only things I've done to it is coding style adjustments, bug fixes, replacing libsn by gdk methods, and adding workspace support.

It seems to work ok in very cursory testing.

Comparing to Dan Winships eggdesktopfile code (see bug 415070), there are a number of things that are currently not supported by GAppInfo, but maybe should:

- clearing the environment
- setting environment variables
- setting the directory to launch in
- setting GSpawnFlags 
- setting the wmclass
- providing child setup function
- getting the pid of the launched process
- access to stdout/stderr/stdin
- getting the startup id
Comment 1 Matthias Clasen 2007-12-12 06:43:59 UTC
Created attachment 100804 [details] [review]
GdkAppLaunchContext patch
Comment 2 Alexander Larsson 2007-12-12 09:06:10 UTC
I got a bunch of feedback from desrt about how we should not expose e.g. setting the environment etc directly in the API, because launching might not be via a traditional spawn. For instance he had some plan to use dbus by having a well-known dbus name in the desktop file.

Also, such things would be unix specific, and wouldn't really work on win32.
Comment 3 Alexander Larsson 2007-12-12 09:10:33 UTC
Patch looks good to me.
Comment 4 Dan Winship 2007-12-12 15:43:37 UTC
Some of the things egg_desktop_file_launch() does are basically just because
the functionality was already there in g_spawn(), and it wasn't difficult to
expose it in egg_desktop_file_launch() as well, so I did. (Eg, I'm not aware
of any app that actually needs the "access to stdin/stderr/stdout"
functionality when launching a desktop file.)

Other stuff is there because EggDesktopFile is a "desktop file spec"
implementation, as opposed to GAppInfo, which is an "apps for mime types"
implementation that uses desktop files as an implementation detail under
unix. Eg, gnome-session needs to be able to set environment variables in
order to do autostart correctly, and gnome-panel needs to be able to get
information about categories and supported desktop environments from the
.desktop file, etc. But apps that need those bits of functionality aren't
going to be able to use GDesktopAppInfo anyway, because they're not doing
"apps for mime types", they're specifically doing fdo desktop file stuff.

So I don't know if any of those things are really needed by GAppInfo users.


(But I do feel like this:

    context = gdk_app_launch_context_new ();
    g_app_info_launch (info, files, context, &err);
    g_object_unref (context);

is clunky, and it would be nice to find some way to not need to do that.
Admittedly, it's *far* less clunky than the equivalent gnome-vfs code...)
Comment 5 Matthias Clasen 2007-12-12 18:21:36 UTC
> Some of the things egg_desktop_file_launch() does are basically just because
> the functionality was already there in g_spawn(), and it wasn't difficult to
> expose it in egg_desktop_file_launch() as well, so I did. (Eg, I'm not aware
> of any app that actually needs the "access to stdin/stderr/stdout"
> functionality when launching a desktop file.)

Fair enough. I think it is fine to hold off on these additions until we have acutal demand for them. Shouldn't be too hard to add a

g_app_info_launch_full (GAppInfo *info, 
                        GSList *files, 
                        GAppLaunchContext *context,
                        GError **error,
                        ...)

later, that would take key-value pairs for the extra parameters, similar to
your eggdesktopfile launch function.
Comment 6 Matthias Clasen 2007-12-13 05:53:30 UTC
Committed