nushell / reedline

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

clear_screen to fit in Warp's manner #811

Closed tisonkun closed 3 months ago

tisonkun commented 3 months ago

Warp always has an upper block for the last command:

image

Currently, Reedline's clear_screen will print empty lines to move the current insert point to the most upper position, which is hidden by the block quoted.

I don't know how mysql can "clear screen" (ctrl+l) properly as shown above, but this can be a compatibility request.

tisonkun commented 3 months ago

rustyline's implementation uses self.write_and_flush("\x1b[H\x1b[J") and it seems work.

fdncred commented 3 months ago

you can try that out with keybindings to see if that makes a difference with $"(ansi cursor_home)(ansi clear_screen)"

tisonkun commented 3 months ago

@fdncred I saw the custom keybindings right now but I'm wondering what to put here ...

    let mut keybindings = reedline::default_emacs_keybindings();
    keybindings.add_binding(
        reedline::KeyModifiers::CONTROL,
        reedline::KeyCode::Char('l'),
        todo!("not yet find the effect alternative clear_screen impl"),
    );

    let mut state = Reedline::create()
        .with_validator(Box::new(..))
        .with_highlighter(Box::new(..))
        .with_edit_mode(Box::new(Emacs::new(keybindings)));
tisonkun commented 3 months ago

Perhaps EditCommand, but I don't interact with the buffer, I'd talk to the painter.

fdncred commented 3 months ago

I was giving you nushell commands to try, sorry.

It looks like the reedline clear_screen function isn't quite working like rustyline's.

    pub(crate) fn clear_screen(&mut self) -> Result<()> {
        self.stdout.queue(cursor::Hide)?;
        let (_, num_lines) = terminal::size()?;
        for _ in 0..2 * num_lines {
            self.stdout.queue(Print("\n"))?;
        }
        self.stdout.queue(MoveTo(0, 0))?; # This is go to home
        self.stdout.queue(cursor::Show)?;

        self.stdout.flush()?;
        self.initialize_prompt_position()
    }

Seems like it might should be closer to the clear_scrollback function but without the purge.

    pub(crate) fn clear_scrollback(&mut self) -> Result<()> {
        self.stdout
            .queue(crossterm::terminal::Clear(ClearType::All))?
            .queue(crossterm::terminal::Clear(ClearType::Purge))?
            .queue(cursor::MoveTo(0, 0))?
            .flush()?;
        self.initialize_prompt_position()
    }
fdncred commented 3 months ago

To mimic rustline you'd have to use MoveTo(0, 0) and then ClearType::FromCursorDown. You could play around and make your own function to see what happens. Both of these clear functions are in the painter.

One last thing related to keybindings. You could try ReedlineEvent::ExecuteHostCommand(string) with some derivative of "\x1b[H\x1b[J" that the terminal understands like in nushell it would be "\e[H\e[J" or $"(ansi cursor_home)(ansi clear_screen)". I'm guessing your best bet is to fix the clear_screen function or add another one as suggested above.

tisonkun commented 3 months ago

to fix the clear_screen function or add another one as suggested above

This should happen within reedline (so a PR). Does it what you mean ;)?

fdncred commented 3 months ago

to fix the clear_screen function or add another one as suggested above

This should happen within reedline (so a PR). Does it what you mean ;)?

I mean, you make the changes in your cloned reedline repo and see how it works. If it works well, then you can think about a PR for reedline.

tisonkun commented 3 months ago
    pub(crate) fn clear_screen(&mut self) -> Result<()> {
        self.stdout.queue(cursor::Hide)?;
        self.stdout.queue(Clear(ClearType::All))?;
        self.stdout.queue(MoveTo(0, 0))?;
        self.stdout.queue(cursor::Show)?;

        self.stdout.flush()?;
        self.initialize_prompt_position(None)
    }

This change set looks work well (for both Warp and MacOS's Terminal, I don't test other terminals since I don't have one). I'll prepare a patch later after #812 merged.

It's 11 PM now so perhaps I'll resume this task tomorrow :D

fdncred commented 3 months ago

If you want it to work like rustyline, ClearType::All is the wrong enum. It should be ClearType::FromCursorDown. Also MoveTo(0,0) would need to be first to be like rustyline.

sholderbach commented 3 months ago

History time: that we print those newlines is down to my second PR ever to reedline in #30, basically the goal was to replicate the Ctrl-L behavior from bash in a somewhat faithful manner (#28). ^L is equivalent to form-feed in ASCII so should not delete any existing content but only move that off the screen and redraw the prompt at the top of the visible area.

Not sure if \e[2J or \e[J is the right one I have seen mentions of both.

Our ClearScreen should in my view do the same as the bash clear-screen/Ctrl-L and not be the same as clear-display

tisonkun commented 3 months ago

@sholderbach Thanks for the background!

I typically have a few options to start a PR for discussions:

  1. Replace clear_screen entirely, which may cause compatibility issues with all historical considerations.
  2. Switch the manner with a feature flag, but it looks like random changes.
  3. Add a new ReedlineEvent perhaps called ClearDisplay and allow users to rebind the keybindings as I try above (we just cannot directly talk to the painter).
  4. Try to determine the terminal type and use a proper implementation.

I'll send a PR first adopt 3 and see if we can reach 4.