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 DECSCUSR setting #17659

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Aug 3, 2024

Summary of the Pull Request

This PR adds support for querying the cursor style - technically the
state of the DECSCUSR setting - using a DECRQSS escape sequence.

References and Relevant Issues

The initial DECRQSS support was added in PR #11152, but it wasn't
practical to report the cursor style until conpty passthrough was added
in PR #17510.

Detailed Description of the Pull Request / Additional comments

If the user has chosen a cursor style that isn't one of the shapes
supported by the DECSCUSR control, we report those as 0 (i.e. the
default style). That way, if an application later tries to restore the
cursor using the returned value, it should still be reset to its
original state.

I also took the opportunity in this PR to do some refactoring of the
other DECRQSS reports, since several of them were using unnecessary
appending that could be simplified to a single fmt::format call, or
even just static strings in some cases.

Validation Steps Performed

I've checked the reports are working as expected in Vttest, and also
added some unit tests.

PR Checklist

  • Tests added/passed

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@DHowett DHowett merged commit 5174c96 into microsoft:main Aug 5, 2024
15 checks passed
@j4james j4james deleted the feature-decscusr-report branch August 8, 2024 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants