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 the DECST8C escape sequence #16534

Merged

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Jan 6, 2024

Summary of the Pull Request

This PR adds support for the DECST8C escape sequence, which resets the
tab stops to every 8 columns.

Detailed Description of the Pull Request / Additional comments

This is actually a private parameter variant of the ANSI CTC sequence
(Cursor Tabulation Control), which accepts a selective parameter which
specifies the type of tab operation to be performed. But the DEC variant
only defines a single parameter value (5), which resets all tab stops.
It also considers an omitted parameter to be the equivalent of 5, so we
support that too.

Validation Steps Performed

I've extended the existing tab stop tests in ScreenBufferTests with
some basic coverage of this sequence.

I've also manually verified that the DECTABSR script in #14984 now
passes the DECST8C portion of the test.

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jan 6, 2024
@j4james j4james marked this pull request as ready for review January 6, 2024 21:58
@lhecker
Copy link
Member

lhecker commented Jan 10, 2024

BTW I didn't realize that _tabStopColumns uses a std::vector<bool>. Would it be alright to ask you to change it to a std::vector<uint8_t> or similar as part of this PR, to avoid the rather surprising bool-vector class?

@microsoft-github-policy-service microsoft-github-policy-service bot added Area-VT Virtual Terminal sequence support Product-Conhost For issues in the Console codebase labels Jan 10, 2024
@j4james
Copy link
Collaborator Author

j4james commented Jan 10, 2024

Would it be alright to ask you to change it to a std::vector<uint8_t> or similar as part of this PR, to avoid the rather surprising bool-vector class?

Done.

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 24, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit f589888 into microsoft:main Jan 24, 2024
15 checks 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 for this!

@DHowett DHowett added this to To Cherry Pick in 1.19 Servicing Pipeline via automation Jan 24, 2024
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.19 Servicing Pipeline Jan 25, 2024
DHowett pushed a commit that referenced this pull request Jan 25, 2024
## Summary of the Pull Request

This PR adds support for the `DECST8C` escape sequence, which resets the
tab stops to every 8 columns.

## Detailed Description of the Pull Request / Additional comments

This is actually a private parameter variant of the ANSI `CTC` sequence
(Cursor Tabulation Control), which accepts a selective parameter which
specifies the type of tab operation to be performed. But the DEC variant
only defines a single parameter value (5), which resets all tab stops.
It also considers an omitted parameter to be the equivalent of 5, so we
support that too.

## Validation Steps Performed

I've extended the existing tab stop tests in `ScreenBufferTests` with
some basic coverage of this sequence.

I've also manually verified that the `DECTABSR` script in #14984 now
passes the `DECST8C` portion of the test.

## PR Checklist
- [x] Closes #16533
- [x] Tests added/passed

(cherry picked from commit f589888)
Service-Card-Id: 91631721
Service-Version: 1.19
@j4james j4james deleted the feature-decst8c branch February 10, 2024 12:56
@DHowett DHowett moved this from Cherry Picked to Validated in 1.19 Servicing Pipeline Feb 21, 2024
@DHowett DHowett moved this from Validated to Shipped in 1.19 Servicing Pipeline Feb 21, 2024
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 AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase
Projects
Development

Successfully merging this pull request may close these issues.

Add support for resetting tab stops (DECST8C)
4 participants