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

environ_api and window context provider classes module #157

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

RedBearAK
Copy link
Contributor

Changes

This PR contains a new API function, to allow the user to specify a session type and (Wayland) desktop environment, from limited lists, to be given as parameters to KeyContext when creating the context object. KeyContext in turn creates an instance of a "window context provider" class object while giving it the same parameters, which internally re-routes to the correct provider class object for the given environment, without KeyContext needing to know anything else.

Provides support for X11/Xorg and Wayland+GNOME environments (with the installation of either of the compatible GNOME Shell extensions).

Adding future providers will mean adding a new class in window_context.py and adding to the lists in config_api.py used by the API function to verify that the environment choice is valid, then pointing the logic in the generic provider class to the correct new provider class with the actual code to get the job done.

Checklist

  • Added tests (if necessary)
  • Updated docs or README...

This was referenced May 20, 2023
@RedBearAK
Copy link
Contributor Author

RedBearAK commented May 20, 2023

Some things I discovered caused me to rework this a bit. It's now in a good enough state that I'm using it on my main system, under Wayland.

A window context object is created outside of on_event() so that there will only be one object created per run. This is then given to the KeyContext instance that is being created on each key event, inside on_event(). So the general context object is given the device object and the window context object to pull information from.

The API function has lists of valid session types ("x11" or "wayland"), and Wayland desktop environments (which are ignored and set to None if session type is "x11"). Session type defaults to "x11" to provide backwards compatibility with any keyszer config file that doesn't use the API function yet, so nobody's existing setup should suddenly stop working just because they didn't use the new API. In other words, the "assumed" environment, if no information is given, remains X11/Xorg, to match the existing behavior inherited from xkeysnail.

The API function checks that you're giving it a valid set of values for an environment for which there is a provider available, and shows some clear errors in the verbose logging about what would be valid for each argument, if you get it wrong.

If the values are strings they get casefolded, so the user can accidentally capitalize a value such as "Wayland" instead of "wayland", and it will still work.

The provider for Wayland+GNOME will separately have a useful logging message about the compatible shell extensions that would make it work. This is inherited from the previous attempt at Wayland+GNOME support.

The generic window context provider class takes care of picking and instantiating the correct actual provider object for the environment arguments it was given.

Even with quite a bit of whitespace for readability, the provider module is only a couple hundred lines, and that is including everything that was in xorg.py, turned into a class in the new module. Kind of surprising how compact it still is at this point, even though it can handle multiple methods of acquiring the window info for Wayland+GNOME.

@RedBearAK
Copy link
Contributor Author

RedBearAK commented May 21, 2023

Added the ability to let config_api construct the list of supported environments at runtime, by inspecting the window_context module's classes with a certain class method:

    @classmethod
    def get_supported_environments(cls):
        # This class supports the GNOME environment on Wayland
        return [('wayland', 'gnome')]

This should mean that the only module that would need to be modified to support additional environments would be the window_context module, where there is an abstract base class in place to show what methods any new class should implement to be functional as a provider.

As far as I know, this means nothing else in the code needs to care about what goes on in there, just that the given or assumed environment matches a provider class within that module, which then gets an instance created in transform.

It also means that the debugging log will show a list of supported environments that actually comes from the existing providers in window_context, and that list should automatically show any new supported environments as providers are added.

@RedBearAK
Copy link
Contributor Author

Latest change: Made the "redirection" logic inside the "generic" provider class constructor self-maintaining. It will analyze the provider classes and automatically redirect to the get_window_context() method inside the class that advertises a match to the incoming environment info.

This should mean that when a new provider class is added, nothing outside that new provider class has to be messed with. As long as the new class implements the required method, as fully documented in the abstract base class, to advertise the environment it supports, it will "just work". And that should include it advertising itself in the verbose logging, if the user gives a bad environment argument from config. It will be automatically shown as one of the supported options.

@RedBearAK
Copy link
Contributor Author

Stumbled on a third GNOME Shell extension "Focused Window D-Bus" and integrated support for it into the Wayland+GNOME provider. So that's three different extensions, at least one of which should always be compatible with the latest version of GNOME. For older versions of GNOME, such as that encountered in Red Hat clones (AlmaLinux 9.2 has GNOME 40.x still) and distros like Zorin OS that get themselves stuck on old Ubuntu LTS releases (GNOME 3.38.x), the Xremap extension is the only one that has support for GNOME versions that old. But it works fine.

I've been using this branch for a while now on my main system (which I upgraded from Fedora 36 to Fedora 38) and haven't had any issues, or any reason to make changes since fixing the issue with potentially missing window info. The only example of that I've seen is when the GNOME desktop has the focus. That should no longer be a problem no matter which extension is in use.

I'm working on the Wayland+KDE_Plasma provider, but that's in a new branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant