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.

Sign up for GitHub

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

Jump to bottom

Cursor position seems to be mispositioned when using WLS2 & git rebase #4644

Closed
AndrejMitrovic opened this issue Feb 19, 2020 · 2 comments · Fixed by #4902
Closed

Cursor position seems to be mispositioned when using WLS2 & git rebase #4644

AndrejMitrovic opened this issue Feb 19, 2020 · 2 comments · Fixed by #4902
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@AndrejMitrovic
Copy link

Environment

Windows build number: Win32NT             10.0.18363.0 Microsoft Windows NT 10.0.18363.0
Windows Terminal version (if applicable): 0.9.433.0
Git: 2.25.0 (also reproduced with 2.17.1)
Vim: 8.0 (also reproduced with Vim 8.2)

Steps to reproduce

  • Run a WSL2 instance (Ubuntu)
  • Clone any Git repository
  • Begin a rebase git rebase -i HEAD~2

Expected behavior

The cursor should be at the beginning of the line:

Capture

Actual behavior

The cursor position is not at the beginning of the line:

Capture

Other notes:

  • When running WSL (bash.exe) through the command prompt (Linux host), it works OK. But then I'm not using Windows Terminal.
  • When running Git directly on Windows with the command prompt, it also works OK. But again, it's not using Windows Terminal.

So I think this is a Windows Terminal bug.
@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 Feb 19, 2020
@AndrejMitrovic
Copy link
Author

I have also recently upgraded to a new laptop, so this is the second machine I've been able to reproduce this on.

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Feb 19, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 19, 2020
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Feb 19, 2020
@zadjii-msft zadjii-msft mentioned this issue Feb 19, 2020
Closed
16 tasks
@DHowett-MSFT
Copy link
Contributor

Do you, per chance, know if Vim is trying to hide the cursor? If so, we are definitely ignoring that request and keeping it visible (and when we think it's hidden, we are lazier about updating its position.)

@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 Feb 24, 2020
@cinnamon-msft cinnamon-msft added v1-Scrubbed and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 28, 2020
@zadjii-msft zadjii-msft mentioned this issue Mar 13, 2020
Merged
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Mar 13, 2020
@ghost ghost closed this as completed in #4902 Mar 13, 2020
ghost pushed a commit that referenced this issue Mar 13, 2020
@zadjii-msft
Add support for setting the cursor visibility in Terminal (#4902)
38058a7 
Adds support for setting the cursor visibility in Terminal. Visibility
is a property entirely independent from whether the cursor is "on" or
not. The cursor blinker _should_ change the "IsOn" property. It was
actually changing the "Visible" property, which was incorrect. This PR
additionally corrects the naming of the method used by the cursor
blinker, and makes it do the right thing.

I added a pair of tests, one taken straight from conhost. In
copy-pasting that test, I took it a step further and implemented
`^[[?12h`, `^[[?12l`, which enables/disables cursor blinking, for the
`TerminalCore`. THIS DOES NOT ADD SUPPORT FOR DISABLING BLINKING IN THE
APP. Conpty doesn't emit the blinking on/off sequences quite yet, but
when it _does_, the Terminal will be ready.

## References
* I'd bet this conflicts with #2892
* This isn't a solution for #1379
* There shockingly isn't an issue for cursor blink state via conpty...?

## PR Checklist
* [x] Closes #3093
* [x] Closes #3499
* [x] Closes #4644
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 13, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Add support for setting the cursor visibility in Terminal microsoft/terminal
4 participants
@DHowett-MSFT @AndrejMitrovic @zadjii-msft @cinnamon-msft