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

Allow keys to be mapped to sequences of commands #589

Merged
merged 7 commits into from
Nov 11, 2021

Conversation

Omnikar
Copy link
Contributor

@Omnikar Omnikar commented Aug 15, 2021

This allows the user to use keymappings like this in config.toml in order to bind to a sequence of commands:

"a" = ["open_below", "normal_mode"]

helix-term/src/keymap.rs Outdated Show resolved Hide resolved
@archseer archseer linked an issue Aug 16, 2021 that may be closed by this pull request
helix-term/src/keymap.rs Show resolved Hide resolved
helix-term/src/keymap.rs Outdated Show resolved Hide resolved
@cessen
Copy link
Contributor

cessen commented Aug 16, 2021

One design question I have (that I'm not sure we actually need to resolve in this PR) is how things like this should interact with the undo system.

Specifically, if I bind a sequence of edit commands to a key, I think I would probably expect that to get pushed onto the undo stack as a single item. The same applies to e.g. custom commands written in a plugin or in a future possibly-lisp-based config file. (Edit: and also macros. And there may be further things that just aren't coming to mind at the moment.)

Again, we may not actually need to resolve this question in this PR. Commands sequences are only intended as a stop-gap solution anyway, so I think it's okay if there's a little jank to them. But I wanted to raise the question anyway since it's at least related.

@cessen cessen mentioned this pull request Aug 19, 2021
@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 28, 2021

@cessen Yeah, I agree that it should be added to undo history as a single entry. But how would I go about doing that?

@cessen
Copy link
Contributor

cessen commented Aug 28, 2021

@cessen Yeah, I agree that it should be added to undo history as a single entry. But how would I go about doing that?

Since this is a stop-gap solution, I don't know if it's actually important to figure out before merging this PR. I mainly mentioned it just to plant the thought.

@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 28, 2021

Okay, so other than that, I think there's only this left before this PR is ready?

@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 29, 2021

If we were to address the undo behaviour, would it work to have Document store a HashMap<ViewId, bool> of whether a command sequence is active, and have methods which take a ViewId — one for beginning sequences which just sets inserts true into the hashmap for the view id, and another for ending sequences that sets the hashmap entry to false and then calls append_changes_to_history — and then change append_changes_to_history to only actually do anything if the bool corresponding to the view id is false?

@archseer
Copy link
Member

I would skip the undo behavior right now but this should be doable by either composing into a previous transaction in history, or accumulating changes before calling append_changes_to_history in a similar vein to insert mode.

@archseer
Copy link
Member

Can you add an usage example to book/remapping.md?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop-gap support for command sequences/macros in keymaps
4 participants