kkawakam / rustyline

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

Caret key (`^`) does not work as expected in evcxr_repl in vi edit mode #645

Closed TonalidadeHidrica closed 2 years ago

TonalidadeHidrica commented 2 years ago

I originally posted the issue in google/evcxr#239 , but it turned out to be a bug of thi scrate, rustyline.

Caret key should jump the cursor to the first non-empty character in the current line. But it is implemented that it instead jumps ...

> abc def
^ here.

Seems that it instead moves the cursor to the character right after the first whitespace instead.

This seems due to a wrong implementation here:

        Cmd::Move(Movement::ViFirstPrint) => {
            s.edit_move_home()?;
            s.edit_move_to_next_word(At::Start, Word::Big, 1)?;
        }

In this implementation, it first jumps to the beginning of the line, and then move to the "next" word considering any consecutive sequence of non-empty characters as a word. This is problematic only when the first character is a non-empty character; since the cursor is already at the beginning of a "word", it jumps it to the succeeding word, causing the unintuitive behavior.

I am nst sure how should I properly fix it. s.edit_move_to_next_word eventually calls LineBuffer::next_word_pos to find the actual position to jump to. Should I modify the signature of this method to accept an additional argument for "Inclusive/Exclusive" enum? Or shuold I call a different method in the Move(ViFirstPrint) match arm?

gwenn commented 2 years ago

Thanks for the bug report.

diff --git a/src/command.rs b/src/command.rs
index c16f966..652e9f1 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -51,7 +51,9 @@ pub fn execute<H: Helper>(
         }
         Cmd::Move(Movement::ViFirstPrint) => {
             s.edit_move_home()?;
-            s.edit_move_to_next_word(At::Start, Word::Big, 1)?;
+            if s.line.starts_with(char::is_whitespace) {
+                s.edit_move_to_next_word(At::Start, Word::Big, 1)?;
+            }
         }
         Cmd::Move(Movement::BackwardChar(n)) => {
             // Move back a character.

not tested...

TonalidadeHidrica commented 2 years ago

It worked! (I modified examples/example.rs to change Emcas to Vi and ran it.)