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

When changing cursor shape using DECSCUSR escape sequences, the cursor shape doesn't change until the cursor is moved #4106

Closed
daxian-dbw opened this issue Jan 3, 2020 · 2 comments · Fixed by #4896
Assignees
Labels
Area-VT Virtual Terminal sequence support 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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@daxian-dbw
Copy link
Member

Environment

Windows build number: 10.0.18363.0
Windows Terminal version (if applicable): 0.7.3451.0

Any other software?
`pwsh + PSReadLine`

Steps to reproduce

I tried to fix PowerShell/PSReadLine#1233 in PSReadLine by always applying the standard escape sequences, so the cursor shape can be changed when switching between Insert and Command modes in the VI edit mode.

A quick fix looks like this: daxian-dbw/PSReadLine@d29dad9
It works fine in the terminals on Linux and macOS.
However, in Windows Terminal and VS Code terminal, the cursor shape doesn't changed immediately after writing out the escape sequences. The change happens only after the cursor is moved.

Expected behavior

The behavior should be the same as on Linux.

  1. type Esc to switch from insert mode to command mode. Cursor shape is changed to steady block.
  2. type i to switch from command mode to insert mode. Cursor shape is changed to blinking bar.
  3. type a to insert the character a.

linux

Actual behavior

This is what happens in Windows Terminal.

  1. type Esc to switch from insert mode to command mode. Cursor shape is changed to steady block.
  2. type i to switch from command mode to insert mode. Cursor shape doesn't change.
  3. type a to insert the character a. Cursor shape is changed to blinking bar. Moving the cursor with LeftArrow or RightArrow will also trigger the cursor shape change.

winterminal

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

I'm almost certain that this means conpty isn't immediately flushing the updated cursor shape to the terminal pipe.

@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-Conpty For console issues specifically related to conpty labels Jan 3, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 3, 2020
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Jan 3, 2020
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 6, 2020
@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 11, 2020

I've got this crazy theory that #2665 will fix this too, but that PR is kinda months old now. Lemme pull it into a new branch and see what happens.


note to self
Set-PSReadLineOption -EditMode vi -viModeIndicator Cursor

EDIT: It did not help. It somehow made things way way worse. I'll try this again fresh.

EDIT 2: I'm thinking it wasn't actually that branch that was much much worse. Maybe there's something between master and whatever frankenstein I'm running that actually caused the weird character erasure I'm seeing

@zadjii-msft zadjii-msft self-assigned this Mar 11, 2020
zadjii-msft added a commit that referenced this issue Mar 12, 2020
…h the frame

    ## Summary of the Pull Request

    When Conpty encounters a string we don't understand, immediately flush the frame.

    ## References

    This PR superceeds #2665. This solution is much simpler than what was proposed in that PR.
    As mentioned in #2665: "This might have some long-term consequences for #1173."

    ## PR Checklist
    * [x] Closes #2011
    * [x] Closes #4106
    * [x] I work here
    * [x] Tests added/passed
    * [n/a] Requires documentation to be updated
@ghost ghost added the In-PR This issue has a related PR label Mar 12, 2020
DHowett-MSFT pushed a commit that referenced this issue Mar 12, 2020
When ConPTY encounters a string we don't understand, immediately flush the frame.

## References

This PR supersedes #2665. This solution is much simpler than what was proposed in that PR. 
As mentioned in #2665: "This might have some long-term consequences for #1173."

## PR Checklist
* [x] Closes #2011
* [x] Closes #4106
* [x] I work here
* [x] Tests added/passed
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements 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 12, 2020
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 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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants