Allow for merged KcCGST files and a system path

[whot]

Three additions in this branch, it sits on top of #160 otherwise, starting at commit
"Add asprintf_safe helper function"

/etc/xkb as lookup path

This adds /etc/xkb as a lookup path for system-wide keyboard configurations. Fixes #40, see my long comment on why just a single path and not a conf.d style directory.

So our lookup path sequence becomes - in that order:

• XDG_CONFIG_HOME/xkb
• $HOME/.xkb
• /etc/xkb
• /usr/share/X11/xkb

Which gives us roughly the same behaviour as most other tools that have config files.

xkbcli scaffold tool

All directories still need to adhere to the same folder structure since the folder names and file names are part of the API. To make it simpler to get started with a custom XKB layout, there's a new tool called xkbcli scaffold. So a user would run, e.g.

$ xkbcli scaffold --user --layout "us(myvariant)" --option "custom:foo"

And that would create the required bits in $XDG_CONFIG_HOME/xkb. All they need now is learn how XKB files work and fill them in :) Passing --system does the same in the new /etc/xkb folder. Otherwise it's a fairly simple tool and it only writes to files that don't yet exist so it's safe to call twice (though ineffective).

Merge-include behaviour for KcCGST files

And since it's currently not possible to have us(myvariant) (see the same comment), change the KcCGST include behavior to run through all include paths.
So an include statement like include "us(basic)" will be attempted in in all symbols/us files in the include paths until the first one that works is found. It's thus possible to have a custom variant extending a system layout like this:

$ cat $XDG_CONFIG_HOME/xkb/symbols/us
partial alphanumeric_keys modifier_keys
xkb_symbols "myvariant" {
    name[Group1]= "myvariant (us)";
    include "us(basic)"
    key <CAPS> { [ Escape ] };
};

And this does the right thing.

cc @fooishbar and @svu because while I think this is save to do, I'm not sure if there are hidden side effects I haven't though of.

[bluetech]

I'll review this when I get the next chance. In the mean time a rebase would be good.

Is this something you think 0.11.0 should include, or is it fine to release without?

[whot]

Rebased on top of #173 now.

I renamed the tool to xkbcli-scaffold-new-layout but marked it install: false until we know it's useful. Rest is the same but has seen a few improvements in the code handling. Marking it as ready

Is this something you think 0.11.0 should include, or is it fine to release without?

I think we should include it. 0.10.0 added the user-specific files but they were only usable in a narrow context. With libxkbregistry and this the handling of custom files is complete (I think) and we have a good replacement for xmodmap (and of course custom/experimental keyboard layouts). If we get that in 0.11 in one go, life is a bit easier for everyone including writing documentation.

At least on my TODO list the only thing left now is to fix the xkeyboard-config format to be less awkward, but that's not going to happen anytime soon...

[bluetech]

Hi @whot, I looked at the changes and they mostly look good to me. Sorry for the delay.

Note that it needs a rebase.

One high level comment regarding the KcCGST merging behavior:

The strategy you've used for this is to reduce the granularity from the file level to the map level. This is good. The first file which has a matching map is used. Another alternative is to merge the maps of all files together. XKB has a bunch of merging machinery for this, with 3 different merge keywords augment, override, replace. Have you considered using that somehow instead of first-wins? I haven't considered it deeply myself, but wanted to mention it.

[bluetech]

Perhaps we can skip this envvar, unless it's really needed?

But if we keep it, xkbcommon.h has a doc section that needs to mention it (search for envvars).

There's also a include-path section there that needs to be updated for /etc/xkb in general.

[whot]

we sort-of need it for the tests. By overriding that variable we can test default include path handling without having to actually rely on correct configuration in /etc/

[bluetech]

This I think will show up with timestamps and stuff like that.

I think a print to stderr would be best, but can set a custom formatter otherwise.

[whot]

Default output is this one which I thought was good enough for a niche tool like this:

ERROR:xkbcli:Unable to resolve base directory from $XDG_CONFIG_HOME or $HOME

[bluetech]

I wonder if --user is used we should also somehow hook up /etc/xkb here? Might be a little difficult as it might (probably) not exist...

[whot]

yeah, and we'd need to run as root. And it's a niche case anyway, so overall I think it's not worth the bother. Or at least not until we see that users actually need this, right now it's all a bit of stab in the dark.

[whot]

The strategy you've used for this is to reduce the granularity from the file level to the map level. This is good. The first file which
has a matching map is used. Another alternative is to merge the maps of all files together. XKB has a bunch of merging
machinery for this, with 3 different merge keywords augment, override, replace. Have you considered using that somehow
instead of first-wins? I haven't considered it deeply myself, but wanted to mention it.

My understanding is that those keywords apply at the map level, not at the file level. Technically, the "layout is the filename" is an implementation detail of xkeyboard-config and our parsers, there is nothing that requires this explicitly. We could switch to sqlite with the layout as index and it'd be correct too. So augment is really just include-with-custom-merge-behaviour but like the pure include it doesn't explicitly specify where the data should come from.

Merging the files together thus wouldn't change much I think - if anything it makes our behaviour parser-dependent. If you have a (virtual combined) file with two identically-named sections, is the parser going to pick the first one? or the last one? or will it throw an error in that case? The current approach is a bit more expressive, you know where things should be picked up from.

We could possibly design some sort of custom-tailored include instruction set but I think it'd be overkill for what we need.

[bluetech]

My understanding is that those keywords apply at the map level, not at the file level.

They apply at a bunch of places -- search for MergeMode and OptMergeMode in src/xkbcomp/parser.y.

If you have a (virtual combined) file with two identically-named sections, is the parser going to pick the first one? or the last one? or will it throw an error in that case? The current approach is a bit more expressive, you know where things should be picked up from.

I'm not sure I understand what is parser-dependent. xkbcomp only ever looks at the XKB root, and the only other parser is us :) And this PR is changing the behavior.

Also not sure I follow about the expressiveness. The first-wins strategy is replace (in reverse?), so it can't be more expressive than something that offers more choices.

To be more concrete, let's say I want to modify something about the us layout. With the first-wins/replace strategy, the procedure is:

• Add a variant, us(myvariant) to us file
• Make myvariant include us(basic)
• Add the desired modifications to myvariant
• Add myvariant to the rules
• Configure my system to use myvariant

With a map-merging augment strategy it will be

• Add a us(basic) map to us file
• Add the desired modifications to it

The trouble with this are:

1. It changes the meaning of the us(basic) layout itself - maybe we do want to force a different name, but I think it's fine.
2. I would require changing the parsing order such that system files are processed before the user files (so the user files can augment them). I think -- maybe not, I need to remind myself of the code here...

---

To be clear, I'm not advocating for this change, I'm pretty much offloading all of the thinking on this to you 😁 But I want to make sure we discuss all options. I'm also having a deja vu that we discussed this before already but maybe it's just a glitch in the Matrix.

[whot]

Add myvariant to the rules

just ftr, this is only needed for options, not for variants due to how the rules catchall commands work. but yeah, that's just a lucky accident.

My argument of the parser dependency was: if you merge all files into a virtual combined file, you then need to also specify how the parser will treat sections that are doubly defined and ensure that all parsers (well, "both") parse correctly. By making it a lookup-path dependency, this behaviour is implied. It's a slightly wobbly argument but it's not a fully theoretical case - we recently had a bug where xkbcomp and libxkbcommon's parsing behaviour differed, see #153.

It changes the meaning of the us(basic) layout itself - maybe we do want to force a different name, but I think it's fine.

I disagree here, primarily for our benefit. Being able to modify the us layout and have it still be named us makes bug reports less reliable, let alone the various forums where custom modifications will spread like wildfire and make it more confusing for everyone. It's also a (subjective) interpretation of what layouts are and how people want to modify them. The naming of "it's a US layout but it's the foo variant" is useful and explicit enough, "it's a modified US layout" is harder. This isn't a technical argument, it's mostly one of long-term convenience for us as maintainers. And in terms of use-case: I'm aiming for the top-down configuration only. We assume the system files are constant and custom configuration can modify them or hide them altogether, but there's no bidirectional flow - you can't (easily) create a custom variant that includes a system file that then includes your custom other variant. It's probably technically possible but it's not really a use-case we should be worrying about. Where something like that is needed, it's cheaper to just modify the whole layout and effectively copy the system one into the custom configuration.

Ok, so with all that said, let's think about the technical implementation. I simply don't know where we'd put those instructions. Our components are spread across up to three files, $XDG_CONFIG_HOME, /etc/xkb and /usr/share/X11/xkb. The lookup behaviour is merely defined in the lookup paths [1] but otherwise it's not specified and it has no effect on the actual behaviour. Right now, augment should work just as well as include (not tested though).

IOW, right now there's nothing explicit about where us(basic) will come from. Picking different merge behaviours would require a place where to set those and right now, we don't really have one. I guess we could expand to allow things like include %S/us(basic) and so on but I'm not sure we really want to.

Maybe I'm missing something obvious here but right now I just don't know where we'd even configure different merge behaviours across files (and whether this would really have an effect).

[1] lookup paths are controlled by the parser, not the config. That's a potential issue but, well, uhm... don't know what to do about that either.

[whot]

Force-pushed with the requested fixed, the ones I've not marked as resolved haven't been changed - please see my comments there.

[bluetech]

Thanks for tackling this @whot! I think we're about ready for release. I'll aim for this weekend. And let's call it 1.0.0 😁

[bam80]

So our lookup path sequence becomes - in that order:

• XDG_CONFIG_HOME/xkb

• $HOME/.xkb

I think $HOME/.xkb is not necessary any more,
the random dotstyle-configs/dirs in $HOME are deprecated nowadays, being fully replaced by XDG_CONFIG_HOME path which renders to something like ${XDG_CONFIG_HOME:-$HOME/.config}:

$XDG_CONFIG_HOME defines the base directory relative to which user-specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.

https://specifications.freedesktop.org/basedir-spec/latest/#variables

So I would suggest just drop $HOME/.xkb as legacy one to teach people good practice.