Render the cursor state to VT #2829

zadjii-msft · 2019-09-20T20:32:18Z

Summary of the Pull Request

Render the visibility state of the cursor to the attached terminal, via the VT renderer.

PR Checklist

Closes Conpty needs to render the on/off state of the cursor #1127
I work here
Tests added/passed
[n/a] Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

The trick here is that PaintCursor is only ever called when the cursor is visible, and in PaintCursor, isOn will be on if the cursor is blinked on, and false if the cursor is blinked off. So we need to assume each frame that the cursor is not visible, and update our frame-by-frame tracker only when PaintCursor is called.

Validation Steps Performed

Used the following python script (on windows):

import os
import sys
import time # time.sleep is in seconds

# Turns VT output support on
def enable_vt_support():
    if os.name == 'nt':
        import ctypes
        hOut = ctypes.windll.kernel32.GetStdHandle(-11)
        out_modes = ctypes.c_uint32()
        ENABLE_VT_PROCESSING = ctypes.c_uint32(0x0004)
        # ctypes.addressof()
        ctypes.windll.kernel32.GetConsoleMode(hOut, ctypes.byref(out_modes))
        out_modes = ctypes.c_uint32(out_modes.value | 0x0004)
        ctypes.windll.kernel32.SetConsoleMode(hOut, out_modes)


enable_vt_support()

sys.stdout.write('\x1b[?25h[ Yo ]')
sys.stdout.flush()
time.sleep(1)

sys.stdout.write('\x1b[?25l[Peace out]')

sys.stdout.flush()
time.sleep(1)

I probably should add tests, I'm just putting this out there before I head out for the weekend.

miniksa

I am fine with this so far.

I'd like to see the promised tests around the output of the cursor state, though, before I sign off.

DHowett-MSFT · 2019-09-27T01:02:36Z

I am not a million percent happy that the renderer needs to be told when the cursor has blunk. ... but hey, I'll review this anyway 😄

DHowett-MSFT

I'm comfortable with this, but I do want to see the tests. I'll be one of the Two though.

zadjii-msft · 2019-09-30T19:47:00Z

/azp run

azure-pipelines · 2019-09-30T19:47:10Z

Azure Pipelines successfully started running 1 pipeline(s).

DHowett-MSFT · 2019-09-30T22:53:58Z

The tests Niksa wanted were added, but he's not around to s/o.

src/renderer/vt/XtermEngine.hpp

…ender-cursor-state

* Render the cursor state to VT * Remove TestPaintXterm entirely, as it's unused now

* Render the cursor state to VT * Remove TestPaintXterm entirely, as it's unused now (cherry picked from commit a9f3849)

- Fix snapping to the cursor in "Terminal Scrolling" mode (GH-2705) fixes GH-1222 PSReadline calls SetConsoleCursorPosition on each character. We try to snap. However, we'd only ever do this with the visible viewport, the viewport defined by `SCREEN_INFORMATION::_viewport`. When there's a virtual viewport in Terminal Scrolling mode, we actually need to snap the virtual viewport, so that this behavior looks more regular. (cherry picked from commit 6f4b98a) - Passthrough BEL in conpty (GH-2990) fixes GH-2952. (cherry picked from commit 6831120) - make filling chars (and, thus, erase line/char) unset wrap (GH-2831) EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with chars, we were setting the wrap flag. We need to specifically not do this on ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to the end of the line. Originally, we had a boolean `setWrap` that would mean... - **true**: if writing to the end of the row, SET the wrap value to true - **false**: if writing to the end of the row, DON'T CHANGE the wrap value ,- current wrap value | ,- fill last cell? | | ,- new wrap value | | | ,- comments |-|-|-| |1|0|0| THIS CASE WAS UNHANDLED To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have ~setWrap~ `wrap` mean the following: - **true**: if writing to the end of the row, SET the wrap value to TRUE - **false**: if writing to the end of the row, SET the wrap value to FALSE - **nullopt**: leave the wrap value as it is Closes GH-1126 (cherry picked from commit 4dd9f9c) - When reverse indexing, preserve RGB/256 attributes (GH-2987) (cherry picked from commit 847d6b5) - do not allow CUU and CUD to move "across" margins. (GH-2996) If we're moving the cursor up, its vertical movement should be stopped at the top margin. Similarly, this applies to moving down and the bottom margin. Fixes GH-2929. (cherry picked from commit 0ce08af) - Prevent the v1 propsheet from zeroing colors, causing black text on black background. (GH-2651) fixes GH-2319 (cherry picked from commit b97db63) - Render the cursor state to VT (GH-2829) (cherry picked from commit a9f3849) - Don't put NUL in the buffer in VT mode (GH-3015) (cherry picked from commit a82d6b8) - Return to ground when we flush the last char (GH-2823) The InputStateMachineEngine was incorrectly not returning to the ground state after flushing the last sequence. That means that something like alt+backspace would leave us in the Escape state, not the ground state. This makes sure we return to ground. Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was originally used for a theoretical time when we only open-sourced the parser. It's unnecessary now, and we can get rid of it. Also includes a small patch to bcz.cmd, to make sure bx works with projects with a space in their name. (cherry picked from commit 53c81a0) - Add support for passing through extended text attributes, like… (GH-2917) ... Related work items: #23678919, #23678920, #23731910, #23731911, #23731912, #23731913, #23731914, #23731915, #23731916, #23731917, #23731918

ghost · 2019-10-23T19:55:05Z

🎉Windows Terminal Preview v0.6.2951.0 has been released which incorporates this pull request.:tada:

Handy links:

fixes #1127

5461faf

zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty labels Sep 20, 2019

zadjii-msft added 2 commits September 24, 2019 14:20

This fixes the tests

9096d23

Remove TestPaintXterm entirely, as it's unused now

cbe2b6d

zadjii-msft requested review from DHowett-MSFT, carlos-zamora and miniksa September 25, 2019 22:08

miniksa reviewed Sep 26, 2019

View reviewed changes

DHowett-MSFT approved these changes Sep 27, 2019

View reviewed changes

zadjii-msft added 2 commits September 27, 2019 12:48

Merge branch 'master' into dev/migrie/b/1127-render-cursor-state

2e665c0

Add a test

26c7bd2

zadjii-msft requested a review from miniksa September 27, 2019 21:55

carlos-zamora reviewed Oct 1, 2019

View reviewed changes

src/renderer/vt/XtermEngine.hpp Outdated Show resolved Hide resolved

zadjii-msft added 2 commits October 1, 2019 14:47

Merge remote-tracking branch 'origin/master' into dev/migrie/b/1127-r…

8bdb30f

…ender-cursor-state

nit

7a97d75

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Oct 1, 2019

carlos-zamora approved these changes Oct 2, 2019

View reviewed changes

carlos-zamora merged commit a9f3849 into master Oct 2, 2019

carlos-zamora deleted the dev/migrie/b/1127-render-cursor-state branch October 2, 2019 23:11

DHowett-MSFT pushed a commit that referenced this pull request Oct 3, 2019

Render the cursor state to VT (#2829)

e035c8f

* Render the cursor state to VT * Remove TestPaintXterm entirely, as it's unused now

DHowett-MSFT mentioned this pull request Oct 6, 2019

Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25, DECTCEM) #3093

Closed

DHowett-MSFT pushed a commit that referenced this pull request Oct 17, 2019

Render the cursor state to VT (#2829)

9c15a8e

* Render the cursor state to VT * Remove TestPaintXterm entirely, as it's unused now (cherry picked from commit a9f3849)

ghost mentioned this pull request Oct 23, 2019

Conpty needs to render the on/off state of the cursor #1127

Closed

DHowett-MSFT mentioned this pull request Apr 8, 2020

ESC key press in CMD prompt #5239

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render the cursor state to VT #2829

Render the cursor state to VT #2829

zadjii-msft commented Sep 20, 2019 •

edited

miniksa left a comment

DHowett-MSFT commented Sep 27, 2019

DHowett-MSFT left a comment

zadjii-msft commented Sep 30, 2019

azure-pipelines bot commented Sep 30, 2019

DHowett-MSFT commented Sep 30, 2019

ghost commented Oct 23, 2019

Render the cursor state to VT #2829

Render the cursor state to VT #2829

Conversation

zadjii-msft commented Sep 20, 2019 • edited

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

miniksa left a comment

Choose a reason for hiding this comment

DHowett-MSFT commented Sep 27, 2019

DHowett-MSFT left a comment

Choose a reason for hiding this comment

zadjii-msft commented Sep 30, 2019

azure-pipelines bot commented Sep 30, 2019

DHowett-MSFT commented Sep 30, 2019

ghost commented Oct 23, 2019

zadjii-msft commented Sep 20, 2019 •

edited