xkbcommon / libxkbcommon

keymap handling library for toolkits and window systems
https://xkbcommon.org
Other
286 stars 125 forks source link

Add support for conditional comments #432

Open wismill opened 10 months ago

wismill commented 10 months ago

(EDIT: This need reformulation. See my comment below.

TLDR;

I propose to add support for conditional comments in the keymap text format, in order to introduce non-backward compatible features, in files parsable by both legacy XOrg tools and xkbcommon.

In a gist (syntax subject to change):

xkb_symbols "example-1" {
    // Comments starting with “@” are treated as special comments.
    // They are ignored by XOrg tools and previous versions of xkbcommon.

    // The following conditional comment starts a conditional block.
    // It guards XKB code for xkbcommon with support for multiple keysyms.

    //@if support-multiple-keysyms

    // Next comment is commented normal XKB code, parsed and interpreted only by 
    // xkbcommon versions that support both conditional comments and multiple keysyms output.
    // This allow us to support both legacy and modern XKB next to each other.

    //@  key <AD01> { [ {a, b}, Q ] };

    // Next comment is a conditional comment to guard the fallback code.

    //@else

    // The following line is normal code, always parsed by XOrg tools and old xkbcommon versions,
    // but parsed by newer version of xkbcommon only without support for multiple keysyms.
         key <AD01> { [ q, Q ] };

    // Next comment ends the conditional block.

    //@endif
}

Context

I started to look at pretty old issues such as:

I am not going to discuss them in details here. What they have in common is that they need to act against the XKB protocol. Or rather, they need to extend the protocol. Also: they are not going to be fixed in XOrg[^1].

[^1]: Well, they could, in the unlikely case somebody stands up.

XKB devs were pretty clear that going against the XKB specification is risky in general. Hard-coding ad-hoc solutions for the issues mentioned before does not seem a good idea either.

Instead, I think we should extend the protocol and the keymap text syntax. Again, I am not going to share a solution here, but rather raise a question in the following section.

Note that we already introduced a backward-incompatible syntax for multiple keysyms:

xkb_symbols "example-2" {
    key <AD01> { [ {a, b}, Q ] };
}

Problem

Extending the syntax is viable only if we can maintain backward-compatible code in xkeyboard-config. I am convinced it should be answered before elaborating any solution that extends the syntax.

Possible solutions

Keep backward-incompatible code in separate files

Keep backward-incompatible code in separate files and use different rules files depending on the supported features.

Cons: related code (e.g. new feature & its fallback) are in separate files. Also, it may duplicate rules files and requires introduce a new logic to select the proper rules file.

Create the keymap text format 2

Same issue as previously: we will have duplication of the code. But above all, this solution should be probably kept for the wonderfull XKB V2 protocol.

(Ab)use existing syntax

The action Private is currently not supported in xkbcommon and is ignored. We could start parsing it and take advantage of the unused bits in the XKB protocol.

Cons: Counter-intuitive ugly hack. Also, this would not allow to support the multiple keysyms feature.

Use a preprocessor with conditional macros, a la C

Big hammer solution.

Cons: What about XOrg tools? There is no plan to modify them. And even if it were the case, updating xkeyboard-config would require to wait that the new XOrg tools are used in most distributions, … which may never happen.

Proposed solution: use conditional special comments

The idea is similar to the C preprocessor[^2]. See the section TLDR above for the commented syntax.

[^2]: Also, some of you may remember the infamous Internet Explorer conditional comments. While ugly, they did their job.

Pros:

Cons:

wismill commented 10 months ago

So, I slept on it and now I see a major use case I did not develop sufficiently: how to gracefully handle server → client keymap transfer, in case we have a different version of xkbcommon in both.

I am going to close this issue and reformulate it properly, without proposing a specific solution.

whot commented 10 months ago

how to gracefully handle server → client keymap transfer, in case we have a different version of xkbcommon in both

very much a quickfire comment only here: the wayland protocol specifies the format as xkb_v1 so I think this use-case could be covered by adding a new version.

Alas, there is no version negotiation, so we'd have to add something like "I support keymap versions blah" to this protocol, or alternatively move into a protocol extension that supplies other protocol formats.

wismill commented 10 months ago

@whot Yes, I think this is the proper way. I really wanted to avoid this and do minimal effort to fix the aforementioned bugs. In my mind, xkb_v2 would be the glorious XKB 2, with probably a simpler text format. Maybe xkb_v1_1 would better suit. But I have no strong opinion on this.

This does not solves the issue in xkeyboard-config though. The current solution I propose here helps for the step where the compositor parses the keymap. But in order to be a proper solution, it should parse the guarded feature and its fallback, and then provide one or the other depending of the version negotiation aforementioned.

My current solution is more a hack and now I do not feel comfortable with it.

wismill commented 10 months ago

So, the gist of my approach to solve the issues above[^1][^2] in xkbcommon is to add a new boolean field “onRelease” (subject to bikeshedding) to LockMods and LockGroup actions. I also withdraw from the idea of conditional comments.

[^1]: Need to kick hotkeys on release, not press, etc. [^2]: Option to apparently unlock the Lock modifier on press of Caps Lock

As this is a backward-incompatible syntax, I opened the current ticket in order to assess the handling in xkeyboard-config of such change. It would be silly to dedicate time to develop a solution that we cannot deploy!

Let’s put the pieces together. I understand that the feasibility of extending the syntax could be achieved:

That is certainly a good amount of work!

Some further comments:

So, to me it sounds feasible this way, although this can be long way, especially in Wayland.

@whot @bluetech I would like your feedback:

whot commented 10 months ago

Recommended addition: adding parsing (but not necessarily handling) to xkbcomp. This should be simple enough once the libxkbcommon part is done and will effectively give you full coverage on anything that updates, so you effectively punt much of the backwards compatibility trickiness to distributions that need to support old combinations.

For xkeyboard-config: I would definitely advise against separate files within the installation. I think a easier-to-manage approach would be something like duplicating the whole tree via e.g. /usr/share/X11/xkb/v1.1/. Using hard/symlinks this can be reduced to a negligible size.

For Wayland: I'm not a 100% sure if it needs to be a new extension or can be added to a new version of wl_keyboard, this is something best discussed at in the wayland gitlab repo. (as an extension it'd be possible to even add a generic version negotiation extension that can be re-used by other protocols but that probably goes too far).

I was hoping we might be able to add some new action like LatchModsOnRelease but xkbcomp doesn't like that. Anyway, leaving this idea here in case it sparks something else in your head :)

Other than my hope for a new action the only ad-hoc fixes I can think of are using the Private action (we use that for XF86LogGrabInfo). The feature is very contained so going with a private action here may be a, haha, option.