diff --git a/src/XTerm.NET.Tests/StyledUnderlineTests.cs b/src/XTerm.NET.Tests/StyledUnderlineTests.cs new file mode 100644 index 0000000..ae1d4cf --- /dev/null +++ b/src/XTerm.NET.Tests/StyledUnderlineTests.cs @@ -0,0 +1,232 @@ +using System.Runtime.CompilerServices; +using XTerm; +using XTerm.Buffer; +using XTerm.Common; +using XTerm.Options; + +namespace XTerm.Tests; + +/// +/// Styled underlines (SGR 4:1–4:5, 21) and underline colour (SGR 58/59). +/// +/// +/// The squiggly underline an LSP puts under an error. The style enum and sub-parameter parsing +/// already existed — the sub-parameters were being read and then dropped, so a program asking for a +/// curly underline got a straight one. +/// +public class StyledUnderlineTests +{ + private const string Esc = "\u001b"; + + private static Terminal Fresh() => new(new TerminalOptions { Cols = 20, Rows = 3 }); + + private static BufferCell FirstCell(Terminal terminal) + => terminal.Buffer.Lines[terminal.Buffer.YBase]![0]; + + [Theory] + [InlineData("4", UnderlineStyle.Single)] + [InlineData("4:0", UnderlineStyle.None)] + [InlineData("4:1", UnderlineStyle.Single)] + [InlineData("4:2", UnderlineStyle.Double)] + [InlineData("4:3", UnderlineStyle.Curly)] + [InlineData("4:4", UnderlineStyle.Dotted)] + [InlineData("4:5", UnderlineStyle.Dashed)] + [InlineData("21", UnderlineStyle.Double)] + public void Sgr_selects_the_underline_style(string sgr, UnderlineStyle expected) + { + var terminal = Fresh(); + terminal.Write($"{Esc}[{sgr}mx"); + + Assert.Equal(expected, FirstCell(terminal).Attributes.GetUnderlineStyle()); + } + + /// + /// A style nobody has defined is still an underline. Drawing a plain one is closer to what the + /// program asked for than drawing nothing at all. + /// + [Fact] + public void An_unknown_style_still_underlines() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[4:9mx"); + + Assert.Equal(UnderlineStyle.Single, FirstCell(terminal).Attributes.GetUnderlineStyle()); + } + + /// + /// The style is the single source of truth, so a cell underlined by any of these reports it. + /// Keeping a separate flag beside the style is how a cell ends up underlined by one and not + /// the other. + /// + [Fact] + public void IsUnderline_follows_the_style() + { + var terminal = Fresh(); + + terminal.Write($"{Esc}[4:3mx"); + Assert.True(FirstCell(terminal).Attributes.IsUnderline()); + + terminal.Write($"{Esc}[24m{Esc}[1;1Hy"); + Assert.False(FirstCell(terminal).Attributes.IsUnderline()); + Assert.Equal(UnderlineStyle.None, FirstCell(terminal).Attributes.GetUnderlineStyle()); + } + + // ---- colour --------------------------------------------------------------------------------- + + [Fact] + public void Sgr58_sets_a_truecolor_underline_as_subparameters() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[4:3;58:2::255:0:0mx"); + + var attr = FirstCell(terminal).Attributes; + Assert.True(attr.TryGetUnderlineColor(out var color, out var mode)); + Assert.Equal((255 << 16) | (0 << 8) | 0, color); + Assert.Equal(1, mode); + Assert.Equal(UnderlineStyle.Curly, attr.GetUnderlineStyle()); + } + + /// + /// Both spellings are in use, and a terminal that takes only one looks broken to half its + /// callers. + /// + [Fact] + public void Sgr58_also_accepts_separate_parameters() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[58;2;0;128;255mx"); + + Assert.True(FirstCell(terminal).Attributes.TryGetUnderlineColor(out var color, out var mode)); + Assert.Equal((0 << 16) | (128 << 8) | 255, color); + Assert.Equal(1, mode); + } + + [Fact] + public void Sgr58_accepts_an_indexed_colour() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[58:5:196mx"); + + Assert.True(FirstCell(terminal).Attributes.TryGetUnderlineColor(out var color, out var mode)); + Assert.Equal(196, color); + Assert.Equal(0, mode); + } + + [Fact] + public void Sgr59_puts_the_underline_back_to_the_foreground() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[58:2::255:0:0m{Esc}[59mx"); + + Assert.False(FirstCell(terminal).Attributes.TryGetUnderlineColor(out _, out _)); + } + + [Fact] + public void A_reset_clears_the_style_and_the_colour() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[4:3;58:2::255:0:0m{Esc}[0mx"); + + var attr = FirstCell(terminal).Attributes; + Assert.Equal(UnderlineStyle.None, attr.GetUnderlineStyle()); + Assert.False(attr.TryGetUnderlineColor(out _, out _)); + } + + /// + /// The same colour used twice is one entry, which is what keeps twenty bits of id enough. + /// + [Fact] + public void The_same_colour_interns_once() + { + var terminal = Fresh(); + + terminal.Write($"{Esc}[58:2::10:20:30mx"); + var first = FirstCell(terminal).Attributes.GetUnderlineColorId(); + + terminal.Write($"{Esc}[0m{Esc}[58:2::10:20:30m{Esc}[1;1Hy"); + var second = FirstCell(terminal).Attributes.GetUnderlineColorId(); + + Assert.Equal(first, second); + Assert.NotEqual(0, first); + } + + // ---- an abandoned sequence must not poison the next one -------------------------------------- + // + // Raised in review. The sub-parameter accumulator is parser-lifetime state, and nothing cleared + // it when a sequence was abandoned rather than dispatched -- so every digit of the NEXT sequence + // up to its first separator was swallowed into the stale sub-parameter and its first parameter + // read as 0. Worse than a dropped sequence, because 0 means something for most of them. + + private static AttributeData AttrAt(Terminal terminal) + => terminal.Buffer.Lines[terminal.Buffer.YBase]![0].Attributes; + + /// What a clean SGR 31 gives, to compare a poisoned one against. + private static int RedForeground() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[31mx"); + return AttrAt(terminal).GetFgColor(); + } + + [Theory] + [InlineData("\u001b[4:3")] // ESC begins the next sequence and abandons this one + [InlineData("\u001b[4:3\u0018")] // CAN + [InlineData("\u001b[4:3\u001a")] // SUB + [InlineData("\u001b[4:3\u001bc")] // RIS + public void An_abandoned_sequence_does_not_swallow_the_next_one(string abandoned) + { + var terminal = Fresh(); + terminal.Write(abandoned); + terminal.Write($"{Esc}[31mx"); + + Assert.Equal(RedForeground(), AttrAt(terminal).GetFgColor()); + } + + /// + /// Not only SGR: a lost first parameter homes the cursor instead of moving it. + /// + [Fact] + public void An_abandoned_sequence_does_not_swallow_a_cursor_move() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[4:3"); + terminal.Write($"{Esc}[2;5H"); + terminal.Write("z"); + + Assert.Equal("z", terminal.Buffer.Lines[terminal.Buffer.YBase + 1]![4].Content); + } + + // ---- the reason this is stored as an id ------------------------------------------------------ + + /// + /// The whole feature had to fit in bits the cell already owned. + /// + /// + /// A full RGB underline colour plus its mode is more bits than were left, and growing + /// AttributeData grows every cell in the buffer — the thing measured as costing most on fills. + /// So the cell carries an interned id, and this asserts the cost of the feature is zero. + /// + [Fact] + public void The_cell_did_not_grow() + { + Assert.Equal(12, Unsafe.SizeOf()); + Assert.False(RuntimeHelpers.IsReferenceOrContainsReferences()); + } + + /// + /// Style and colour live in the same int as the boolean attributes and must not disturb them. + /// + [Fact] + public void The_style_and_colour_do_not_disturb_the_other_attributes() + { + var terminal = Fresh(); + terminal.Write($"{Esc}[1;3;4:3;58:2::255:0:0;9mx"); + + var attr = FirstCell(terminal).Attributes; + Assert.True(attr.IsBold()); + Assert.True(attr.IsItalic()); + Assert.True(attr.IsStrikethrough()); + Assert.Equal(UnderlineStyle.Curly, attr.GetUnderlineStyle()); + Assert.True(attr.TryGetUnderlineColor(out _, out _)); + } +} diff --git a/src/XTerm.NET/Buffer/AttributeData.cs b/src/XTerm.NET/Buffer/AttributeData.cs index aaba78e..8a71c8e 100644 --- a/src/XTerm.NET/Buffer/AttributeData.cs +++ b/src/XTerm.NET/Buffer/AttributeData.cs @@ -10,20 +10,21 @@ namespace XTerm.Buffer; public struct AttributeData : IEquatable { /// - /// Bit layout: - /// 0-8: Foreground color (9 bits) - /// 9-17: Background color (9 bits) - /// 18-26: Extended attributes (9 bits) - /// 27-31: Flags (5 bits) + /// Three ints, twelve bytes, in every cell of the buffer. /// + /// + /// Fg and Bg each hold a colour in bits 0-24 and its mode above that. + /// Extended holds the boolean attributes in bits 0-8, the underline style in 9-11, and an + /// underline colour id in 12-31. + /// Underline colour is an id rather than a colour because a full RGB value plus its mode + /// does not fit in the bits left, and growing this struct grows every cell — the thing measured + /// as costing most on fills. See for why interning it + /// adds nothing to the write path, unlike interning a whole style. + /// public int Fg; public int Bg; public int Extended; - private const int FG_MASK = 0x1FF; - private const int BG_MASK = 0x1FF << 9; - private const int EXT_MASK = 0x1FF << 18; - // Attribute flags stored in upper bits of fg/bg private const int BOLD = 1 << 0; private const int DIM = 1 << 1; @@ -35,6 +36,13 @@ public struct AttributeData : IEquatable private const int STRIKETHROUGH = 1 << 7; private const int OVERLINE = 1 << 8; + // Underline style in bits 9-11, underline colour id in 12-31. Both were free: Extended is a + // 32-bit field with nine flags in it. + private const int UNDERLINE_STYLE_SHIFT = 9; + private const int UNDERLINE_STYLE_MASK = 0x7 << UNDERLINE_STYLE_SHIFT; + private const int UNDERLINE_COLOR_SHIFT = 12; + private const uint UNDERLINE_COLOR_MASK = 0xFFFFFu << UNDERLINE_COLOR_SHIFT; + public static AttributeData Default => new AttributeData { Fg = 256, // Default foreground @@ -69,7 +77,7 @@ public AttributeData(AttributeData other) public bool IsBold() => (Extended & BOLD) != 0; public bool IsDim() => (Extended & DIM) != 0; public bool IsItalic() => (Extended & ITALIC) != 0; - public bool IsUnderline() => (Extended & UNDERLINE) != 0; + public bool IsUnderline() => GetUnderlineStyle() != Common.UnderlineStyle.None; public bool IsBlink() => (Extended & BLINK) != 0; public bool IsInverse() => (Extended & INVERSE) != 0; public bool IsInvisible() => (Extended & INVISIBLE) != 0; @@ -79,7 +87,16 @@ public AttributeData(AttributeData other) public void SetBold(bool value) => SetFlag(BOLD, value); public void SetDim(bool value) => SetFlag(DIM, value); public void SetItalic(bool value) => SetFlag(ITALIC, value); - public void SetUnderline(bool value) => SetFlag(UNDERLINE, value); + /// + /// Plain underline on or off, which is what SGR 4 and 24 mean. + /// + /// + /// Turning it on sets the SINGLE style rather than a flag, so the style is the one source of + /// truth. Two places to say the same thing is how a cell ends up underlined by one and not the + /// other. + /// + public void SetUnderline(bool value) + => SetUnderlineStyle(value ? Common.UnderlineStyle.Single : Common.UnderlineStyle.None); public void SetBlink(bool value) => SetFlag(BLINK, value); public void SetInverse(bool value) => SetFlag(INVERSE, value); public void SetInvisible(bool value) => SetFlag(INVISIBLE, value); @@ -110,6 +127,43 @@ public void SetBgColor(int color, int mode = 0) Bg = (mode << 25) | (color & 0x1FFFFFF); } + /// How this cell's underline is drawn. + public Common.UnderlineStyle GetUnderlineStyle() + => (Common.UnderlineStyle)((Extended & UNDERLINE_STYLE_MASK) >> UNDERLINE_STYLE_SHIFT); + + public void SetUnderlineStyle(Common.UnderlineStyle style) + => Extended = (Extended & ~UNDERLINE_STYLE_MASK) + | ((((int)style) << UNDERLINE_STYLE_SHIFT) & UNDERLINE_STYLE_MASK); + + /// + /// The interned id of this cell's underline colour, or zero for "same as the foreground". + /// + public int GetUnderlineColorId() + => (int)(((uint)Extended & UNDERLINE_COLOR_MASK) >> UNDERLINE_COLOR_SHIFT); + + /// + /// Sets the underline colour, interning it so the cell carries only an id. + /// + /// + /// Full RGB is representable; it is the count of DISTINCT colours that is bounded, at about a + /// million. See . + /// + public void SetUnderlineColor(int color, int mode) + => SetUnderlineColorId(Common.UnderlineColorTable.Intern(color, mode)); + + /// Clears the underline colour, so the underline follows the foreground again. + public void ResetUnderlineColor() => SetUnderlineColorId(Common.UnderlineColorTable.None); + + private void SetUnderlineColorId(int id) + => Extended = (int)(((uint)Extended & ~UNDERLINE_COLOR_MASK) + | (((uint)id << UNDERLINE_COLOR_SHIFT) & UNDERLINE_COLOR_MASK)); + + /// + /// The underline's colour, or false when it follows the foreground. + /// + public bool TryGetUnderlineColor(out int color, out int mode) + => Common.UnderlineColorTable.TryGet(GetUnderlineColorId(), out color, out mode); + public bool Equals(AttributeData other) { return Fg == other.Fg && Bg == other.Bg && Extended == other.Extended; diff --git a/src/XTerm.NET/Common/UnderlineColorTable.cs b/src/XTerm.NET/Common/UnderlineColorTable.cs new file mode 100644 index 0000000..ea764fe --- /dev/null +++ b/src/XTerm.NET/Common/UnderlineColorTable.cs @@ -0,0 +1,89 @@ +using System.Collections.Concurrent; + +namespace XTerm.Common; + +/// +/// Interns underline colours, so a cell can refer to one by id. +/// +/// +/// An underline colour is a full RGB value plus the mode that says how to read it, which is +/// more bits than a cell has left. Growing to carry it would grow +/// every cell in the buffer, and cell size is the thing that costs most on fills — measured, going +/// from 24 bytes to 32 cost scroll-heavy output 22%. +/// +/// So the cell carries an id and this holds the colour. Any RGB value an application sets is +/// representable; what is bounded is how many DISTINCT colours can coexist, and twenty bits of id +/// allows about a million against the handful a real session uses. An LSP marking errors, warnings, +/// hints and information uses four. +/// +/// Nothing is ever released, and that is the point. Interning a whole style — as Ghostty +/// does — needs reference counting, which forces every cell write to read the old id so it can be +/// released. That read does not exist in this fork's run writer, and adding it measured at 240 ns +/// per line against 165. An underline colour needs no such bookkeeping: writing a cell stores an +/// int, and nothing is added to the write path at all. The lookup happens once per run per frame, +/// on the render side. +/// +/// The cost of never releasing is a table that only grows. A program cycling underline colours +/// per cell would grow it without bound, which is the same exposure +/// already carries — and once the id space is exhausted, new colours resolve to the nearest already +/// interned rather than failing. +/// +internal static class UnderlineColorTable +{ + /// Id 0 means "no underline colour" — the underline takes the foreground colour. + public const int None = 0; + + /// + /// The largest id the cell can hold: twenty bits, since three are spent on the underline style. + /// + public const int MaxId = (1 << 20) - 1; + + private static readonly ConcurrentDictionary ById = new(); + private static readonly ConcurrentDictionary Ids = new(); + + private static int _next = None; + + /// A colour and the mode that says how to read it. + internal readonly record struct UnderlineColor(int Color, int Mode); + + /// + /// Id for a colour, allocating one the first time it is seen. + /// + public static int Intern(int color, int mode) + { + var key = new UnderlineColor(color, mode); + + if (Ids.TryGetValue(key, out var existing)) + return existing; + + var id = Interlocked.Increment(ref _next); + + if (id > MaxId) + { + // Out of ids. Reuse whatever is already interned rather than fail: an underline drawn in + // a near-enough colour is a far better outcome than a terminal that stops working + // because a program insisted on a million distinct ones. + Interlocked.Exchange(ref _next, MaxId); + return MaxId; + } + + ById[id] = key; + Ids[key] = id; + return id; + } + + /// The colour for an id, or false when the id is or unknown. + public static bool TryGet(int id, out int color, out int mode) + { + if (id != None && ById.TryGetValue(id, out var entry)) + { + color = entry.Color; + mode = entry.Mode; + return true; + } + + color = 0; + mode = 0; + return false; + } +} diff --git a/src/XTerm.NET/InputHandler.cs b/src/XTerm.NET/InputHandler.cs index fb9f76a..9bc10ca 100644 --- a/src/XTerm.NET/InputHandler.cs +++ b/src/XTerm.NET/InputHandler.cs @@ -2778,8 +2778,11 @@ private void CharAttributes(Params parameters) case 3: // Italic _curAttr.SetItalic(true); break; - case 4: // Underline - _curAttr.SetUnderline(true); + case 21: // Double underline + _curAttr.SetUnderlineStyle(UnderlineStyle.Double); + break; + case 4: // Underline, with an optional style as a sub-parameter + _curAttr.SetUnderlineStyle(ReadUnderlineStyle(parameters, i)); break; case 5: // Blink _curAttr.SetBlink(true); @@ -2818,6 +2821,12 @@ private void CharAttributes(Params parameters) case >= 30 and <= 37: // Foreground color _curAttr.SetFgColor(param - 30); break; + case 58: // Underline colour + i = HandleUnderlineColor(parameters, i); + break; + case 59: // Underline colour back to the foreground + _curAttr.ResetUnderlineColor(); + break; case 38: // Extended foreground color i = HandleExtendedColor(parameters, i, true); break; @@ -2843,6 +2852,92 @@ private void CharAttributes(Params parameters) } } + /// + /// The underline style from SGR 4, which may carry it as a sub-parameter: 4:3 is curly. + /// + /// + /// Plain SGR 4 with no sub-parameter is a single underline, which is what it has always + /// meant. The sub-parameters were already being parsed and then discarded, so a program asking + /// for a curly underline — which is how an LSP marks an error — got a straight one. + /// + private static UnderlineStyle ReadUnderlineStyle(Params parameters, int index) + { + var sub = parameters.GetSubParams(index); + if (sub is null || sub.Count == 0) + return UnderlineStyle.Single; + + return sub[0] switch + { + 0 => UnderlineStyle.None, + 1 => UnderlineStyle.Single, + 2 => UnderlineStyle.Double, + 3 => UnderlineStyle.Curly, + 4 => UnderlineStyle.Dotted, + 5 => UnderlineStyle.Dashed, + + // An unknown style is still an underline. Drawing a plain one is closer to what the + // program asked for than drawing nothing. + _ => UnderlineStyle.Single, + }; + } + + /// + /// SGR 58 — the underline's own colour, in the same forms as 38 and 48. + /// + /// + /// Accepts the colour as sub-parameters (58:2::r:g:b) as well as separate parameters + /// (58;2;r;g;b), because both are in use and a terminal that takes only one of them looks + /// broken to half its callers. + /// + private int HandleUnderlineColor(Params parameters, int index) + { + var sub = parameters.GetSubParams(index); + + if (sub is { Count: > 0 }) + { + // 58:2::r:g:b — the empty slot is a colour space id nobody uses. + if (sub[0] == 2 && sub.Count >= 4) + { + var offset = sub.Count >= 5 ? 2 : 1; + var rgb = (sub[offset] << 16) | (sub[offset + 1] << 8) | sub[offset + 2]; + _curAttr.SetUnderlineColor(rgb, 1); + return index; + } + + // 58:5:n + if (sub[0] == 5 && sub.Count >= 2) + { + _curAttr.SetUnderlineColor(sub[1], 0); + return index; + } + + return index; + } + + if (index + 1 >= parameters.Length) + return index; + + var kind = parameters.GetParam(index + 1, 0); + + if (kind == 2 && index + 4 < parameters.Length) + { + var rgb = (parameters.GetParam(index + 2, 0) << 16) + | (parameters.GetParam(index + 3, 0) << 8) + | parameters.GetParam(index + 4, 0); + + _curAttr.SetUnderlineColor(rgb, 1); + return index + 4; + } + + if (kind == 5 && index + 2 < parameters.Length) + { + _curAttr.SetUnderlineColor(parameters.GetParam(index + 2, 0), 0); + return index + 2; + } + + return index; + } + private int HandleExtendedColor(Params parameters, int index, bool isForeground) { if (index + 1 >= parameters.Length) diff --git a/src/XTerm.NET/Parser/EscapeSequenceParser.cs b/src/XTerm.NET/Parser/EscapeSequenceParser.cs index 9c1adb9..44a97ab 100644 --- a/src/XTerm.NET/Parser/EscapeSequenceParser.cs +++ b/src/XTerm.NET/Parser/EscapeSequenceParser.cs @@ -303,11 +303,6 @@ private void ParseChar(int code) Collect(code); Transition(ParserState.CsiIntermediate); } - else if (code == 0x3A) // : - { - // Sub-parameter separator - Transition(ParserState.CsiIgnore); - } break; case ParserState.CsiIntermediate: @@ -514,6 +509,14 @@ private void Transition(ParserState newState) case ParserState.DcsEntry: _params.Reset(); _collect.Clear(); + // The sub-parameter accumulator is transient state like the rest, and nothing else + // clears it when a sequence is ABANDONED rather than dispatched -- FlushSubParam + // runs on a separator or at dispatch, none of which happen then. Left set, the digit + // branch swallows every digit of the NEXT sequence up to its first separator, so its + // first parameter reads as 0: ESC[31m becomes SGR 0 and resets every attribute + // instead of setting red. + _inSubParam = false; + _subParamValue = 0; _params.AddParam(0); break; @@ -544,16 +547,63 @@ private void Collect(int code) _collect.Append((char)code); } + /// + /// True between a colon and the next separator, while digits belong to a sub-parameter rather + /// than to the parameter itself. + /// + private bool _inSubParam; + + private int _subParamValue; + + /// + /// Ends the current parameter or sub-parameter and starts a sub-parameter. + /// + /// + /// An empty slot is a real value, not an omission — 58:2::255:0:0 carries a colour space + /// id nobody uses, and dropping it would shift the three components by one and turn red into + /// black. + /// + private void BeginSubParam() + { + FlushSubParam(); + _inSubParam = true; + _subParamValue = 0; + } + + private void FlushSubParam() + { + if (!_inSubParam) + return; + + _params.AddSubParam(_subParamValue); + _inSubParam = false; + _subParamValue = 0; + } + private void Param(int code) { - if (code == 0x3B) // ; + if (code == 0x3A) // : + { + // Handled HERE and not in the state machine, because 0x3A sits inside the 0x30..0x3F + // parameter-byte range the digit branch already claims -- a colon case beside that + // branch can never be reached, which is how this went unnoticed. + BeginSubParam(); + } + else if (code == 0x3B) // ; { + FlushSubParam(); _params.AddParam(0); } else if (code >= 0x30 && code <= 0x39) // 0-9 { var digit = code - 0x30; - + + if (_inSubParam) + { + _subParamValue = _subParamValue * 10 + digit; + return; + } + // Get current value of last parameter and update it var currentValue = _params.GetParam(_params.Length - 1, 0); var newValue = currentValue * 10 + digit; @@ -563,6 +613,8 @@ private void Param(int code) private void DispatchCsi(int code) { + FlushSubParam(); + var finalChar = ((char)code).ToString(); // Clone params so handlers get their own copy var paramsClone = _params.Clone(); @@ -816,6 +868,11 @@ public void Reset() _state = ParserState.Ground; _params.Reset(); _collect.Clear(); + + // Cleared here too, so an application can recover in-band: a partial write followed by RIS + // would otherwise leave the terminal misreading the first sequence after the reset. + _inSubParam = false; + _subParamValue = 0; _osc.Clear(); _dcs.Clear(); _dcsChunkLength = 0; diff --git a/src/XTerm.NET/Parser/Params.cs b/src/XTerm.NET/Parser/Params.cs index 39e10bd..2255995 100644 --- a/src/XTerm.NET/Parser/Params.cs +++ b/src/XTerm.NET/Parser/Params.cs @@ -6,8 +6,23 @@ namespace XTerm.Parser; public class Params : ICloneable { private readonly List _params; + + /// + /// Every sub-parameter in the sequence, flat. says where each + /// parameter's run begins. + /// + /// + /// Flat rather than a list per parameter because almost no sequence has sub-parameters at all, + /// and the ones that do have a handful — a list of lists would allocate on every CSI to describe + /// nothing. + /// private readonly List _subParams; - private int _subParamsStart; + + /// + /// Index into where each parameter's sub-parameters begin, and -1 for + /// a parameter that has none, which is nearly all of them. + /// + private readonly List _subParamStart; public int Length => _params.Count; @@ -17,8 +32,8 @@ public class Params : ICloneable public Params() { _params = new List(32); - _subParams = new List(32); - _subParamsStart = 0; + _subParams = new List(8); + _subParamStart = new List(32); } /// @@ -28,7 +43,7 @@ public Params(Params other) { _params = new List(other._params); _subParams = new List(other._subParams); - _subParamsStart = other._subParamsStart; + _subParamStart = new List(other._subParamStart); } /// @@ -50,6 +65,7 @@ public int GetParam(int index, int defaultValue = 0) public void AddParam(int value) { _params.Add(value); + _subParamStart.Add(-1); } /// @@ -63,31 +79,68 @@ public void UpdateLastParam(int value) } else { - _params.Add(value); + // Through AddParam, which extends BOTH lists. Adding to _params alone leaves + // _subParamStart a element short, and every sub-parameter lookup indexes it by the + // parameter's own index -- so the next AddSubParam or GetSubParams would throw. The + // parser cannot reach this today because entering CsiEntry always seeds a parameter, + // but this type is public and the invariant is new. + AddParam(value); } } /// /// Adds a sub-parameter. /// + /// + /// Adds a sub-parameter to the parameter most recently added. + /// public void AddSubParam(int value) { + if (_params.Count == 0) + { + // A sequence beginning with a colon has nothing to attach to. Give it a parameter to + // belong to rather than dropping it, so CSI :1 m is read as parameter 0 with a + // sub-parameter rather than vanishing. + AddParam(0); + } + + var last = _params.Count - 1; + if (_subParamStart[last] < 0) + _subParamStart[last] = _subParams.Count; + _subParams.Add(value); } /// - /// Gets sub-parameters for a specific parameter index. + /// The sub-parameters of one parameter, or an empty list when it has none. /// - public List GetSubParams(int index) + /// + /// These carry the colon forms: 4:3 for a curly underline, 58:2::r:g:b for an + /// underline colour, 38:2::r:g:b for a foreground. Before this returned anything the + /// parser discarded such sequences outright, so a program using the colon form of truecolor got + /// no colour at all. + /// + public IReadOnlyList GetSubParams(int index) { - var result = new List(); - if (index >= 0 && index < _params.Count) + if (index < 0 || index >= _params.Count) + return Array.Empty(); + + var start = _subParamStart[index]; + if (start < 0) + return Array.Empty(); + + // Runs are contiguous and in order, so this one ends where the next begins. + var end = _subParams.Count; + for (var i = index + 1; i < _subParamStart.Count; i++) { - // Sub-parameters are stored contiguously - // This is a simplified version - return result; + if (_subParamStart[i] >= 0) + { + end = _subParamStart[i]; + break; + } } - return result; + + return _subParams.GetRange(start, end - start); } /// @@ -97,7 +150,7 @@ public void Reset() { _params.Clear(); _subParams.Clear(); - _subParamsStart = 0; + _subParamStart.Clear(); } ///