Fix SGR misparse of private-marker CSI sequences ending in 'm' - #230
Open
LarryHsiao wants to merge 2 commits into
Open
Fix SGR misparse of private-marker CSI sequences ending in 'm'#230LarryHsiao wants to merge 2 commits into
LarryHsiao wants to merge 2 commits into
Conversation
_csiHandleSgr() was invoked for any CSI sequence ending in 'm', regardless of a private-marker prefix (`?`, `>`, `<`, `=`). This meant xterm's own `CSI > Ps ; Ps m` (modifyOtherKeys control, unrelated to text style) had its params misread as plain SGR codes. In practice, `CSI > 4 ; 2 m` was being read as "set underline; set faint" and, since nothing ever explicitly reset it, that bogus style stuck for everything drawn afterward — every line in the terminal, including box-drawing borders, rendered underlined. Fix: when the CSI carries a private-marker prefix, route it to handler.unknownCSI() like any other unrecognized sequence, instead of into the SGR parameter loop. Plain (unprefixed) SGR is unaffected. Also dropped dart_code_metrics from dev_dependencies — an unmaintained lint dependency whose own transitive constraints made `pub get` fail to resolve on current Dart/Flutter SDKs; it's dev-only tooling, not part of the library's runtime behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An Opus-model adversarial review of the prior commit found that `_csi.prefix != null` was two bytes too broad: `_consumeCsi()` also captures `:` (colon) and `;` (semicolon) into `_csi.prefix`, since they share the same consumption range as the real private markers (`< = > ?`). A leading `;` is legal SGR for an omitted/default first parameter (e.g. `CSI ; 1 m`) — the prior guard silently dropped it to unknownCSI instead of applying bold, confirmed by running the parser. Narrowed to `_csi.prefix! >= Ascii.lessThan`, which still catches every real private marker while letting `:`/`;`-led sequences reach SGR normally. Added a regression test for the leading-`;` case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
xterm sends
CSI > 4 ; 2 mat session start (themodifyOtherKeyskeyboard-mode control). The parser dispatches purely by final byte, so it read that as SGR —4as underline,2as faint — and since nothing resets it, everything drawn afterward stayed underlined for the rest of the session.Fix: skip SGR handling when the sequence carries a private-marker prefix (
< = > ?), same as any other CSI we don't recognize. Kept;/:out of that check, since a leading;is legal SGR (e.g.CSI ; 1 m, an omitted first parameter) and would otherwise get dropped too.Found and reproduced by feeding real terminal output (from a CLI app that sends this sequence) through
Terminaldirectly and checkingCellFlags.underlineon the buffer. Two new tests cover it; full suite (115) passes.