Conversation
src/libkeymap/dump.c
Outdated
| for (i = 0; i < ctx->keymap->total; i++) { | ||
| if (ctx->keywords & LK_KEYWORD_ALTISMETA && i == (i | M_ALT)) | ||
| if (skip_alt && i == (i | M_ALT)) { | ||
| fprintf(stderr, "~~~ skip alt keymap %d\n", i); |
There was a problem hiding this comment.
Yes 😄 and I have have already fixed it, along with minor formatting issues. Will sync once I have enough changes. I created the MR to let you know I am investigating, but it’s not ready (draft). The current fix is incomplete; there seem to be further issues with LK_KEYWORD_ALTISMETA.
It’s not easy to follow the logic, there are a lot of special cases.
There was a problem hiding this comment.
Will sync once I have enough changes. I created the MR to let you know I am investigating.
Thanks!
It’s not easy to follow the logic, there are a lot of special cases.
A lot of code is carried over from the days before git by the original author. A lot of legacy code :(
d412463 to
3ff73b6
Compare
|
I refactored my fix. Then I tried to compare the output of
They do not match for any table shape!! Even with my current fix. So there is definitively something fishy with the serializer (and maybe the parser too). I see the failing tests, so I have to investigate further: what I just break and how to fix master. |
|
Since loadkeys has never had the ability to dump keymap, no one has ever done such conformance tests. |
3ff73b6 to
9933688
Compare
There are two issues related when dumping a keymap in text format: - These is a conflict between skipping the keymaps in `lk_dump_keymaps` and forcing all keymaps in `lk_dump_keys`. Fixed by always including all defined keymaps, independently of the `LK_KEYWORD_ALTISMETA` flag. - The function `lk_add_constants` is not called, so implicit mappings such as `Ctrl + q = Control_q` are not set. Fixed by properly calling `lk_add_constants` before each call of `lk_dump_keymap`. `lk_add_constants` is not added to `lk_dump_keymap`, since it modifies the keymap. Note that since it is easy to forget to call `lk_add_constants`, a better fix could be to call `lk_add_constants` just before returning from `lk_parse_keymap`. Signed-off-by: Pierre Le Marre <dev@wismill.eu>
9933688 to
4b4ab37
Compare
|
OK, I think I got it: |
|
@wismill Any news about this ? Do you need any help ? |
|
@legionus just not enough freetime. |
These is a conflict between skipping the keymaps in
lk_dump_keymapsand forcing all keymaps inlk_dump_keys.Introduced a new function,
lk_dump_keymaps2, that ensures consistency between the 2 functions.Fixes #135
TODO: