nushell / reedline

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

Attempt to fix multiline prompt resize issue #841

Closed blindFS closed 1 month ago

blindFS commented 1 month ago

Tried to fix #809 in this PR.

The problem lies in the handle_resize function https://github.com/nushell/reedline/blob/4634f71705785a772c894aa45e30db88f3c7a8d5/src/painting/painter.rs#L475-L492 where the new position is set according to cursor, while the cursor's row is larger than the prompt start row in multiline cases.

fdncred commented 1 month ago

Thanks for taking this on. We've had prompt issues for a long time and a while back (incorrectly) disabled some of it. What I've learned since then is some terminals, when resized, leave it to the shell to redraw things. Ghostty is one of those shells. May be related or unrelated, just thought I'd share.

I'm trying to figure how where/how to test this. Do we have an example with a multi-line prompt?

blindFS commented 1 month ago

Thanks for taking this on. We've had prompt issues for a long time and a while back (incorrectly) disabled some of it. What I've learned since then is some terminals, when resized, leave it to the shell to redraw things. Ghostty is one of those shells. May be related or unrelated, just thought I'd share.

I'm trying to figure how where/how to test this. Do we have an example with a multi-line prompt?

No test case found for the resize handling. Hope these recordings are convincing.

nu 0.98

https://github.com/user-attachments/assets/24d722d6-3ded-449f-b155-cbdf16ee97a1

nu 0.98 + fixed reedline

https://github.com/user-attachments/assets/f8412d90-5f86-4770-b71c-579108534766

blindFS commented 1 month ago

BTW, I tested it on both wezterm and alacrity. This PR probably won't fix all multiline prompt drawing issues at once, but the code I quoted above is obviously buggy, prompt_start_row will just increase over each resize op. So I think we'd better start from fixing this function.

fdncred commented 1 month ago

When I tested earlier, it seems ok on the left prompt, but the right prompt was still wrapping but not unwrapping. I'm fine with leaving that to another PR.

blindFS commented 1 month ago

When I tested earlier, it seems ok on the left prompt, but the right prompt was still wrapping but not unwrapping. I'm fine with leaving that to another PR.

You are right, the right prompt wrapping is not tackled at all. In my setup, this single line fixes the wrapping issue. Do I need to add it to this PR?

diff --git a/src/engine.rs b/src/engine.rs
index c5ae7d2..ee0d930 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -1197,6 +1197,7 @@ impl Reedline {
             ReedlineEvent::OpenEditor => self.open_editor().map(|_| EventStatus::Handled),
             ReedlineEvent::Resize(width, height) => {
                 self.painter.handle_resize(width, height);
+                self.repaint(prompt)?;
                 Ok(EventStatus::Inapplicable)
             }
             ReedlineEvent::Repaint => {

To me personally, the left prompt duplication is much more annoying because tmux new pane will trigger it.

fdncred commented 1 month ago

Maybe in another PR because I'm guessing there will be edge cases.

fdncred commented 1 month ago

Let's run with this to see how it goes this week.