Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lnotify 0.3.4 is incompatible with procps-ng 3.3.12 #339

Open
larsks opened this issue May 22, 2019 · 3 comments
Open

lnotify 0.3.4 is incompatible with procps-ng 3.3.12 #339

larsks opened this issue May 22, 2019 · 3 comments
Labels
bug

Comments

@larsks
Copy link

@larsks larsks commented May 22, 2019

The lnotify script uses ps to determine the name of the currently running window, but it uses command line arguments that are not supported by the version of ps in many distributions. The generate command line looks like:

ps -ho comm -p xterm

And fails like this:

error: unsupported SysV option

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).

Note this is a different failure than the one reported in #334.

@larsks
Copy link
Author

@larsks larsks commented May 22, 2019

@kevr I think this is your plugin. What is the ignore_windows_list code for? It currently has a somewhat arbitrary list of terminal names that is not user-configureable, and this particular chunk of code is responsible for multiple failures.

@larsks
Copy link
Author

@larsks larsks commented May 22, 2019

I wonder if we should:

(a) just remove it:

diff --git a/python/lnotify.py b/python/lnotify.py
index 5070ba3..f3a20ac 100644
--- a/python/lnotify.py
+++ b/python/lnotify.py
@@ -94,20 +94,8 @@ def handle_msg(data, pbuffer, date, tags, displayed, highlight, prefix, message)
     buffer_type = weechat.buffer_get_string(pbuffer, "localvar_type")
     away = weechat.buffer_get_string(pbuffer, "localvar_away")
     x_focus = False
-    window_name = ""
     my_nickname = "nick_" + weechat.buffer_get_string(pbuffer, "localvar_nick")

-    # Check if active window is in the ignore_windows_list and skip notification
-    if (environ.get('DISPLAY') != None) and path.isfile("/bin/xdotool"):
-        cmd_pid="xdotool getactivewindow getwindowpid".split()
-        window_pid = subprocess.check_output(cmd_pid)
-        cmd_name=("ps -ho comm -p %s"%(window_pid)).split()
-        window_name = subprocess.check_output(cmd_name)
-        ignore_windows_list = ["tilda", "gnome-terminal", "xterm"]
-        if window_name in ignore_windows_list:
-            x_focus = True
-            return weechat.WEECHAT_RC_OK
-
     if pbuffer == weechat.current_buffer() and x_focus:
         return weechat.WEECHAT_RC_OK

Or (b) make it user configurable? I'm not clear what the intent is here (e.g., as currently written, this code will suppress notifications whenever I'm using an gnome-terminal terminal, which seems odd).

@larsks larsks changed the title lnotify 0.3.3 is incompatible with procps-ng 3.3.12 lnotify 0.3.4 is incompatible with procps-ng 3.3.12 May 22, 2019
@weechatter weechatter added the bug label May 23, 2019
@sadsfae
Copy link

@sadsfae sadsfae commented May 29, 2019

The lnotify script uses ps to determine the name of the currently running window, but it uses command line arguments that are not supported by the version of ps in many distributions. The generate command line looks like:

ps -ho comm -p xterm

And fails like this:

error: unsupported SysV option

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).

Note this is a different failure than the one reported in #334.

I have the same issue fwiw @larsks

larsks added a commit to larsks/scripts that referenced this issue May 31, 2019
The python "lnotify" plugin had some problematic code that would
suppress notifications if the active window was named "tilda",
"xterm", or "gnome-terminal". This list was not user-configurable, and
the code implementing this feature was broken due to weechat#339
(incompatible with common versions of ps) and weechat#334 (python3
bytes-vs-string incompatibility).

This commit removes the problematic code.

Closes: weechat#334
Closes: weechat#339
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.