franciscolourenco / done

A fish-shell package to automatically receive notifications when long processes finish.
MIT License
767 stars 70 forks source link

done doesn't work on KDE Neon/Ubuntu 18.04 #77

Closed lephuongbg closed 4 years ago

lephuongbg commented 4 years ago

Running __done_get_focused_window_id currently returns this:

$ __done_get_focused_window_id
usage:  xprop [-options ...] [[format [dformat]] atom] ...

where options include:
    -grammar                       print out full grammar for command line
    -display host:dpy              the X server to contact
    -id id                         resource id of window to examine
    -name name                     name of window to examine
    -font name                     name of font to examine
    -remove propname               remove a property
    -set propname value            set a property to a given value
    -root                          examine the root window
    -len n                         display at most n bytes of any property
    -notype                        do not display the type field
    -fs filename                   where to look for formats for properties
    -frame                         don't ignore window manager frames
    -f propname format [dformat]   formats to use for property of given name
    -spy                           examine window properties forever

It is the results of running xprop -version 2>&1 >/dev/null at done.fish#L33. There is no option -version in KDE Neon's (which is Ubuntu 18.04-based) xprop. Commenting out that line makes it work well.

ammgws commented 4 years ago

Looks like it was first added in 2014:

https://gitlab.freedesktop.org/xorg/app/xprop/-/commit/b0ae4b903067017ec7dc19f27d3916d2153410af

lephuongbg commented 4 years ago

The version contains that commit is xprop 1.2.3, which was released 2 years ago. That release didn't make it into Ubuntu 18.04 LTS. The one that 18.04 has is 1.2.2 which was released 6 years ago.

Could we remove or replace that version check line with xprop -help 2>&1 /dev/null since it was just to make sure xprop command is there and working?

franciscolourenco commented 4 years ago

Thanks for the report, should be fixed in 1.14.4.

lephuongbg commented 4 years ago

Please re-open this issue.

xprop -help 2>&1 > /dev/null actually returns 1 because -help is an invalid option.

We can use xprop -grammar 2>&1 > /dev/null instead.

franciscolourenco commented 4 years ago

seriously?

franciscolourenco commented 4 years ago

in which version was -grammar introduced? seems like -help and -version are only available in some versions and were introduced at different points in time.

ammgws commented 4 years ago

https://gitlab.freedesktop.org/xorg/app/xprop/-/commit/23e375f3842b433b0af7f150135537f7381208ae

Grammar existed at least 12 years ago, I think it's a pretty safe bet.

Edit: https://gitlab.freedesktop.org/xorg/app/xprop/-/commit/4cc997bd2d02961b95bff992f6dbfa30138879a9

Even 16 years ago it was there (on mobile so can't check further but that looks like the initial commit for xprop). Also it looks like -help was around then as well, so I have no idea why it's not working for some users.

ammgws commented 4 years ago

@lephuongbg the version you linked to has -help as an option in the manpage. Any idea why it's telling you it's an invalid option?

franciscolourenco commented 4 years ago

-help will fail if xprop cannot connect with the configured $DISPLAY. thats the odd behaviour which make the command be useful to us in the first place.

Is it possible that the command is failing but not because of -help, as we are expecting?

lephuongbg commented 4 years ago

@lephuongbg the version you linked to has -help as an option in the manpage. Any idea why it's telling you it's an invalid option?

I assumed it is an invalid option due to its return code. Re-checked and in the man page it does list -help as an option. So xprop -help seems to fail due to other reason.

franciscolourenco commented 4 years ago

@lephuongbg what is the result of printenv DISPLAY?

jriddy commented 4 years ago

@lephuongbg the version you linked to has -help as an option in the manpage. Any idea why it's telling you it's an invalid option?

I assumed it is an invalid option due to its return code. Re-checked and in the man page it does list -help as an option. So xprop -help seems to fail due to other reason.

This is a bug in xprop. The man page suggests -help as an option, but the program itself returns an error code when you run that. I'll looking for the repo to report that to, and i'll post the link to the relevant issue when i make that report.

In the time being...I think i've found a fix, i'll submit a PR

jriddy commented 4 years ago

Filed that PR #78, unfortunately launchpad seems to be having trouble so I can't file a bug with the x11-utils maintainers for this yet.

08:21:29 $ dpkg -S /usr/bin/xprop
x11-utils: /usr/bin/xprop

08:21:39 $ apt-cache show x11-utils
Package: x11-utils
Architecture: amd64
Version: 7.7+3build1
Multi-Arch: foreign
Priority: optional
Section: x11
Origin: Ubuntu
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Debian X Strike Force <debian-x@lists.debian.org>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 635
Depends: libc6 (>= 2.15), libfontconfig1 (>= 2.11.94), libfontenc1, libgl1-mesa-glx | libgl1, libx11-6, libx11-xcb1, libxaw7, libxcb-shape0, libxcb1 (>= 1.6), libxcomposite1 (>= 1:0.3-1), libxext6, libxft2 (>> 2.1.1), libxi6, libxinerama1, libxmu6, libxmuu1, libxrandr2 (>= 2:1.2.0), libxrender1, libxt6, libxtst6, libxv1, libxxf86dga1, libxxf86vm1
# ...
ammgws commented 4 years ago

@jriddy I ended up sending a PR upstream so if it gets accepted it should hopefully make it's way down to the various distros soon (well at least Arch should be quick anyway). https://gitlab.freedesktop.org/xorg/app/xprop/-/merge_requests/2

jriddy commented 4 years ago

@jriddy I ended up sending a PR upstream so if it gets accepted it should hopefully make it's way down to the various distros soon (well at least Arch should be quick anyway). https://gitlab.freedesktop.org/xorg/app/xprop/-/merge_requests/2

Yeah I use Debian/Ubuntu (I know I'm lame) where this won't get rolled out for another 6 months - 2 years :joy:

franciscolourenco commented 4 years ago

Fixed in v1.14.5, thank you @jriddy and @ammgws

jriddy commented 4 years ago

No prob, thanks for the useful tool!