fennerm / flashfocus

Simple focus animations for tiling window managers.
MIT License
751 stars 9 forks source link

Make i3ipc optional #37

Closed jubalh closed 5 years ago

jubalh commented 5 years ago

Would it be possible to make i3ipc an optional dependency? So that users that actually use awesome-wm don't need to install an i3ipc package that they don't need?

fennerm commented 5 years ago

I'm not aware of an elegant way to do this in python. There are [extra] dependencies but I want pip install flashfocus to just work out of the box for sway users.

I think it would be possible for package maintainers to handle this though. i3ipc is isolated into a single module and is never imported unless it is detected that wayland is running. You could simply sed out the entry from setup.py and I think it would work.

jubalh commented 5 years ago

I removed it from setup.py. But I still get this:

flashfocus 
Traceback (most recent call last):
  File "/usr/bin/flashfocus", line 8, in <module>
    from flashfocus.cli import cli
  File "/usr/lib/python3.7/site-packages/flashfocus/cli.py", line 12, in <module>
    from flashfocus.config import init_user_configfile, load_merged_config
  File "/usr/lib/python3.7/site-packages/flashfocus/config.py", line 21, in <module>
    from flashfocus.compat import DisplayProtocol, get_display_protocol
  File "/usr/lib/python3.7/site-packages/flashfocus/compat.py", line 22, in <module>
    from flashfocus.display_protocols.sway import (
  File "/usr/lib/python3.7/site-packages/flashfocus/display_protocols/sway.py", line 12, in <module>
    import i3ipc
ModuleNotFoundError: No module named 'i3ipc'

It seems sway.py is still called.

fennerm commented 5 years ago

Thats odd I’ll look into it

fennerm commented 5 years ago

I just tried this on i3 and couldn't replicate. Is WAYLAND_DISPLAY environment variable defined on your machine by any chance?

jubalh commented 5 years ago

It's defined to wayland-0 when I run it from GNOME3 via Wayland. Is it possible to modify flashfocus so it doesn't crash in such a case?

fennerm commented 5 years ago

Could you explain your setup a little bit more? I was under the impression that awesome-wm is X only?

I could improve things a little so that it detects sway more precisely rather than just wayland but it shouldn't matter too much because non-sway wayland wms aren't supported yet.

jubalh commented 5 years ago

Okay sorry for being vague.

The traceback mentioned above occurs when I run flashfocus in GNOME3 wayland session and not in awesome. I'm aware GNOME3 is not supported but it would be cool if flashfocus could not crash in such case (the case where Wayland is used but not sway).

I use python3-PyYAML 5.1.2. And when starting flashfocus 2.0.3 in sway or openbox I get:

flashfocus    
/usr/lib/python3.7/site-packages/flashfocus/config.py:134: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
  config = yaml.load(f)
INFO: Loading configuration from {'flash_opacity': 0.8, 'default_opacity': 1, 'time': 500, 'simple': False, 'ntimepoints': 10, 'flash_on_focus': True, 'flash_lone_windows': 'always'}
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/flashfocus/config.py", line 188, in validate_config
    validated = ConfigSchema(strict=True).load(config)
TypeError: __init__() got an unexpected keyword argument 'strict'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/flashfocus", line 12, in <module>
    sys.exit(cli())
  File "/home/michael/.local/lib/python3.7/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/michael/.local/lib/python3.7/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/home/michael/.local/lib/python3.7/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/michael/.local/lib/python3.7/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/lib/python3.7/site-packages/flashfocus/ui.py", line 108, in cli
    init_server(kwargs)
  File "/usr/lib/python3.7/site-packages/flashfocus/ui.py", line 122, in init_server
    config = load_merged_config(cli_options)
  File "/usr/lib/python3.7/site-packages/flashfocus/config.py", line 287, in load_merged_config
    config = merge_config_sources(cli_options, user_config, default_config)
  File "/usr/lib/python3.7/site-packages/flashfocus/config.py", line 229, in merge_config_sources
    validated_config = validate_config(config)
  File "/usr/lib/python3.7/site-packages/flashfocus/config.py", line 191, in validate_config
    validated = ConfigSchema().load(config)
  File "/usr/lib/python3.7/site-packages/marshmallow/schema.py", line 684, in load
    data, many=many, partial=partial, unknown=unknown, postprocess=True
  File "/usr/lib/python3.7/site-packages/marshmallow/schema.py", line 824, in _do_load
    field_errors=field_errors,
  File "/usr/lib/python3.7/site-packages/marshmallow/schema.py", line 1104, in _invoke_schema_validators
    partial=partial,
  File "/usr/lib/python3.7/site-packages/marshmallow/schema.py", line 726, in _run_validator
    validator_func(output, original_data, partial=partial, many=many)
TypeError: check_unknown_fields() got an unexpected keyword argument 'partial'
travankor commented 5 years ago

Alternatively, could you make the xcffib/xcb stuff optional? Packaging xcffib is quite annoying due to Haskell, and Xorg will eventually be deprecated anyways in the medium/long term.

fennerm commented 5 years ago

@jubalh I can make it cleanly exit with an error in gnome, is that what you’re looking for. It won’t actually work with gnome3 unless someone contributes some code to interact with that compositor. The unfortunate thing about wayland is that we’ll need to separately add support for each wm; there is no standard api like with X.

The error you’re seeing there is actually separate bug that I think should be fixed in the latest version. You might need to force marshmallow to downgrade to 2.x.

@travankor The same thing I said about making i3ipc optional should also apply to xcffib. You could remove cffi, xcffib and from the dependencies.

jubalh commented 5 years ago

I can make it cleanly exit with an error in gnome

That would be good. Or make it exit if wayland-0 is set but not sway is used.

The error you’re seeing there is actually separate bug that I think should be fixed in the latest version. You might need to force marshmallow to downgrade to 2.x.

Actually I'm using the latest release. Downgrading to marshmallog 2.x is unfortunately not possible since the distribution package already is at 3.0.1.

travankor commented 5 years ago

The same thing I said about making i3ipc optional should also apply to xcffib. You could remove cffi, xcffib and from the dependencies.

Doesn't i3 only use the xcb backend, even though i3 can be supported through i3-ipc? So maybe prefer i3-ipc for i3 and fall back to xcb if it's not there?

fennerm commented 5 years ago

@jubalh ugh that’s unfortunate about the marshmallow version. I’m mostly done with adding support for the new version, hopefully will be patched this weekend.

@travankor Right yeah I guess you’re out of luck at the moment if you want to use i3ipc with i3. Would be easy enough to add a config option for this though.

I’ll think more about making these dependencies optional but I don’t have any great ideas at the moment.

jubalh commented 5 years ago

I’m mostly done with adding support for the new version, hopefully will be patched this weekend.

Sounds awesome! I'll wait for this then and either backport it to our package, or take the new release if you create one.

Thanks for your work.

fennerm commented 5 years ago

@jubalh I just released v2.0.5 with marshmallow v3 support, let me know if you have any issues.

EDIT: Also, could you possibly do me a favor and try to install the latest master on GNOME and see if it gives you a clean error without crashing? I don't have a good way of testing this myself

fennerm commented 5 years ago

@travankor I looked more into your suggestion to use i3ipc by default for i3 - turns out this doesn't work because i3ipc's opacity command is only available for sway. The xcffib dependency is unavoidable if you're running X11 I'm afraid.

Also I don't really see a path forward on making the display-backend dependencies optional. It would add considerable complexity to the install process which I think would be a mistake.

jubalh commented 5 years ago

I just released v2.0.5 with marshmallow v3 support, let me know if you have any issues.

This works fine now (tested in openbox) :-)

could you possibly do me a favor and try to install the latest master on GNOME and see if it gives you a clean error without crashing?

flashfocus
INFO: Detected display protocol: wayland - other
ERROR: This window manager is not supported yet.
Unrecoverable error, exiting...

Awesome! Thanks for your work!

Although i3ipc is not optional yet, on my side I'm okay with everything and you could close this bugreport.

travankor commented 5 years ago

I looked more into your suggestion to use i3ipc by default for i3 - turns out this doesn't work because i3ipc's opacity command is only available for sway. The xcffib dependency is unavoidable if you're running X11 I'm afraid.

Thanks for looking into my suggestion! Not ideal, but at least as many platforms as possible are supported. Unfortunately, I can't get this packaged for Void, but installing with pip works well.

fennerm commented 5 years ago

Awesome thanks @jubalh, i’ll include that in a patch release soon.

Sorry to hear that packaging for void proved difficult @travankor