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

Prefix history search broken in vi-mode #772

Closed
nixypanda opened this issue Mar 16, 2024 · 5 comments
Closed

Prefix history search broken in vi-mode #772

nixypanda opened this issue Mar 16, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@nixypanda
Copy link
Contributor

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);
            }
        }
    }
}
  1. Write a bunch of commands. eg. (ls rock, ls paper, sl sic, ls rofl)
  2. ls<esc><k>

Expected behaviour

Cycles through the commands that start with ls

Actual Behaviour (bug)

Cycles through all the history commands

Screenshots/Screencaptures

Additional Info

Reverting #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.

@nixypanda nixypanda added the bug Something isn't working label Mar 16, 2024
@fdncred
Copy link
Collaborator

fdncred commented Mar 16, 2024

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

@andreistan26
Copy link
Contributor

Looking into it

@andreistan26
Copy link
Contributor

andreistan26 commented Mar 18, 2024

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
Copy link
Collaborator

fdncred commented Mar 18, 2024

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 #773

@sholderbach
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants