Bug 37899 - Factor out scripts' detectDE as xdg-detect-de? ["PATCH"]
Summary: Factor out scripts' detectDE as xdg-detect-de? ["PATCH"]
Status: NEW
Alias: None
Product: Portland
Classification: Unclassified
Component: xdg-utils (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Portland Bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-03 16:16 UTC by Ryan Beasley
Modified: 2013-03-09 12:48 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
xdg-env script (785 bytes, patch)
2013-03-07 18:57 UTC, Antonielly
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Beasley 2011-06-03 16:16:17 UTC
I have a nagging suspicion that, while desktop environments aren't unified under a single spec, application developers will have to special case each desktop environment's quirks.  For a project I worked on, I ended up stealing the detectDE subroutine and shoving it into its own script.

What do Portland's custodians think of doing the same officially?
Comment 1 Rex Dieter 2011-08-12 07:15:42 UTC
I like the idea, though turns out desktops are gaining some sanity here and supporting a XDG_CURRENT_DESKTOP env var themselves.
Comment 2 Ryan Beasley 2011-08-12 16:21:43 UTC
(In reply to comment #1)
> I like the idea, though turns out desktops are gaining some sanity here and
> supporting a XDG_CURRENT_DESKTOP env var themselves.

That still leaves those pesky legacy releases to worry about.  ;)
Comment 3 Ryan Beasley 2011-11-03 17:06:42 UTC
Related to this, the detectDE routine seems to convert everything to lowercase, though this doesn't necessarily match the environment names in menu-spec.  If xdg-detect-de were a standalone, I'd suggest that the names should match those in menu-spec (which are case sensitive).

LMK if I could/should whip up a patch which could be applied to the raw sources to generate a standalone xdg-detect-de and have the other scripts use/source this.
Comment 4 Antonielly 2013-03-07 18:57:43 UTC
Created attachment 76130 [details] [review]
xdg-env script
Comment 5 Antonielly 2013-03-07 19:00:23 UTC
Turn detectDE() into a separate utility

Most applications in xdg-utils need to execute the same piece of code, currently named detectDE(), to detect the Desktop Environment (DE) for the current user session. When detectDE() needs to evolve (for instance, to know how to detect a new DE), it is necessary to change all the applications that have implemented it.

If we modularize detectDE() into a separate application, the current xdg-utils tools will still be able to correctly detect the Desktop Environment for the current user session. The added benefits are that:

1) the code for DE detection will be kept into a single place

2) the size of the code for the current applications in xdg-utils will get smaller and better focused on their "core missions"

3) inconsistencies will be removed from the current code base (e.g. compare the current detectDE() implementations in xdg-open and xdg-settings)

4) applications from projects other than xdg-utils will also be able to benefit from a consistent way to detect the Desktop Environment, by invoking that program. This opens the door for them to integrate better their look and feel to each DE, based on runtime detection, and also to provide additional DE-specific features.

Suggested name for the tool:
xdg-env

Rationale: the "d" in "xdg" means "Desktop". "env" means Environment.

The proposed code for xdg-env is the following:
-------
#!/bin/bash
if [ x"$KDE_FULL_SESSION" = x"true" ]; then echo kde;
elif [ x"$GNOME_DESKTOP_SESSION_ID" != x"" ]; then echo gnome;
elif `dbus-send --print-reply --dest=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.GetNameOwner string:org.gnome.SessionManager > /dev/null 2>&1` ; then echo gnome;
elif [ x"$MATE_DESKTOP_SESSION_ID" != x"" ]; then echo mate;
elif `dbus-send --print-reply --dest=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.GetNameOwner string:org.mate.SessionManager > /dev/null 2>&1`; then echo mate;
elif xprop -root _DT_SAVE_MODE 2> /dev/null | grep ' = \"xfce4\"$' >/dev/null 2>&1; then echo xfce;
elif [ x"$DESKTOP_SESSION" = x"LXDE" ]; then echo lxde;
elif [ x"$XDG_CURRENT_DESKTOP" = x"LXDE" ]; then echo lxde;
else echo ""
fi
-------

After including the xdg-env application in xdg-utils, the following changes will have to be performed for each xdg-utils application (such as xdg-open and xdg-email):

1) remove the detectDE() function

2) globally replace the line
detectDE
by
DE=`xdg-env`

3) test the application

Migration of the code in xdg-utils can be done piecemeal, application by application, after xdg-env is made available as an external program. Refactoring a given application would not affect other xdg-utils programs.

Somewhat related bug reports:
https://bugs.freedesktop.org/show_bug.cgi?id=45604
https://bugs.freedesktop.org/show_bug.cgi?id=23123
Comment 6 Rex Dieter 2013-03-07 19:06:52 UTC
point 3 is invalid, the detectDE code is shared in all tools already (copied, admittedly), see xdg-utils-common.in in git
Comment 7 Antonielly 2013-03-09 12:43:00 UTC
Still, there should be a separate "xdg-env" application so that external apps can benefit from DE detection as well.
Comment 8 Rex Dieter 2013-03-09 12:48:16 UTC
True, I'm not against adding this new feature, as a matter of fact, wanted to try to modularize things even more for 2.0 (or whatever).

That said, priority is on pure bugfixes at the moment.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.