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

fix: adjust cursor start position for large buffers #900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qilme
Copy link

@qilme qilme commented Apr 1, 2025

@fdncred
Copy link
Contributor

fdncred commented Apr 1, 2025

Thanks for trying to make nushell and reedline better. Please help me understand how this fixes the issue. If my screen height is 50 how is queuing 50 \n's going to help? Is there a way we can easily try this in the reedline demo to see if it works?

@qilme
Copy link
Author

qilme commented Apr 1, 2025

Since reedline uses crossterm and crossterm uses ansi escape sequences to implement the function, I can use nushell to emulate the behavior.

  1. scope commands or other command judged as large buffer.If it is a lagre buffer, then prompt_start_row is 0 and nushell send "ESC [0;0H" to terminal.

  2. ansi -e "H" is the same as "ESC [0;0H". It clear my viewpoint in the Windows Terminal, vscode, alacritty and termux.

  3. Scrool up and content as high as my screen height disappears.

  4. scope commands

  5. for x in 1..50 { if $x > 30 { break }; print ""}

  6. ansi -e "H"

  7. Scrool up and content exists

@fdncred
Copy link
Contributor

fdncred commented Apr 1, 2025

But don't you have a bunch of empty lines after the buffer?

@qilme
Copy link
Author

qilme commented Apr 1, 2025

The demo does have, but the nushell I compiled after patching not.

@qilme
Copy link
Author

qilme commented Apr 1, 2025

Origin:
origin.gif
My demo:
demo.gif
My patch:
patch.gif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants