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

DCS sequences are no longer passed through in real time #17111

Closed
j4james opened this issue Apr 23, 2024 · 2 comments · Fixed by #17195
Closed

DCS sequences are no longer passed through in real time #17111

j4james opened this issue Apr 23, 2024 · 2 comments · Fixed by #17195
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 Product-Conpty For console issues specifically related to conpty

Comments

@j4james
Copy link
Collaborator

j4james commented Apr 23, 2024

Windows Terminal version

Commit 99061ee

Windows build number

10.0.19045.4291

Other Software

No response

Steps to reproduce

Run the following python script:

import sys
import time

sys.stdout.write('\033[37;40m\033[2J')
sys.stdout.flush()
sys.stdout.write('\033P2$p')
for i in range(1,11):
  sys.stdout.write('0;2;%d;0;0/' % (i*10))
  sys.stdout.flush()
  time.sleep(1)
sys.stdout.write('0;2;0;0;0\033\\')

Expected Behavior

The screen should clear, and the background should then slowly animate from dark red to bright red over a period of 10 seconds. This works as expected in version 1.20.10822.0.

Actual Behavior

Using a recent commit from the main branch, the screen now just remains black for 10 seconds - it never changes to red.

The problem is that the DCS sequence that alters the palette is not being passed through to the conpty client as it's received, and by the time the DCS sequence terminates, the palette will have been reset to black, so it appears to have no effect.

I believe this broke in commit 1ede023.

@j4james j4james added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 23, 2024

This comment was marked as resolved.

@DHowett
Copy link
Member

DHowett commented Apr 23, 2024

CORKING STRIKES AGAIN.

@carlos-zamora carlos-zamora added Product-Conpty For console issues specifically related to conpty Area-VT Virtual Terminal sequence support and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 25, 2024
@carlos-zamora carlos-zamora added this to the Terminal v1.22 milestone Apr 25, 2024
github-merge-queue bot pushed a commit that referenced this issue May 6, 2024
When the `DCS` passthrough code was first implemented, it relied on the
`ActionPassThroughString` method flushing the given string immediately.
However, that has since stopped being the case, so `DCS` operations end
up being delayed until the entire sequence has been parsed.

This PR fixes the issue by introducing a `flush` parameter to force an
immediate flush on the `ActionPassThroughString` method, as well as the
`XtermEngine::WriteTerminalW` method that it calls.

## Validation Steps Performed

I've confirmed that the test case in issue #17111 now updates the color
table as soon as each color entry is parsed, instead of delaying the
updates until the end of the sequence.

Closes #17111
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label May 6, 2024
DHowett pushed a commit that referenced this issue May 6, 2024
When the `DCS` passthrough code was first implemented, it relied on the
`ActionPassThroughString` method flushing the given string immediately.
However, that has since stopped being the case, so `DCS` operations end
up being delayed until the entire sequence has been parsed.

This PR fixes the issue by introducing a `flush` parameter to force an
immediate flush on the `ActionPassThroughString` method, as well as the
`XtermEngine::WriteTerminalW` method that it calls.

## Validation Steps Performed

I've confirmed that the test case in issue #17111 now updates the color
table as soon as each color entry is parsed, instead of delaying the
updates until the end of the sequence.

Closes #17111

(cherry picked from commit 9c16c5c)
Service-Card-Id: 92501226
Service-Version: 1.21
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 Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants