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

Emacs cursor displayed incorrectly #2642

Closed
rlpeacock opened this issue Sep 2, 2019 · 6 comments · Fixed by #4372
Closed

Emacs cursor displayed incorrectly #2642

rlpeacock opened this issue Sep 2, 2019 · 6 comments · Fixed by #4372
Labels
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 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

@rlpeacock
Copy link

Environment

Windows build number: [run `[Environment]::OSVersion` for powershell, or `ver` for cmd]
 Win32NT             10.0.18922.0 Microsoft Windows NT 10.0.18922.0

Windows Terminal version (if applicable):
0.4.2382.0

Any other software?
Ubuntu 18.04.2
GNU Emacs 25.2.2

Steps to reproduce

echo "abc
def" > foo
emacs foo

Press down arrow, then up arrow. Cursor is now on 'a' on first line.
Try C-e (control+e). Cursor should move to end of line but does not appear to move. Hit right arrow. Cursor moves to 'd' on 2nd line.

Press up arrow again. Press right arrow four times. Cursor remains on first character until 4th press at which point it jumps to 'd' on 2nd line again.

Clearly the cursor is moving but the display is not reflecting that.

Note that as with #283 setting terminal to rxvt or using tmux fixes the issue, however, that bug was marked as fixed in version 1903, which I have.

Expected behavior

Cursor should move in response to keystrokes.

Actual behavior

Cursor appears not to move until movement takes it to a new line.
@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 Sep 2, 2019
@DHowett-MSFT
Copy link
Contributor

version 1903, which I have

That means it's been fixed for the in-box console host. Does this reproduce if you don't use Windows Terminal, but instead just launch WSL directly?

@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 Sep 3, 2019
@rlpeacock
Copy link
Author

No, this only happens within Terminal.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Sep 3, 2019
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Sep 3, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Sep 3, 2019
@zadjii-msft
Copy link
Member

I can verify that this repros only in the terminal and not the 1903 conhost. So while this seems related to #283 & #1825, it's definitely a unique bug.

@zadjii-msft zadjii-msft removed the Needs-Attention The core contributors need to come back around and look at this ASAP. label Sep 3, 2019
@zadjii-msft zadjii-msft mentioned this issue Sep 4, 2019
Closed
@zadjii-msft zadjii-msft added the Area-VT Virtual Terminal sequence support label Sep 9, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 9, 2019
@bitcrazed bitcrazed removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 9, 2019
@egmontkob
Copy link

Here's a tiny reproducer, created by totally stripping down emacs's script log:

printf 'abcd\r\e[?12h\e[C'; sleep 1; printf '\e[C'; sleep 100

Expected behavior: The cursor is between a and b for a second, and then moves forward to between b and c.

Actual behavior: The cursor does not move forward after one second.

Somehow that \e[?12h (start blinking cursor) triggers the misbehavior with a slight delay. It has to be placed towards the end before the sleep, e.g. after the carriage return.

DEC private mode 12 doesn't seem to be supported by WT, toggling it high/low doesn't affect cursor blinking. It's also unclear to me how it should relate to DECSCUSR.

Emacs loves toggling DEC private modes 12 as well as 25 (cursor visibility) back and forth, often combined into a single escape sequence.

This bug is probably closely related to, if not the same as #3093.

@egmontkob egmontkob mentioned this issue Oct 15, 2019
Closed
@zadjii-msft zadjii-msft added Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty labels Jan 22, 2020
@zadjii-msft
Copy link
Member

I have a sneaking suspicion that this is the same bug as #4102, but I'll leave both open to make sure.

zadjii-msft added a commit that referenced this issue Jan 27, 2020
@zadjii-msft
This fixes #2642
2e48ed3
zadjii-msft added a commit that referenced this issue Jan 27, 2020
@zadjii-msft
This fixes #2642
5e3e59e 
(cherry picked from commit 2e48ed3)
@zadjii-msft zadjii-msft mentioned this issue Jan 27, 2020
Merged
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Jan 27, 2020
@zadjii-msft zadjii-msft mentioned this issue Jan 28, 2020
Closed
16 tasks
@ghost ghost closed this as completed in #4372 Jan 30, 2020
ghost pushed a commit that referenced this issue Jan 30, 2020
@zadjii-msft
A pair of fixes related to cursor movement in conpty (#4372)
7e2f51f 
## Summary of the Pull Request

This is a pair of related fixes to conpty. For both of these bugs, the root cause was that the cursor was getting set to Off in conpty. Without the `CursorBlinkerTimer`, the cursor would remain off, and frames that only had cursor movements would not update the cursor position in the terminal.

## References

## PR Checklist
* [x] Closes #4102 
* [x] Closes #2642
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Recall that there's a bunch of cursor state that's hard to parse without looking up:
* `Visibility` This controls whether the cursor is visible _at all_, regardless if it's been blinked on or off
* `Blinking` controls whether the blinker timer should do something, or leave the cursor alone.
* `IsOn`: When the cursor is blinking, this alternates between true and false. 

The trick here is that we only `TriggerCursorMoved` when the cursor is `On`, and there are some scenarios where the cursor is manually set to off. 

Fundamentally, these two bugs are similar cases, but they are triggered by different things:
* #2642 was caused by `DoSrvPrivateAllowCursorBlinking(false)` (`^[[?12l`) also manually turning the cursor off.
* #4102 was caused by the client calling `SetConsoleScreenBuffer` to change the active buffer. `win-curses` actually uses that API instead of the alt buffer.
@ghost ghost removed the In-PR This issue has a related PR label Jan 30, 2020
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jan 30, 2020
@ghost
Copy link

ghost commented Feb 13, 2020

🎉This issue was addressed in #4372, which has now been successfully released as Windows Terminal Preview v0.9.433.0.:tada:

Handy links:

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. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty 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.

A pair of fixes related to cursor movement in conpty microsoft/terminal
6 participants
@bitcrazed @egmontkob @rlpeacock @DHowett-MSFT @zadjii-msft @cinnamon-msft