Fix formatting to properly handle the Reset VT sequences that appear in the middle of a string#26424
Conversation
…middle of a string
There was a problem hiding this comment.
Pull Request Overview
This PR fixes improper handling of VT Reset sequences (\e[0m) that appear in the middle of strings during formatting operations. Reset sequences void all previous VT sequences, so the formatting system now clears any accumulated VT sequences when a Reset is encountered, ensuring proper text decoration and word wrapping behavior.
Key Changes
- Modified
GetWordsandSplitLinesmethods inComplexWriter.csto detect Reset sequences and clear accumulated VT sequences when found - Added
VtResetAddedflag toGetWordsResultstruct to track whether a Reset was automatically added, enabling proper suffix insertion - Updated
ListWriter.csto useTrimEnd()before checking for Reset sequences at line endings - Added comprehensive test coverage for various scenarios involving Reset sequences
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs |
Implements Reset sequence detection in GetWords and SplitLines methods; adds VtResetAdded field to track automatic Reset additions; removes unused System.Text.RegularExpressions import |
src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs |
Improves Reset sequence detection by trimming trailing whitespace before checking line endings |
test/powershell/engine/Formatting/PSStyle.Tests.ps1 |
Adds 7 new test cases covering word wrapping, multi-line splitting, and MatchInfo formatting with various Reset sequence combinations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SeeminglyScience
left a comment
There was a problem hiding this comment.
LGTM!
One little note of something that might be worth checking for, but maybe not worth it
| vtSeqs.Append(vtSpan); | ||
|
|
||
| wordHasVtSeqs = true; | ||
| if (vtSpan.SequenceEqual(PSStyle.Instance.Reset)) |
There was a problem hiding this comment.
Is it worth checking for `e[m as well?
There was a problem hiding this comment.
Good point. There are many other places where PSStyle.Instance.Reset is checked but not "\x1b[m". I will open an issue tracking this, and maybe do it in a separate PR.
…r in the middle of a string (PowerShell#26424)
…r in the middle of a string (PowerShell#26424)
PR Summary
Fix #25833
Also, this is a follow-up fix to #17316
When dealing with word wrapping with a string that contains VT sequences in the formatting system, the Reset VT sequences appear in the middle of the string are not handled properly, which caused the resulted string to be poorly decorated with VT sequences.
The Reset VT sequences in a string essentially void the previous VT sequences. So, any VT sequences appeared before the Reset should be cleared before processing the rest of the string.
For this script, the formatting results are as follows:
Before the fix:
After the fix:
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header