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 custom sequence bindings starting with non-modified keys #743

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MattX
Copy link

@MattX MattX commented Nov 2, 2023

Single-character bindings of keys pressed without a modifier are already allowed (emacs, vi command, vi insert), but these don't work for multi-character sequences whose first character is not modified. The current behavior for them just prints the first character.

This change modifies InputState::emacs and InputState::vi_insert to check whether an unmodified key is the beginning of a custom sequence binding before inserting.

If a sequence binding is started and fails to find a match, all printable keys pressed are emitted.

Single-character bindings of keys pressed without a modifier are already allowed ([emacs](https://github.com/kkawakam/rustyline/blob/23cb8a16135dd885a356cc243d3a8cd0da0ef0e6/src/keymap.rs#L526), [vi command](https://github.com/kkawakam/rustyline/blob/23cb8a16135dd885a356cc243d3a8cd0da0ef0e6/src/keymap.rs#L697), [vi insert](https://github.com/kkawakam/rustyline/blob/23cb8a16135dd885a356cc243d3a8cd0da0ef0e6/src/keymap.rs#L874)), but these don't work for multi-character sequences whose first character is not modified. The current behavior for them just prints the first character.

This change modifies `InputState::emacs` and `InputState::vi_insert` to check whether an unmodified key is the beginning of a custom sequence binding before inserting.

If a sequence binding is started and fails to find a match, all printable keys pressed are emitted.
@gwenn
Copy link
Collaborator

gwenn commented Nov 4, 2023

(a) Could you please provide an example (like this) ?
(b) Why not just doing this fix:

diff --git a/src/keymap.rs b/src/keymap.rs
index b7303df..d97279e 100644
--- a/src/keymap.rs
+++ b/src/keymap.rs
@@ -521,7 +521,7 @@ impl<'b> InputState<'b> {
         let (n, positive) = self.emacs_num_args(); // consume them in all cases

         let mut evt = key.into();
-        if let Some(cmd) = self.custom_binding(wrt, &evt, n, positive) {
+        if let Some(cmd) = self.custom_seq_binding(rdr, wrt, &mut evt, n, positive)? {
             return Ok(if cmd.is_repeatable() {
                 cmd.redo(Some(n), wrt)
             } else {

?

@MattX
Copy link
Author

MattX commented Nov 16, 2023

Sorry about the delay! I've added an example, let me know if more is needed.

The way you're proposing works just as well for what I need, but I think it changes the existing behavior by making built-in sequences overridable. I figured this could be surprising for existing users. I can change to your proposed fix if you think it's better.

@gwenn
Copy link
Collaborator

gwenn commented Nov 16, 2023

it changes the existing behavior by making built-in sequences overridable. I figured this could be surprising for existing users.

I checked other libraries and it seems to be the expected behavior.
But my patch needs some work because it has a side effect => it may consume bytes which don't match custom sequence but would have match a built-in sequence (like Ctrl-X something)...

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.

2 participants