Bug 26058 (add_lxde_support)

Created attachment 32655 [details] [review]
lxde support patch

Created attachment 39473 [details] [review]
Improved patch.

this one is against the 'xdg-*.in' files in the cvs.

(In reply to comment #2)
> Created an attachment (id=39473) [details]
> Improved patch.
> 
> this one is against the 'xdg-*.in' files in the cvs.

Also, it adds support for pcmanfm in xdg-open, which was actually my main motivation.

Created attachment 39477 [details] [review]
Improved patch #2.

Like the above except it now handles relative filepaths in the xdg-open/lxde case.
So, now it does pass the full test suite...

Well... ok, it fails in the xdg-email tests but since i still use the generic handler, the breakage is none of my doing (in my case it calls firefox, which calls sylpheed, but it fails to open a compose window). Ideally, i should use the preferred email client as configured in lxde but i don't know how to get that info without grepping config files which does not seem like a good idea (better would be a lxde command that output that info on stdout and is guaranteed to be future-proof).

(In reply to comment #4)
> Created an attachment (id=39477) [details]
> Improved patch #2.
> 
> Like the above except it now handles relative filepaths in the xdg-open/lxde
> case.
> So, now it does pass the full test suite...
> 
> Well... ok, it fails in the xdg-email tests but since i still use the generic
> handler, the breakage is none of my doing (in my case it calls firefox, which
> calls sylpheed, but it fails to open a compose window). Ideally, i should use
> the preferred email client as configured in lxde but i don't know how to get
> that info without grepping config files which does not seem like a good idea
> (better would be a lxde command that output that info on stdout and is
> guaranteed to be future-proof).


souldn't we consider lxterminal as lxde terminal?

if lxterminal is not found, open generic terminal.

in the same way.. isn't my "su" approach beeter than your?

+su_lxde()
+{
+    if which gnomesu &>/dev/null ; then
+        su_gnome
+    else
+        su_generic
+    fi
+}

Created attachment 39478 [details] [review]
Improved patch #3.

Now use lxterminal as the default terminal in xdg-terminal; if lxterminal is not available, fallback to terminal_generic. (ideally it should use whatever preferred term emulator has been configured in pcmanfm, but see my note about xdg-email above).

About xdg_su: as discussed with Andrea on irc; su_gnome now does proper error handling and fallback (if not gnomesu then xsu; if not xsu then su_generic) so it is safe to use directly.

Thanks, patch looks good, I'll incorporate it asap.

patch commited to cvs, thanks!