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

Scrolling within regions that include the top of the screen doesn't push lines into scrollback #3673

Closed
aananthcn opened this issue Nov 23, 2019 · 24 comments · Fixed by #14874
Labels
Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@aananthcn
Copy link

Environment

Windows Terminal version: Version: 0.6.2951.0
Microsoft Windows [Version 10.0.18362.418]

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd]
Windows Terminal version (if applicable):

Any other software?

Steps to reproduce

  1. Install Ubuntu 18.04 (on WSL 1.0), install minicom inside Ubuntu.
  2. Open Windows Terminal and run Ubuntu 18.04
  3. Open minicom from Windows Terminal and try to capture serial console message from any embedded dev. board (say Raspberry PI)
  4. You will see messages scrolling above the view port. If you scroll up you won't see the messages from the minicom.

Expected behavior

If I scroll up, I should be able to see the serial console messages from my embedded target.

Actual behavior

Scroll up shows my command that I typed long ago. The serial console message is lost. If I run Ubuntu terminal directly (instead of doing the same from Windows Terminal), I could scroll up and see right messages.

Not sure, may be ncurses behavior is not compatible with Windows Terminal's expectations?

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 23, 2019
@j4james
Copy link
Collaborator

j4james commented Nov 23, 2019

I think this is related to the scrolling margins. You'll see a similar problem when setting the scrolling margins to the first 10 lines of the screen with a bash command like this:

printf "\e[1;10r"

Then try doing a directory listing or output a lot of content that will cause the view to scroll. The scrolling will be limited to the top 10 lines, as expected, but anything that scrolls off the top of the screen is lost.

I think this is just a limitation of the current conpty implementation. It doesn't know anything about scrolling regions, so unless you're scrolling the full screen height, it won't update the scrollback buffer correctly.

@DHowett-MSFT DHowett-MSFT added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Product-Conpty For console issues specifically related to conpty labels Nov 25, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 25, 2019
@DHowett-MSFT DHowett-MSFT added Needs-Tag-Fix Doesn't match tag requirements and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 25, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 25, 2019
@DHowett-MSFT
Copy link
Contributor

We'll need to capture some VT from minicom to figure out exactly what it's doing here.

Would you be willing to run it under script to capture a transcript? Thanks!

@DHowett-MSFT DHowett-MSFT added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 25, 2019
@ghost
Copy link

ghost commented Nov 29, 2019

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Nov 29, 2019
@ghost ghost closed this as completed Dec 2, 2019
@vdoo-connected-trust-oper

We'll need to capture some VT from minicom to figure out exactly what it's doing here.

Would you be willing to run it under script to capture a transcript? Thanks!

I'm experiencing the same issues.
Can you elaborate about running it under 'script' mode?

@ghost ghost removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Apr 2, 2020
@DHowett-MSFT
Copy link
Contributor

Sure! Check this out. If you run script, it will run another copy of your shell, but it will capture all the output that any program you run afterwards emits in raw form.

Here's the steps, roughly.

  1. Run script
    • It will say something like Script started, file is typescript
  2. You will get another copy of your shell.
  3. Run minicom, and cause the incorrect scroll behavior.
  4. Exit minicom.
  5. Exit your shell (exit)
    • Script will say something like Script done, file is typescript
  6. It made a file called "typescript"
  7. Send us that file!

The file looks something like this:

image

All of those blue characters are the raw escape sequences that came out of my shell, and for you will be the ones coming out of minicom.

@meirtsvi
Copy link

meirtsvi commented Apr 3, 2020

Attached is a script file
typescript.zip

@DHowett-MSFT
Copy link
Contributor

Thanks a lot! Just to make sure: I am seeing the bottom bar, which is red, duplicated multiple times on the screen. Is that the only issue?

If there's more issues than that, would you mind sharing a screenshot?

@DHowett-MSFT
Copy link
Contributor

(If it's the bug where the scrollback is deleted, that's just how it's designed right now, unfortunately.)

@meirtsvi
Copy link

meirtsvi commented Apr 3, 2020

Yes, the red bottom bar is fine - no problem here. It's part of minicom when enabling color mode.
The problem is indeed with the scrollback - either it's not exist or when scrolling up I see commands I ran before invoking minicom

@DHowett-MSFT
Copy link
Contributor

Ah! Thanks again for the test case. James's initial suspicion is right: there's a scrolling region, and we don't handle that great right now.

@zadjii-msft
Copy link
Member

I'm re-opening this to track the scrolling margins + conpty issue, and tossing it on the backlog. Thanks!

@zadjii-msft zadjii-msft reopened this Apr 3, 2020
@zadjii-msft zadjii-msft added Priority-2 A description (P2) and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. labels Apr 3, 2020
@zadjii-msft zadjii-msft added this to the 21H1 milestone Apr 3, 2020
@DHowett-MSFT DHowett-MSFT changed the title Incorrect scroll behavior if I use minicom on Ubuntu guest Scrolling within regions that include the top of the screen doesn't push lines into scrollback Apr 3, 2020
@ThomasHornschuh
Copy link

Hi, is there any plan to fix this issue soon?

@zadjii-msft
Copy link
Member

@ThomasHornschuh Not particularly - it's being tracked as a part of our "Next Windows Release" milestone, but that doesn't really have any sort of timeline (certainly not one I can share publicly). Sorry about that!

@Jayce3032
Copy link

(If it's the bug where the scrollback is deleted, that's just how it's designed right now, unfortunately.)

It seems that I met the same issue, as below, the scroll bar seems be fixed length when i use cmd minicom,as right window.
image

@CoryC3K
Copy link

CoryC3K commented Oct 13, 2021

Just writing to say I found this post googling the same issue.

  • In Windows terminal, SSH'd into a rPi, running minicom results in not being able to see previous output from minicom. If you scroll up it shows the bash command before minicom launched, but not the output from minicom.

  • In Visual Studio Code Terminal, SSH'd into the same rPi, running minicom results in being able to scroll up and down like normal, and see the full history of minicom.

  • In a vanilla windows command prompt (as well as powershell), SSH/rPi/minicom works as expected (you can mousewheel up to see previous minicom output).

It's only in Windows Terminal where it's all screwy and annoying. :(

Thanks for the hard work though guys! I'm a big fan of terminal!

@julee
Copy link

julee commented Mar 19, 2022

Any news for this issue?
Seems this bug cause minicom under wsl 1 cannot scroll up. Use vscode terminal to wsl, minicom can scroll up.
Seems Fluent Terminal has same problem.

@zadjii-msft
Copy link
Member

Nope. We'll make sure to update this thread when there is. In the meantime, might I recommend the Subscribe button?
image
That way you'll be notified of any updates to this thread, without needlessly pinging everyone on this thread ☺️

This is also vaguely similar to #6056. I'm xlinking just in case, but I might be totally off base on that. Telnet doesn't actually use the margins, and I believe our theory with that one is that telnet.exe is actually running its own VT parser and using the Console API to scroll the buffer. But, that might be a similar enough scenario that doesn't work in ConPTY that we may want to come up with a universal solution to both.

@j4james
Copy link
Collaborator

j4james commented Feb 7, 2023

FYI, I think I may have inadvertently fixed this issue while I was working on merging the LineFeed implementations into AdaptDispatch for issue #13408. It will probably be a while still before I have PR ready for that, and I'm not positive it'll fix this issue completely, but it looks promising so far.

@DHowett
Copy link
Member

DHowett commented Feb 7, 2023

That's quite exciting! 😄

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Feb 18, 2023
DHowett pushed a commit that referenced this issue Mar 30, 2023
The main purpose of this PR was to merge the `ITerminalApi::LineFeed`
implementations into a shared method in `AdaptDispatch`, and avoid the
VT code path depending on the `AdjustCursorPosition` function (which
could then be massively simplified). This refactoring also fixes some
bugs that existed in the original `LineFeed` implementations.

## References and Relevant Issues

This helps to close the gap between the Conhost and Terminal (#13408).
This improves some of the scrollbar mark bugs mentioned in #11000.

## Detailed Description of the Pull Request / Additional comments

I had initially hoped the line feed functionality could be implemented
entirely within `AdaptDispatch`, but there is still some Conhost and
Terminal-specific behavior that needs to be triggered when we reach the
bottom of the buffer, and the row coordinates are cycled.

In Conhost we need to trigger an accessibility scroll event, and in
Windows Terminal we need to update selection and marker offsets, reset
pattern intervals, and preserve the user's scroll offset. This is now
handled by a new `NotifyBufferRotation` method in `ITerminalApi`.

But this made me realise that the `_EraseAll` method should have been
doing the same thing when it reached the bottom of the buffer. So I've
added a call to the new `NotifyBufferRotation` API from there as well.

And in the case of Windows Terminal, the scroll offset preservation was
something that was also needed for a regular viewport pan. So I've put
that in a separate `_PreserveUserScrollOffset` method which is called
from the `SetViewportPosition` handler as well.

## Validation Steps Performed

Because of the API changes, there were a number of unit tests that
needed to be updated:

- Some of the `ScreenBufferTests` were accessing margin state in the
`SCREEN_INFORMATION` class which doesn't exist anymore, so I had to add
a little helper function which now manually detects the active margins.

- Some of the `AdapterTest` tests were dependent on APIs that no longer
exist, so they needed to be rewritten so they now check the resulting
state rather than expecting a mock API call.

- The `ScrollWithMargins` test in `ConptyRoundtripTests` was testing
functionality that didn't previously work correctly (issue #3673). Now
that it's been fixed, that test needed to be updated accordingly.

Other than getting the unit tests working, I've manually verified that
issue #3673 is now fixed. And I've also checked that the scroll markers,
selections, and user scroll offset are all being updated correctly, both
with a regular viewport pan, as well as when overrunning the buffer.

Closes #3673
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 30, 2023
@ClaireCJS
Copy link

FYI, I think I may have inadvertently fixed this issue while I was working on merging the LineFeed implementations into AdaptDispatch for issue #13408. It will probably be a while still before I have PR ready for that, and I'm not positive it'll fix this issue completely, but it looks promising so far.

Hi... It doesn't seem to have happened? I head the idea to make headers on some scripts only to find out that doing so made me lose all the output in the scrollback. (Alternate buffer isn't what I want either.)

@j4james
Copy link
Collaborator

j4james commented Nov 6, 2024

@ClaireCJS It sounds like you're trying to set a scroll region starting from line 2, e.g. if your screen height is 24, I'm guessing you're setting the scroll region to something like \e[2;24r so that line 1 remains fixed. But that's not expected to push lines into the scrollback - anything that scrolls off the top of that region will be lost.

The scenario we were trying to fix here was a scroll region like \e[1;23r, where row 24 is fixed (typically used for something like a status bar). In that case the top part of the screen is expected to push lines into the scrollback. This can only work when the first parameter of the DECSTBM sequence is 1.

@ClaireCJS
Copy link

@ClaireCJS It sounds like you're trying to set a scroll region starting from line 2, e.g. if your screen height is 24, I'm guessing you're setting the scroll region to something like \e[2;24r so that line 1 remains fixed. But that's not expected to push lines into the scrollback - anything that scrolls off the top of that region will be lost.

The scenario we were trying to fix here was a scroll region like \e[1;23r, where row 24 is fixed (typically used for something like a status bar). In that case the top part of the screen is expected to push lines into the scrollback. This can only work when the first parameter of the DECSTBM sequence is 1.

You did ascertain correctly what I was doing.

So there's just no way to do that? To me, everything should just scroll past the locked part.

This kinda destroys my concept of having headers at the top describing what is being done. Of course the tab title exists but that's not nearly as pretty (wish i could change its color to indicate status).

Though if i'm hearing you right — the idea would work at the BOTTOM of the screen, but ONLy with the last row, not with any more rows?

Basically, i just wanted to make a pretty 3 (or 4) line header at top saying what was happening, that looks like this:
Image

Is that even possible at the bottom, or am I restricted to just 1 line, not 3 like i used here?

@j4james
Copy link
Collaborator

j4james commented Nov 7, 2024

To me, everything should just scroll past the locked part.

@ClaireCJS I'm afraid that's not how margins work. You might be able to achieve something like that manually, if you're controlling the scrolling yourself, but you wouldn't be able to run a random application or shell within the margin area and just expect it to work.

If you want more info on how that could be achieved, it would probably be best to continue this conversation in the discussions area. I think we've strayed quite far off topic here.

the idea would work at the BOTTOM of the screen, but ONLy with the last row, not with any more rows?

You can lock as many rows as you want at the bottom of the screen. The only requirement is that the first row of the margin area is at the top of the screen, i.e. there can be no gap between the area you are scrolling and the scrollback buffer if you want the content to be pushed into the scrollback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging a pull request may close this issue.