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

Add support for querying the DECAC settings #14990

Merged
merged 2 commits into from
Mar 17, 2023

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Mar 14, 2023

This PR adds support for querying the color indices set by the DECAC
control, using the existing DECRQSS implementation.

References and Relevant Issues

The initial DECRQSS support was added in PR #11152.
The DECAC functionality was added in PR #13058, but at the time we
didn't know how to format the associated DECRQSS query.

Detailed Description of the Pull Request / Additional comments

For most DECRQSS queries, the setting being requested is identified by
the final characters of its escape sequence. However, for the DECAC
settings, you also need to include a parameter value, to indicate which
color item you're querying.

This meant we needed to extend the DECRQSS parser, so I also took this
opportunity to ensure we correctly parsed any parameter prefix chars. We
don't yet support any setting requiring a prefix, but this makes sure we
don't respond incorrectly if an app does query such a setting.

Validation Steps Performed

Thanks to @al20878, we've been able to test how these queries are parsed
on a real VT525 terminal, and I've manually verified our implementation
matches that behavior.

I've also extended the existing DECRQSS unit test to confirm that we
are responding to the DECAC queries as expected.

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-VT Virtual Terminal sequence support Product-Conhost For issues in the Console codebase labels Mar 14, 2023
@j4james j4james marked this pull request as ready for review March 14, 2023 16:27
@j4james
Copy link
Collaborator Author

j4james commented Mar 14, 2023

I think the CI test failure is just the flaky feature test timeout.

All good now.

@DHowett
Copy link
Member

DHowett commented Mar 17, 2023

I love it. Thank you! :)

@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Mar 17, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Second It's a PR that needs another sign-off label Mar 17, 2023
@DHowett DHowett merged commit 2810155 into microsoft:main Mar 17, 2023
@j4james j4james deleted the feature-decac-query branch March 21, 2023 11:09
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for querying DECAC with DECRQSS
3 participants