Closed RedBearAK closed 1 year ago
One big PR is not what we need... I'd recommend you learn to keep the separate features you're working on in different branches and have a PR for each. This PR should focus on just the swappable DBus support... not any of the other input timing fixes (or other things).
Are these per-user environmental variables accessible if keyszer is running as an isolated user? Are you impinging someone would manually create these environment vars when running keyszer that way? I would have guessed those variables are set by the WM at runtime and wouldn't be accessible when keyszer starts as a system service.
Are these per-user environmental variables accessible if keyszer is running as an isolated user? Are you imagining someone would manually create these environment vars when running keyszer that way? I would have guessed those variables are set by the WM at runtime and wouldn't be accessible when keyszer starts as a system service.
That is an annoyingly good question. Since I'm always working in the venv
and constantly modifying keyszer
, I've never gone through the trouble of creating a separate user and trying to get a system service working. You know the link to the service file example in the readme is still broken, right?
There is such a thing as user-level service files, that get run from the user's home. I know that I got one running in the past, while I was messing with trying to use the since-removed launch
capabilities of xkeysnail
. A user service should have access to everything from the user. But I'm guessing that wouldn't provide the protections you're trying to get from running as an isolated user.
When running keyszer
as the separate keymapper
user, would the user have access to the keyszer
log output? Or is the idea of the special user to disallow access to that information? I don't completely understand the security implications of just running as my own user with access to the input
group and uinput
.
Are you imagining someone would manually create these environment vars when running keyszer that way?
As mentioned in the Wayland thread, I was just about to put in an API function to allow the environment info to be injected from the config, for the exact reason of the environment possibly not being accessible (or something being misidentified due to some customized environment).
But I really feel like if the program can get access to the user's display server, to the point where it can read the window attributes of windows your user is interacting with, it seems like the rest of the needed environment information should theoretically be available to it before it even attempts to do that. And there is a backup for the DE detection that checks for things like "gnome-shell" or "sway" in the process list. Although if multiple users are logged in simultaneously, operating independent graphical environments that show up in the process list, that would probably not produce reliable results. But that's really an edge case, I would hope.
I'll have to try to do a system service and test without my user being in the input
group, before I can know for sure how much of a problem it will be to get the environment info.
I've been thinking about this a lot and I'm afraid the direction this is heading has all just become a bit too much. This is WAY more code than I imagined to support Weyland... just reading thru the env
stuff makes me realize this is NOT within the scope of the key mapper - we don't need to auto-detect the broader environment... that should be a configuration detail, not a auto-detection detail. If there was a second utility (Kinto?) that does all this work and then writes a config (like xorg-config), that might be ok - though honestly I'm not imagining the config really needs to be all that complex.
I also don't think I'm interested in supporting live WM/DE auto-switching directly... so here are my current thoughts:
Simply have a config knob/method for context provider class:
context_provider ("builtin/x11")
context_provider MyCustomProvider # defined in the config
That reference must refer to a class that can be instantiated that handles context with a getCurrentContext
call (or similar). The class is also responsible for managing state, staying connected to the WM, etc... and the key mapper would instantiate that class once at start-up time.
We could perhaps ship a few popular options (IF they don't increase the requirements terribly), or they all might be made available as modules you just install into your config once... If someone wanted to "switch" DE/WM they'd need to write a small script to kill/restart the key mapper in-between - OR write a MultiProvider
that attempted to do what this PR originally attempted to do. I'm just not interested in owning or maintaining all that complexity in the core software.
So to reboot this I think we'd need to:
For the key context perhaps it's no different, just another context provider.
add_context_provider("kb", "builtin/key_context")
add_context_provider("wm", "builtin/x11")
# later
ctx.kb.capslock_on
ctx.wm.name
Or perhaps key context is simply a given and your other providers inherit from it... keeping the space flat.
So most importantly we'd need to flesh out the context provider API/architecture first... then (if you're still interested) you'd port this work (each provider) over to simply being a single, self-container plug-in provider class. After those worked and we had a few people testing them we could circle back to whether to publish as external modules or part of the core app.
Of course you'd be free to keep exploring the MultiProvider idea if you wanted, but right now I'm pretty sure I don't want to become the owner of all that complexity.
Other than the complexity of env.py
and the fact that I haven't established any classes, I kind of feel like this is already the basic gist of what is happening. Technically it can all work by just manually injecting session and desktop info, (which is kind of like what you're describing) and the env.py
level of auto-adapting functionality can be pulled into the end project (like Kinto) itself.
Although it's kind of sad to me that keyszer would remain a very manual thing and make users replicate what I'm doing just to have some auto-adaptation, I can understand not wanting to take on so much intelligence in what is just supposed to be a utility that remaps keys. 👍🏽
If you're willing to work with me a bit to get to the form of what you're talking about with the classes, I'm fine with pulling env.py
out to use it elsewhere. And if Kinto also doesn't want to accept all the stuff that supports auto-adaptation, I guess I'd just end up having to do a real fork of Kinto.
I'm about 95% of the way to having a Kinto config that automatically treats all different keyboard types correctly without manual intervention, and adapts to the distro/DE as well as using globals for choices like "Want to use Sublime shortcuts in VSCode?" (and toggling multi-language modmap variations), so they can be flipped anytime the user wants, rather than rewriting so many lines of the config with sed
each time and restarting. So I don't really feel like stopping now that I've got almost everything working intelligently the way I wished it was working years ago.
Once you pull out env.py
that does all the environment analysis, do you feel like the idea of the "connector", that lets keycontext.py
remain ignorant to what is actually gathering the context, is an acceptable idea to have within the scope of keyszer? I feel like it just needs to morph a little bit to fit into the paradigm you've described, that's sort of what it's already doing.
And without env.py
involved, do you feel like the specific Wayland+GNOME module is too complex to take on board? It kind of ended up actually being simpler than the xorg
module, in my opinion. It has to connect to D-Bus, but doesn't need to do the whole "parent window" thing.
What about the part that automatically works with either of the two GNOME extensions, and adapts if you turn one off and turn another one on, without needing a restart? Seems a real shame to take that part out. It isn't really even checking anything in the environment, just trying what it knows how to try until one extension succeeds, similar to how xorg
was giving back "NO_CONTEXT" whenever there was something wrong with the X server, but would auto-recover itself when it reconnected. The extension choice could be another thing the user has to specify from config, but that seems like a real waste of the user's time when all they should need to say from config is "Use the Wayland+GNOME context provider".
Let me know your thoughts on each of these points. This was all just an experiment that went better than I had hoped, since it's all working now that the timing delay is in place.
Would this be vaguely like what you're thinking about? With the module changing to just be context.py
and containing multiple classes, or the window context class maybe being in another module, but still inheriting from KeyContext
?
Just spitballing to try and get an idea of which direction to go.
This is based on the current mainline keycontext.py
, not my version that's already using the context connector.
from .logger import debug
from ..models.key import Key
class KeyContext:
def __init__(self, device):
self._device = device
@property
def device_name(self):
return self._device.name
@property
def capslock_on(self):
return Key.LED_CAPSL in self._device.leds()
@property
def numlock_on(self):
return Key.LED_NUML in self._device.leds()
class WindowContext(KeyContext):
def __init__(self, device):
super().__init__(device)
self._X_ctx = None
def _query_window_context(self):
# cache this, think it might be expensive
if self._X_ctx is None:
self._X_ctx = get_xorg_context()
@property
def wm_class(self):
self._query_window_context()
return self._X_ctx["wm_class"]
@property
def wm_name(self):
self._query_window_context()
return self._X_ctx["wm_name"]
# generic context error, covering both X11 and Wayland
@property
def context_error(self):
self._query_window_context()
return self._X_ctx["context_error"]
do you feel like the idea of the "connector", is an acceptable idea to have within the scope of keyszer?
I don't think it's needed (as a concept)... in my world it would just be another ContextProvider... one that wrapped a bunch of others.... and I do think that a slightly more generic ContextProvider is a good abstraction... it allows one to build both simple things (small modules) AND complex things (huge auto-detection systems).
And without env.py involved, do you feel like the specific Wayland+GNOME module is too complex to take on board?
Well, I'd like to see it first, but we can probably include a few simple context providers... I do have some concerns about supporting a bunch of configs no one is actually using though... and it's weird for someone on a simple X11 system to have to install DBus libraries if they aren't using a complex DE... so that's one consideration. But maybe we don't actually have to specify them all as dependencies if they are optional configuration pieces?
What about the part that automatically works with either of the two GNOME extensions,
I'm not sure I understand exactly what we're talking about here. I'd say lets first trim off all the extraneous stuff and then see what we're left with and have another look.
I'm not sure - I feel like I need to noodle on the architecture here... in some ways the WM and keyboard context are entirely separate, yet in others it's all just part of the large pool of what one could call "context" surrounding the keypress.
I think the simplest thing (fewest changes) to do to get the ball rolling would be to let KeyContext compose WMContext within in... such that when creating a new KeyContext you'd inject your instance of the WMContext...
KeyContext.new(keyboard, wm_context_instance)
Very similar to what we have now but that wm_class and wm_name would defer to the WM context provider instance that was passed in... rather than a hard coded lib.
weird for someone on a simple X11 system to have to install DBus libraries if they aren't using a complex DE... so that's one consideration. But maybe we don't actually have to specify them all as dependencies if they are optional configuration pieces?
Yeah, might be a good point. Another detail to leave to the parent project trying to use keyszer as a tool.
I'm not sure - I feel like I need to noodle on the architecture here... in some ways the WM and keyboard context are entirely separate, yet in others it's all just part of the large pool of what one could call "context" surrounding the keypress.
In that vein, here's an iteration I talked the AI into that makes some sense to me:
from .logger import debug
from ..models.key import Key
class KeyContext:
def __init__(self):
super().__init__()
@property
def capslock_on(self, device):
return Key.LED_CAPSL in device.leds()
@property
def numlock_on(self, device):
return Key.LED_NUML in device.leds()
class WindowContext:
def __init__(self):
super().__init__()
self._X_ctx = None
def _query_window_context(self):
# cache this, think it might be expensive
if self._X_ctx is None:
self._X_ctx = get_xorg_context()
@property
def wm_class(self):
self._query_window_context()
return self._X_ctx["wm_class"]
@property
def wm_name(self):
self._query_window_context()
return self._X_ctx["wm_name"]
# generic context error, covering both X11 and Wayland
@property
def context_error(self):
self._query_window_context()
return self._X_ctx["context_error"]
class Context(KeyContext, WindowContext):
def __init__(self, device):
super().__init__()
self.device = device
I don't feel like the nomenclature of "wm" entirely does justice to the context for the window, or should I say doesn't abstract it quite enough. Since it depends on session type, window manager (in some cases) or the full-fledged desktop environment (which in the case of GNOME is not just about the Mutter window manager, the extensions have to talk to the GNOME Shell).
I like referring to it more generally as "window context". And the providers (the window context "getters") will worry about what exactly that means for each provider's intended environment.
I'm not sure - I feel like I need to noodle on the architecture here... in some ways the WM and keyboard context are entirely separate, yet in others it's all just part of the large pool of what one could call "context" surrounding the keypress.
Yeah, the idea of the code sample above would be that you'd just do Context(device)
and get access to everything available from the subclasses.
I think I'm still missing a step, which is the turning of the xorg
and Wayland+GNOME modules into classes... Not really sure how to think about that yet.
Or actually, if that's necessary, rather than the "connector" becoming a class and talking to the appropriate module?
Trying to think about how the theoretical "custom" provider coming from config might work, but my mind is a big gray fog on that front.
On the other hand, setting up an API to tell keyser "use the X11/Xorg window context provider" seems like a pretty straightforward idea. And could be automated from config, at least at startup, if config has env.py
to draw on.
Closing in favor of the cleaner class-based approach in PR #157
DRAFT - Work in Progress (but functioning)
Changes
Many changes, including the prep work for allowing new environments besides X11 to be supported with independent modules that link to
KeyContext
via a "connector" module that checks the environment (display/session type and desktop environment/window manager) to decide which window context "getter" module to use.Requires
Needs one of the GNOME shell extensions mentioned below, and the
dbus
module, among other things.Related issue
Should partially resolve issue #27, within the limited context of a Wayland+GNOME environment, with the GNOME shell extension "Window Calls Extended" or "Xremap" installed and enabled to provide the DBus interface to acquire the window attributes.
Window Calls Extended extension:
https://extensions.gnome.org/extension/4974/window-calls-extended/ https://github.com/hseliger/window-calls-extended
Xremap extension:
https://extensions.gnome.org/extension/5060/xremap/ https://github.com/xremap/xremap-gnome
These GNOME extensions don't provide a D-Bus "signal" to attach a window focus handler to. Another extension might be able to fulfill that need at some point.
Checklist