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

keyboard: Make it configurable whether an action exits cursorless mode #2117

Closed
pokey opened this issue Dec 11, 2023 · 2 comments · Fixed by #2174
Closed

keyboard: Make it configurable whether an action exits cursorless mode #2117

pokey opened this issue Dec 11, 2023 · 2 comments · Fixed by #2174
Labels

Comments

@pokey
Copy link
Member

pokey commented Dec 11, 2023

It is surprising when a command exits cursorless mode. Let's turn it off by default, and support config like the following:

  "cursorless.experimental.keyboard.modal.keybindings.action": {
    "t": "setSelection",
    "b": {
      "id": "setSelectionBefore",
      "exitCursorlessMode": true,
    }
  },

Note how

  • It mimics what we already support for VSCode command config
  • It is actually defined on a per-keyboard-shortcut basis, so they could in principle have one version that exits and one that doesn't

Fwiw here is an older attempt to make list of commands that exit keyboard mode configurable #1912

@josharian does the format above match your expectations? Is id the right key?

@pokey pokey added the keyboard label Dec 11, 2023
@josharian
Copy link
Collaborator

Looks great.

@josharian josharian mentioned this issue Jan 5, 2024
1 task
josharian added a commit that referenced this issue Jan 17, 2024
Unfortunately, package.json is not as expressive as one might hope.
There doesn't appear to be a way to allow objects and yet also
restrict any strings that appear to a list of enums.

Fixes #2117
@josharian
Copy link
Collaborator

Ended up using actionId as the key, as it read better in other contexts. Happy to adjust as desired, though; we can discuss on #2174 as needed.

github-merge-queue bot pushed a commit that referenced this issue Jan 20, 2024
…2174)

Fixes #2117

## Checklist

- [-] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [-] I have updated the
[docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and
[cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet)
- [-] I have not broken the cheatsheet

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
bjaspan pushed a commit to bjaspan/cursorless that referenced this issue Jan 22, 2024
…ursorless-dev#2174)

Fixes cursorless-dev#2117

## Checklist

- [-] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [-] I have updated the
[docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and
[cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet)
- [-] I have not broken the cheatsheet

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants