Repository navigation
Conversation
EscSeqParser.isEscapeStopRune() ended a CSI sequence at "m" only, so every
other CSI sequence stayed open and the text after it was consumed as part of
the sequence, even though a CSI sequence ends at its first final byte in the
range 0x40-0x7E (ECMA-48, section 5.4).
text.Cursor and text.EraseLine emit exactly such sequences ("\x1b[A",
"\x1b[5B", "\x1b[K", ...):
StripEscape(EraseLine.Sprint() + "Ghost") -> "" (want "Ghost")
StringWidthWithoutEscSequences(CursorUp.Sprintn(3) + "abc") -> 0 (want 3)
LongestLineLen(EraseLine.Sprint() + "abc\ndef") -> 0 (want 3)
Trim(EraseLine.Sprint() + "abcdef", 3) -> "\x1b[Kabcdef"
Pad, Snip, RuneCount, Widen and InsertEveryN inherited the miscount.
A CSI sequence now ends at the first byte in 0x40-0x7E that follows the "ESC ["
introducer, and ParseSeq() ignores the CSI sequences that are not SGR
("...m"): their parameter bytes are not attribute codes, so "\x1b[2;3H"
positions the cursor instead of turning on dim, and it leaves the attributes
in force untouched.
|
Coverage Report for CI Build 41903829197Coverage remained the same at 100.0%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
EscSeqParsernow ends a CSI sequence at its final byte (0x40-0x7E, ECMA-48 section 5.4) instead of ending it atmonly, so the sequences emitted by this package's owntext.Cursor/text.EraseLine(\x1b[A,\x1b[5B,\x1b[K, ...) no longer consume the text that follows them....m) while tracking the attributes in force: their parameter bytes are not attribute codes.TestEscSeqParser_ControlSequences,TestStripEscape_CursorSequencesandTestCursorSequences.StripEscape,StringWidthWithoutEscSequences,LongestLineLen,Trim,Pad,Snip,RuneCount,WidenandInsertEveryNall measure or strip text throughEscSeqParser, so a cell holding a cursor sequence was measured as empty (width 0) and trimmed wrongly:StripEscape(EraseLine.Sprint() + "Ghost")"""Ghost"StringWidthWithoutEscSequences(CursorUp.Sprintn(3) + "abc")03LongestLineLen(EraseLine.Sprint() + "abc\ndef")03Trim(EraseLine.Sprint() + "abcdef", 3)"\x1b[Kabcdef""\x1b[Kabc"\x1b[2;3Hpositions the cursor, so it must not turn on dim and must not clear the colour in force either; the parameter bytes of a non-SGR sequence are not attribute codes.Validation: the three new tests fail on
mainbefore the change and pass after it;go test ./text/passes;gofmt -lreports nothing for the changed files.