nushell / reedline

A feature-rich line editor - powering Nushell
https://docs.rs/reedline/
MIT License
550 stars 155 forks source link

Prefix history search broken in vi-mode #772

Closed nixypanda closed 8 months ago

nixypanda commented 8 months ago

Platform macOS Terminal software kitty

Describe the problem you are observing. prefix history search is not working in vi-mode (normal) when hitting k key.

Steps to reproduce

  1. Script used to run reedline.
    
    use reedline::{
    default_vi_insert_keybindings, default_vi_normal_keybindings, DefaultPrompt, Reedline, Signal,
    Vi,
    };

fn main() { let mut line_editor = Reedline::create().with_edit_mode(Box::new(Vi::new( default_vi_insert_keybindings(), default_vi_normal_keybindings(), )));

let prompt = DefaultPrompt::default();

loop {
    let sig = line_editor.read_line(&prompt);
    match sig {
        Ok(Signal::Success(buffer)) => {
            println!("We processed: {}", buffer);
        }
        Ok(Signal::CtrlD) | Ok(Signal::CtrlC) => {
            println!("\nAborted!");
            break;
        }
        x => {
            println!("Event: {:?}", x);
        }
    }
}

}


2. Write a bunch of commands. eg. (ls rock, ls paper, sl sic, ls rofl)
3. `ls<esc><k>`

## Expected behaviour
Cycles through the commands that start with `ls`

## Actual Behaviour (bug)
Cycles through all the history commands

## Screenshots/Screencaptures

<none>

## Additional Info
Reverting https://github.com/nushell/reedline/pull/699 fixes the issue.

## Thoughts
As someone who briefly contributed to this project a while back and as someone who uses vi-mode (all the time) I really never in my life questioned why, when I hit escape nushell prompt does not go back one place.

I mean why do we want to be exactly like vim? I really think we should focus on doing vi-mode that makes sense for the terminal.
Like if we do this going back stuff and we fix the history navigation. It looks like the prefix will lose it last char in case we want to do history search based on where the cursor position is, and than this case will also require some special handling (afaik).

So, I am really questioning why even we need something like that.
fdncred commented 8 months ago

@andreistan26 your PR seems to have broken this functionality. Any thoughts on how to make both your and this prefix searching work together?

andreistan26 commented 8 months ago

Looking into it

andreistan26 commented 8 months ago

Thinking about it for a bit, @nixypanda is right, for a CLI row editor going back when switching to normal mode is annoying. I would revert that PR. If you think it should stay as it is I can think about a solution @fdncred.

fdncred commented 8 months ago

I have no problem reverting #699 if that's the right thing to do. What do you think @sholderbach?

I prepared this just in case https://github.com/nushell/reedline/pull/773

sholderbach commented 8 months ago

OK with reverting, doing the right thing around the vim cursor position differences between normal and insert mode requires a more holistic approach (see #766 ). So let's pull of the band-aid.