kkawakam / rustyline

Readline Implementation in Rust
https://crates.io/crates/rustyline/
MIT License
1.5k stars 173 forks source link

Allow custom sequence bindings starting with non-modified keys #743

Open MattX opened 8 months ago

MattX commented 8 months ago

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.

gwenn commented 8 months ago

(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 commented 8 months ago

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 commented 8 months ago

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)...