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

Avoid using CSI S sequence for scroll down #577

Merged
merged 1 commit into from Jul 25, 2023
Merged

Avoid using CSI S sequence for scroll down #577

merged 1 commit into from Jul 25, 2023

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jul 21, 2023

After reading PowerShell/PSReadLine#724, I suspected that the use of CSI S sequence for scroll down is also the cause of #576. So I swapped it with just writing \n (inspired by PowerShell/PSReadLine#790), and it seems to work.

However, I couldn't find further information on how using the CSI S sequence would cause the problem and why it only happens in VS Code terminal. So I'm not sure if this is the best fix for the problem.

This seems to fix #576

@st0012 st0012 added the bug Something isn't working label Jul 21, 2023
@st0012
Copy link
Member Author

st0012 commented Jul 21, 2023

I couldn't find further information on how using the CSI S sequence would cause the problem and why it only happens in VS Code terminal

@tompng Do you know anything about these?

@tompng
Copy link
Member

tompng commented Jul 22, 2023

I think this fix is good 👍

CSI+S and \n

CSI+S and \n have some difference.
CSI+S: always scroll down
\n: scroll down only when cursor is at the bottom of the scroll range

Fortunately, LineEditor always moves cursor to the bottom before calling IOGate.scroll_down.

Reline::IOGate.move_cursor_down(@rest_height)
Reline::IOGate.scroll_down(val - @rest_height)

The meaning of the method is a bit changed, but I think adding a comment (limitation that ANSI.scroll_down only works when cursor is at the bottom) is enough.

Why it only happens in VS Code terminal

ANSI escape sequence has scroll-range setting.

If you run this code

# Before running this code, clear the screen with ctrl + l or command + k
use_csi_s = true # or false
scroll_range = 3..8 # or 1..8
print "\e[H" # move cursor to first row of the screen
print "\e[J" # clear screen
(1..10).each { |y| puts y }
print "\e[#{scroll_range.begin};#{scroll_range.end}r" # set scroll range
if use_csi_s
  print "\e[3S" # scroll down 3 rows using CSI+S
else
  print "\e[8H\n\n\n" # \n at the bottom of scroll range will also scroll down
end
print "\e[r" # rest scroll range
print "\e[11H" # move cursor to row 11

You will get this.

1
2
6 # ← scroll range start
7 #
8 #
  #
  #
  # ← scroll range end
9
10

The difference happens when scroll range is set to 1..y. (default is 1..screen_height)
VS Code terminal seems to be doing:

  • Scrolling with \n acts like scroll range is set to -infinity..y
  • Scrolling with CSI+S acts like scroll range is set to 1..y

Other terminal emulators(Terminal, iTerm, Alacritty) seems to be doing:

  • Acts like scroll range is set to -infinity..y in both \n and CSI+S

I don't know which behavior is correct. (I like VSCode's behavior)

@st0012
Copy link
Member Author

st0012 commented Jul 22, 2023

Thanks for the explanation!

You will get this.

I did get the result. But should it be different depending on use_csi_s's value? I don't seem to see a difference from changing it. I also didn't get different result between VS Code Terminal and iTerm2.

@st0012 st0012 marked this pull request as ready for review July 22, 2023 22:22
@st0012 st0012 requested a review from a team July 22, 2023 22:22
@tompng
Copy link
Member

tompng commented Jul 22, 2023

Sorry I forgot to write this.
With use_csi_s = true and scroll_range = 1..8, the screen will be the below in all terminal

4
5
6
7
8



9
10

But the difference is in the scroll buffer.
VS Code terminal: scroll buffer is empty.
Other: There is 1\n2\n3 hidden in the scroll buffer.

@tompng
Copy link
Member

tompng commented Jul 25, 2023

Little good difference
Left: using CSI+S, Right: using "\n"
Scenario: move cursor to bottom, clear screen with command K, then input multiline code.

csi_s.mp4

When you clear the screen using command K instead of \C-l in Terminal.app, Reline can't know when the screen is cleared, (there is no input sequence for command K) and screen might corrupt. This is same for Emacs, Vim, and other CUI applications.
But after this fix, corruption looks gone (or improved).

@tompng tompng merged commit b67ee4e into master Jul 25, 2023
60 checks passed
@tompng tompng deleted the fix-#576 branch July 25, 2023 09:39
@st0012 st0012 mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Previous output/prompt gobbled when scrolled up in VS Code terminal
2 participants