-
Notifications
You must be signed in to change notification settings - Fork 124
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
Experiment: state: support pure virtual modifiers #450
base: master
Are you sure you want to change the base?
Conversation
Referencing #36 here too since there's significant overlap. |
Yeah, nice. I think this is definitely the right approach. |
52ad82a
to
0c66d0a
Compare
Rebased and added first dedicated tests. Since this has the potential to breaks things, I would proceed with care. But honestly I feel it looks pretty solid for now. Maybe modify more tests much like |
This will require #512 for a full implementation. |
@bluetech Compatibility: e.g. So any usual modifier will have to keep its vmod/rmod mapping, probably forever. However, for unusual mods such as |
(Sorry again for possibly uninformed questions, I really did forget most of it...) The Alt vmod sets Mod1 rmod. My question is not about removing this link, this should definitely stay for compatibility. My question is, this patch only makes pure vmods show up in the effective mask, hence Alt (not a pure vmod) still won't show up in the effective mask. What's the reason for restricting this to pure vmods? Why not have Alt set both Mod1 and Alt in the effective mask? |
I guess the answer is normalization. It’s easier to work with the canonical representation, i.e. with only real modifiers, rather than a mix of both types. Or maybe I am just used to it. A good argument however is that this has always been so in both xkbcommon and X11. We could start to provide vmod in the effective mask, but we should not depend on it when updating the state in client: indeed, there is no clue of what version of xkbcommon the compositor is using, so if it is an older one then keyboard handling will be broken in the client. By the way, we should revise the terminology. Or better expressed, let’s not introduce a new one with “pure”. I propose:
By this new terminology, the type of modifier may change depending on the keymap, except of course for the core real modifiers, which are predefined. |
back from a few sick days, still a bit hazy in the brain... the terminology is definitely a problem here and I'm not happy with it. real modifiers are actual modifiers that have effects on the keymap. virtual modifiers as we have them now are really just aliases - unless they alias a real modifier they do not exist in the keymap, regardless whether there are types for it, etc. It's a bit counterintuitive because I can set up key mappings to handle everything based on a virtual modifier but unless it's mapped to e.g. Mod3 nothing happens. and everything built in the last few decades relies on that. "pure virtual modifiers" (using this so we know what I refer to) are somewhat a new concept that didn't exist before, I don't think we should use the same name as for real modifiers that existed before since the pure ones e.g. won't show up under x11. If I had a time machine, I'd name the current virtual modifiers "modifier aliases" or something but then again, if I had a time machine I doubt I'd busy myself hacking on XKB related infrastructure :) In the key maps the pure vmods will still show up as Also, core/non-core in X is core protocol/XKB extension so that term is mostly out, too much documentation out there that uses those terms.
I've paged most of this out too unfortunately but that also requires clients to keep track of an effective duplicate state in the modifier mask which seems unnecessary. Plus it was safer to just not touch anything the way it works right now :) |
I would prefer to avoid “pure”, as it may imply that the rest is “impure”1. I propose the following terminology:
EDIT: after a second thought “canonical modifier” in place of “state modifier” seems the right call after all.
Random thoughts that I discarded:
Footnotes
|
Traditionally, virtual modifiers were merely name aliases for real modifiers, e.g. NumLock was usually mapped to Mod2 (see modifier_map statement). Virtual modifiers that were never mapped to a real one had no effect on the keymap state. This patch introduces the concept of pure virtual modifiers, i.e. virtual modifiers that are not mapped now show up as if the were true modifiers. Note that pure virtual modifiers cannot be used in an interpret action's AnyOf() and an interpret action for a pure virtual modifier must be AnyOfOrNone() to take effect: virtual_modifiers APureMod,...; interpret a+AnyOfOrNone(all) { virtualModifier= APureMod; action= SetMods(modifiers=APureMod); }; The above adds a pure virtual modifier for keysym `a`. Interestingly, this fixes one current issue with our tests: previously the de(neo) layout level5 didn't take effect correctly, with this patch in place it now behaves.
0c66d0a
to
0421feb
Compare
Rebased + added tests. We still have to clarify the terminology. |
By the way, I noted that I would rather always normalize the internal state. Is there a use case for not doing so? Anyway, the tests do check this. |
Traditionally, virtual modifiers were merely name aliases for real modifiers, e.g. NumLock was usually mapped to Mod2 (see modifier_map statement). Virtual modifiers that were never mapped to a real one had no effect on the keymap state.
This patch introduces the concept of pure virtual modifiers, i.e. virtual modifiers that are not mapped now show up as if the were true modifiers.
Note that pure virtual modifiers cannot be used in an interpret action's AnyOf() and an interpret action for a pure virtual modifier must be AnyOfOrNone() to take effect:
The above adds a pure virtual modifier for keysym
a
.Interestingly, this fixes one current issue with our tests: previously the de(neo) layout level5 didn't take effect correctly, with this patch in place it now behaves.
TODO:
This took forever to wrap my head around it almost feels too simple now... The one test case failure was in the
de(neo)
layout but after staring at this - it actually fixes that particular issue.Related to #447
cc @wismill, @bluetech, @fooishbar