Skip to content
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

actions: Add support for multiple actions per level #487

Merged
merged 6 commits into from
Oct 11, 2024

Conversation

wismill
Copy link
Member

@wismill wismill commented Jul 12, 2024

This makes 1 keysym == 1 action holds also for multiple keysyms per level.

The motivation of this new feature are:

  • Make multiple keysyms per level more intuitive.

  • Explore how to fix the issue with shortcuts in multi-layout settings (see the xkeyboard-config issue).

    The idea is to use e.g.:

    key <LCTL> {
          symbols[1] = [ {Control_L,                  ISO_First_Group    } ],
          actions[1] = [ {SetMods(modifiers=Control), SetGroup(group=-4) } ]
    };

    in order to switch temporarily to a reference layout in order to get the same shortcuts on every layout.

    See the xkeyboard-config MR “Add option shortcuts:qwerty”.

This is quite a big change, because it breaks the design “1 action per level”.

This is WIP because there might be lots of corner cases.

I will try to break this into smaller commits, although the nature of the change makes it difficult to have smaller commits that compile.

Fixes #486


TODO:

  • Fix remaining TODO/FIXME in the code
  • Log entry

@wismill wismill added enhancement Indicates new feature requests state Indicates a need for improvements or additions to the xkb_state API labels Jul 12, 2024
@wismill wismill force-pushed the actions/multiple-per-level branch from 80c9844 to 34b9145 Compare September 23, 2024 09:36
@wismill
Copy link
Member Author

wismill commented Sep 23, 2024

Rebased and split commits (a bit). The first commit should be dropped when #507 #516 is merged.

@wismill
Copy link
Member Author

wismill commented Oct 2, 2024

This new feature can be of help for shortcuts-related issues, such as: “Switching layout temporarily while modifier is held”.

For example, using:

    key <LCTL>               {
        symbols[1] = [       {Control_L,                             ISO_First_Group } ],
        actions[1] = [       {SetMods(modifiers=Control,clearLocks), SetGroup(group=-4)}]
    };

and setting keymap RANGE_REDIRECT to 0, every shortcut involving <LCTL> will use the first layout. The RANGE_REDIRECT and -4 are necessary because of how effective group computation works: it’s an addition of the various components (base, latched, locked), so SetGroup(group=1) does not set the effective group to 1.

That said, modifying actions in symbols is not really practical. But interpret statements only support one action and it is probably better so: we want to keep 1 keysym = 1 action.

We could, however, introduce “composite” actions, e.g. RunSequence(action1(…), actions2(…), …) (probably better with an array syntax). It would still count as a single action while parsing.

But in the end, I think we need a dedicated feature that:

  • Add additional action to modifiers automatically;
  • Handle the effective group computation properly;
  • Maybe act differently if the non-modifier keysym of the shortcut is a (Latin) letter or not.

This would only internal with no special syntax, but a specialized API.

@wismill wismill force-pushed the actions/multiple-per-level branch 2 times, most recently from 5d7c75d to 4075c7a Compare October 7, 2024 17:19
@wismill
Copy link
Member Author

wismill commented Oct 7, 2024

Rebased and added exhaustive tests, that enabled to discover some memory issues. We now disallow mixing multiple modifiers or group actions, as it requires to update state handling. I may add it in this PR if it is easy enough, else will do in a further MR/release.

macOS CI is becoming very slow and fails. Looks like some issues with packaging. I do not think we had a full xorg server installed there, but now it does.

@wismill wismill force-pushed the actions/multiple-per-level branch from 4075c7a to 685138c Compare October 8, 2024 05:22
@wismill
Copy link
Member Author

wismill commented Oct 8, 2024

Rebased to fix macOS CI.

@wismill wismill force-pushed the actions/multiple-per-level branch 2 times, most recently from 49eb8f5 to b1a4c7d Compare October 8, 2024 11:41
@wismill
Copy link
Member Author

wismill commented Oct 8, 2024

Rebased after merging #526.

@wismill wismill force-pushed the actions/multiple-per-level branch from b1a4c7d to d0c49c8 Compare October 8, 2024 15:51
@wismill wismill marked this pull request as ready for review October 8, 2024 15:52
@wismill wismill requested review from whot and bluetech October 8, 2024 15:52
@wismill wismill force-pushed the actions/multiple-per-level branch from d0c49c8 to 58c4149 Compare October 8, 2024 17:46
@wismill
Copy link
Member Author

wismill commented Oct 8, 2024

Note to self: we may want to merge #518 and #519 before this one. Or the contrary, whatever is easier.

@wismill wismill force-pushed the actions/multiple-per-level branch from 58c4149 to 3877267 Compare October 11, 2024 14:07
@wismill
Copy link
Member Author

wismill commented Oct 11, 2024

Rebased after merging #519.

Do not allow `{ a }` when a single `a` suffices.
The current field `u` (short for “union”) is not very descriptive.
Next commit will add multiple actions per level, so let’s rename the
keysym field to `s` (short for “symmbols”).
This makes 1 keysym == 1 action holds also for multiple keysyms per level.

The motivation of this new feature are:

- Make multiple keysyms per level more intuitive.
- Explore how to fix the issue with shortcuts in multi-layout settings
  (see the xkeyboard-config issue[^1]). The idea is to use e.g.:

  ```c
  key <LCTL> {
      symbols[1] = [ {Control_L,                  ISO_First_Group    } ],
      actions[1] = [ {SetMods(modifiers=Control), SetGroup(group=-4) } ]
  };
  ```

  in order to switch temporarily to a reference layout in order to get
  the same shortcuts on every layout.

When no action is specified, `interpret` statements are used to find
an action corresponding for *each* keysym, as expected.

For an interpretation matching Any keysym, we may get the same
interpretation for multiple keysyms. This may result in unwanted
duplicate actions. So set this interpretation only if no previous
keysym was matched with this interpret at this level, else set the
default interpretation.

For now, at most one action of each following categories is allowed
per level:
- modifier actions: `SetMods`, `LatchMods`, `LockMods`;
- group actions: `SetGroup`, `LatchGroup`, `LockGroup`.

Some examples:
- `SetMods` + `SetGroup`: ok
- `SetMods` + `SetMods`: error
- `SetMods` + `LockMods`: error
- `SetMods` + `LockGroup`: ok

[^1]: https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/issues/416
@wismill wismill force-pushed the actions/multiple-per-level branch from 3877267 to 35aa91c Compare October 11, 2024 14:19
@wismill
Copy link
Member Author

wismill commented Oct 11, 2024

Rebased after merging #518.

@wismill wismill merged commit 71d64df into xkbcommon:master Oct 11, 2024
4 checks passed
@wismill wismill deleted the actions/multiple-per-level branch October 11, 2024 14:26
@wismill wismill mentioned this pull request Oct 11, 2024
4 tasks
@wismill wismill added this to the 1.8.0 milestone Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests state Indicates a need for improvements or additions to the xkb_state API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support multiple actions for a key per level
1 participant