altdesktop / i3ipc-python

🐍 An improved Python library to control i3wm and sway.
http://i3ipc-python.readthedocs.io
BSD 3-Clause "New" or "Revised" License
858 stars 108 forks source link

No equivalent of Con.find_classed(pattern: str) for Sway WM. #170

Closed rifazn closed 3 years ago

rifazn commented 3 years ago

Sway assigns the attribute app_id to windows instead of the (functionally same) attribute window_class that's assigned in i3wm.

We can just copy and replace instances of c.window_class to c.app_id from the method Con.find_classed(pattern: str) to, perhaps, a new method named Con.find_by_app_id(pattern: str) and it should work. We should then have something like:

...

def find_by_app_id(self, pattern: str) -> list['Con']: # New function that supports sway
    return [c for c in self if c.app_id and re.search(pattern, c.app_id)]

def find_classed(self, pattern: str) -> List['Con']:
    return [c for c in self if c.window_class and re.search(pattern, c.window_class)]

...

Or, we can just modify the existing find_classed() to check whether Connection.get_version().ipc_data['variant'] == 'sway' to use either app_id (in case of sway) or window_class otherwise.

I think having a different method name, and leaving the user to figure out which one to use is the better option.

May I try a Pull Request if the discussion proceeds and you like the idea?

acrisci commented 3 years ago

I don't want to add anymore find functions. Just make find_classed() match on both X11 class and sway app id.

rifazn commented 3 years ago

Is something like this okay?

def findclassed(self, pattern):
     return [c for c in self if (c.window_class and re.search(pattern, c.window_class)) 
        or (c.app_id and re.search(pattern, c.app_id))]

Or should I be more verbose and break up the conditionals a little:

def find_classed(self, pattern):
    if c.app_id:
        return [c for c in self if re.search(pattern, c.app_id)
    else:
     return [c for c in self if (c.window_class and re.search(pattern, c.window_class)]
acrisci commented 3 years ago

The second is better because a window will never have both a class and an app id.

acrisci commented 3 years ago

Actually that's wrong. The first is better because in a group of windows, they may be mixed since some are wayland and some are X11.

rifazn commented 3 years ago

In that case, perhaps the best way would be to search in terms of both app_id and window_class and then concat the two lists... Something like:

def find_classed(self, pattern):
    wayland_windows = [c for c in self if c.app_id and re.search(pattern, c.app_id)]
    x11_windows = [c for c in self if c.window_class and re.search(pattern, c.window_class)]

    return wayland_windows + x11_windows

What do you think?

acrisci commented 3 years ago

Yeah looks good. Go ahead and do a PR.

rifazn commented 3 years ago

Okay! See pull request #171