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

Refactor the VT DSR handler to support both ANSI and private options #14206

Closed
j4james opened this issue Oct 13, 2022 · 2 comments · Fixed by #14290
Closed

Refactor the VT DSR handler to support both ANSI and private options #14206

j4james opened this issue Oct 13, 2022 · 2 comments · Fixed by #14290
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Oct 13, 2022

Description of the new feature/enhancement

Our current implementation for the Device Status Report sequence is only setup to accept ANSI status options. But eventually we'll also need a way to support private DEC options. In particularly, I'm hoping to support a couple of DEC macro status reports as part of issue #14205, so I'd like to get the framework in place for that in advance.

Proposed technical implementation details (optional)

I figured we could follow the exact same pattern that was introduced to support both ANSI and DEC modes in PR #8469.

@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Oct 13, 2022
@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 Oct 13, 2022
@lhecker lhecker added Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Oct 13, 2022
@lhecker lhecker added this to the Backlog milestone Oct 13, 2022
@DHowett
Copy link
Member

DHowett commented Oct 13, 2022

Sounds good to me! Thanks!

@DHowett DHowett added the Product-Conhost For issues in the Console codebase label Oct 13, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 13, 2022
@ghost ghost added the In-PR This issue has a related PR label Oct 25, 2022
@ghost ghost closed this as completed in #14290 Oct 25, 2022
ghost pushed a commit that referenced this issue Oct 25, 2022
The original implementation of the _Device Status Report_ sequence was
only capable of handling ANSI status queries. This PR adds the ability
to respond to private DEC queries as well.

To prove it's working as intended, I've also included support for the
DEC extended cursor position report (`DECXCPR`), which is essentially
the same as the ANSI cursor position report, but with an additional
parameter indicating the page number. Until we support paging, though,
that value is just hardcoded to 1.

## References

The method for distinguishing between ANSI options and the private DEC
options is based on the updates made to the `SM`/`RM` mode sequences in
PR #8469.

## Validation Steps Performed

I've added a couple of unit tests covering the `DECXCPR` report, and
also manually confirmed we now pass the _Extended Cursor-Position_ test
in vttest.

Closes #14206
@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 Oct 25, 2022
@ghost
Copy link

ghost commented Jan 24, 2023

🎉This issue was addressed in #14290, which has now been successfully released as Windows Terminal Preview v1.17.1023.: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
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase 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.

3 participants