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

Command History on wrong line #6546

Closed
nurbles opened this issue Jun 17, 2020 · 23 comments · Fixed by #15701
Closed

Command History on wrong line #6546

nurbles opened this issue Jun 17, 2020 · 23 comments · Fixed by #15701
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) 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-Terminal The new Windows Terminal.
Milestone

Comments

@nurbles
Copy link

nurbles commented Jun 17, 2020

Environment

Windows build number: 10.0.19041.329
Windows Terminal version (if applicable): 1.0.1401.0

Any other software? no

Steps to reproduce

Cannot reproduce on demand, but I know it occurred after dragging the window to a new size (number of rows and columns.)

Expected behavior

I was in a Command Prompt, using the Up Arrow to recall my command history.
I expected the recalled commands to appear on the current command line.

Actual behavior

The FIRST command recalled appeared on the current command line, subsequently recalled commands (I pressed Up Arrow more than once) appeared one row above but indented as if after a prompt (though the line above was previously blank.)

FWIW, this is not the first issue I've seen after resizing the window, but almost all of them go away after another resize and do not return if I try to return to the previous size and test for them. So the triggers are difficult to define, sorry. Hopefully this will at least warrant some comments near where this is handled, just in case something is noticed in the code. :-)

@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 Jun 17, 2020
@zadjii-msft
Copy link
Member

Uhg, I see this every so often, and it drives me mad. I was hoping that I had sorted most of this out in #4200, but I definitely missed something. It's a shame that this is so horribly hard to repro.

Gonna toss this in with the rest over at #5800

@zadjii-msft zadjii-msft added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jun 18, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 18, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Jun 18, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 19, 2020
@nurbles
Copy link
Author

nurbles commented Jun 19, 2020

FWIW, this just happened to me again and there was no window resizing involved. I have a CMD script that updates the test copies of the software I'm working on that emits 5 lines, does a PAUSE, then emits 17 more. My process is to stop my software, run the script and restart my software (which is actually loaded by a small Windows program that I start from the command line -- nothing of mine is a command line program.) After having down that a few times, enough to cause the window to scroll, the behavior I originally described occurs.

The terminal window HAD been MOVED but not resized at the time I saw this today.

So, I may be able to trigger this on demand, if there is anything I can do to help debug/test this, please let me know.

@mda000
Copy link

mda000 commented Jul 1, 2020

I do see something very similar to this with no resizing involved. The last couple of lines at the bottom of the terminal can become unavailable for input, and input is overdrawn one or two lines above. It happens randomly, but only rarely--a couple of time a week for me. I have not been able to reproduce the problem.

I'm more than happy to create a minidump or perform other debugging when I see this.

@nurbles
Copy link
Author

nurbles commented Jul 1, 2020

[using 1.0.1401.0] Not sure this is related but I just rebooted and used a Command Prompt tab to restore three network drives by doing "dir P:" (and Q: and R: if it matters). This, of course, caused the text in the tab to scroll for at least a few pages. I then started to enter another command and pressed ESCAPE to erase it. Instead of being erased, the input cursor jumped to just after my prompt ON THE PREVIOUS LINE. Once this happened, it happened any time I used escape to erase a command. Unfortunately, after restarting the terminal program and opening a new Command Prompt tab I was, of course, unable to get it to happen again.

Are there instructions for capturing anything useful that I can follow if I see this happening again?

@fsigalov
Copy link
Member

fsigalov commented Aug 7, 2020

I have a related repaint issue, if I go into history of command, then use left arrow to go back into it and edit it. It does edit, but the repaint is very weird.
image

@jingyu9575
Copy link

jingyu9575 commented Jan 7, 2021

Hi, I also frequently see this problem. Here are some information I know:

  • On my system when the bug happens, the text always appears 2 lines above. My pwsh prompt is 3 lines (\n 21:07 D:\Desktop WINDOWS-2G64 \n -)

output

  • It is always at the bottom of the terminal window.
  • In this screenshot, there is no scrollbar before the bug occurs. After the bug occurs, if I scroll back to the top, the text will be at the bottom of the terminal, where the prompt - should be. Maybe there are some race conditions about the scrolling?
  • I then tried the mode command which I think affects the scrolling. It seems that repeatly running it triggers this:

LUVUWqlsMn

In this screenshot, the text is at the 3rd line (2 lines lower). If I scroll back to the top, it is exactly where the prompt - should be. It seems to be the reverse problem of the original issue.

@DHowett
Copy link
Member

DHowett commented Jan 7, 2021

Using mode to set the console window size is going to corrupt your Terminal buffer. You may have noticed that it doesn't actually resize the window 😄

@KirillOsenkov
Copy link
Member

Once it gets into this state it's pretty corrupted:
CorruptedTerminal

@KirillOsenkov
Copy link
Member

I just reproduced this again.

This is how it looked like:
image

I typed msbuild /r /m /bl *.sln when the prompt was in the last row, then pressed Tab to complete, and the moment it completed it completed on the line above.

@KirillOsenkov
Copy link
Member

I just tried this exact sequence again and it didn't repro :(

@KirillOsenkov
Copy link
Member

I just noticed that when the caret is dangerously close to the bottom of the screen it sometimes scrolls one line up to bring the caret up further away from the bottom edge, and that scroll happens almost simultaneously with text output.

Is there a lock around that scroll action and the text output? Certainly feels like a race condition where something is not transactional and side effects from scrolling and output are observed before an operation completes.

I'm just speculating of course since I haven't debugged or even looked at the code.

@KirillOsenkov
Copy link
Member

Could we consider bumping the priority on this please? Surprised that corruption and inconsistency of core internal data structures is prioritized as P3. The core data structures should be rock solid and we should do tons of stress testing and fuzzing to catch these issues.

@zadjii-msft @DHowett

@zadjii-msft zadjii-msft added Priority-2 A description (P2) and removed Priority-3 A description (P3) labels Apr 19, 2021
@KirillOsenkov
Copy link
Member

Just hit this in the opposite direction with Tab completion, and there was no scrolling involved as the prompt was roughly in the center of the screen:
image

@zadjii-msft
Copy link
Member

Hooplah, a consistent repro from #9897

Steps to reproduce:

  1. Open Terminal (PowerShell)
  2. Push Enter enough times to get to the bottom of the window
  3. Enter line long enough so it will get broken when window is resized horizontally
  4. Move cursor backwards and enter a different character to mark your position
  5. Resize the window horizontally.
  6. When the line gets broken and text scrolls up observe that the cursor is not where it should be

Old console doesn't have this issue.

cmd.exe in Terminal app has a similar issue

I'll add that I remove-module psreadline's first

9897-did-repro-000

@zzsergant
Copy link

Is there any news about this issue or ETA on fix?
I mean, this issue created almost year ago and there are multiple ways to reproduce it. Isn't this a high priority bug?

For me that issue ruining terminal usage experience every day, when it garbage the output and I need to do the resize window workaround and re-run the command again.

@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 ☺️

@KirillOsenkov
Copy link
Member

Continuing to see weirdness and race conditions:

image

@Chaython

This comment has been minimized.

@zadjii-msft
Copy link
Member

terminal vs pty. Doesn't seem like #5368 affects this
image

though, same thing in just conhost:

image

OH BUT INTERESTING.

If you set conhost's buffer to be the same size as the viewport, then it'll repro exactly the same. Furthermore, same will happen with conhost+cmd.exe+buffer.size=viewport.size
gh-6546-000

@zadjii-msft
Copy link
Member

Okay I'm getting too into the weeds here I think. These feel like issues that'll just go away with #8000. There's weirdness going on where it seems like the TextBuffer circled once to leave a space for this line where the cursor should be, but the cursor is actually at the start of the prompt (and the prompt isn't in the buffer during the resize). So it ends up redrawing itself where it was, which is now a blank line. But that blank line should have never existed!

@zadjii-msft
Copy link
Member

zadjii-msft commented Aug 19, 2021

      // There is a special case here. If the row has a "wrap"
        // flag on it, but the right isn't equal to the width (one
        // index past the final valid index in the row) then there
        // were a bunch trailing of spaces in the row.
        // (But the measuring functions for each row Left/Right do
        // not count spaces as "displayable" so they're not
        // included.)
        // As such, adjust the "right" to be the width of the row
        // to capture all these spaces
        if (row.WasWrapForced())

We're hitting the case where the last row of the buffer thinks it was wrapped, so we fill all the spaces into the new buffer. But that's not really the case! The commandline was hidden, so that line shouldn't be marked as wrapped!

EDIT:

But, this makes me think that this particular repro isn't the same root cause as the other bugs. This is just the Commandline/cooked read failing to unmark a line as wrapped in one very particular scenario. This is far too specific to be relevant to the plethora of other examples here (despite not having consistent repros for those).

@zadjii-msft zadjii-msft modified the milestones: Terminal v2.0, 22H2 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog Jul 5, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 15, 2023
DHowett pushed a commit that referenced this issue Sep 26, 2023
Subjectively speaking, this commit makes 3 improvements:
* Most importantly, it now would work with arbitrary Unicode text.
  (No more `IsGlyphFullWidth` or DBCS handling during reflow.)
* Due to the simpler implementation it hopefully makes review of
  future changes and maintenance simpler. (~3x less LOC.)
* It improves perf. by 1-2 orders of magnitude.
  (At 120x9001 with a full buffer I get 60ms -> 2ms.)

Unfortunately, I'm not confident that the new code replicates the old
code exactly, because I failed to understand it. During development
I simply tried to match its behavior with what I think reflow should do.

Closes #797
Closes #3088
Closes #4968
Closes #6546
Closes #6901
Closes #15964
Closes MSFT:19446208

Related to #5800 and #8000

## Validation Steps Performed
* Unit tests ✅
* Feature tests ✅
* Reflow with a scrollback ✅
* Reflowing the cursor cell causes a forced line-wrap ✅
  (Even at the end of the buffer. ✅)
* `color 8f` and reflowing retains the background color ✅
* Enter alt buffer, Resize window, Exit alt buffer ✅
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 26, 2023
DHowett pushed a commit that referenced this issue Sep 26, 2023
Subjectively speaking, this commit makes 3 improvements:
* Most importantly, it now would work with arbitrary Unicode text.
  (No more `IsGlyphFullWidth` or DBCS handling during reflow.)
* Due to the simpler implementation it hopefully makes review of
  future changes and maintenance simpler. (~3x less LOC.)
* It improves perf. by 1-2 orders of magnitude.
  (At 120x9001 with a full buffer I get 60ms -> 2ms.)

Unfortunately, I'm not confident that the new code replicates the old
code exactly, because I failed to understand it. During development
I simply tried to match its behavior with what I think reflow should do.

Closes #797
Closes #3088
Closes #4968
Closes #6546
Closes #6901
Closes #15964
Closes MSFT:19446208

Related to #5800 and #8000

## Validation Steps Performed
* Unit tests ✅
* Feature tests ✅
* Reflow with a scrollback ✅
* Reflowing the cursor cell causes a forced line-wrap ✅
  (Even at the end of the buffer. ✅)
* `color 8f` and reflowing retains the background color ✅
* Enter alt buffer, Resize window, Exit alt buffer ✅

(cherry picked from commit 7474839)
Service-Card-Id: 90642727
Service-Version: 1.19
@alrz
Copy link

alrz commented Jul 8, 2024

This issue is kind of came back. I'm using preview 1.21.1772.0, just using a curl with a long url and making edits in the middle and hitting enter doesn't exactly send the same command as seen in history with the up key.

image

@lhecker
Copy link
Member

lhecker commented Jul 8, 2024

@alrz Could you open a new issue for this? It'd be especially nice if you had a repro case for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) 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-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.