fix(shared): underline styles and colours no longer scramble command output colours

packages/shared/src/terminalOutput.test.ts

@@ -87,6 +87,50 @@ describe("appendTerminalOutput", () => {
 });
 
 describe("appendTerminalOutput edge cases from review", () => {
+  const boldRed = { color: TERMINAL_PALETTES.dark[1], fontWeight: "600" } as const;
+  it.each([
+    [
+      "a curly underline",
+      "\u001b[1;31m\u001b[4:3mX",
+      { ...boldRed, textDecorationLine: "underline" },
+    ],
+    ["underline off in colon form", "\u001b[1;31m\u001b[4:0mX", boldRed],
+    ["a true-colour underline colour", "\u001b[1;31m\u001b[58;2;255;0;0mX", boldRed],
+    ["a 256-colour underline colour", "\u001b[58;5;31mX", null],
+    [
+      "a colon-form underline colour",
+      "\u001b[4m\u001b[58:2::255:0:0mX",
+      { textDecorationLine: "underline" },
+    ],
+    ["extra fields in a colon-form indexed underline colour", "\u001b[58:5:0:32mX", null],
+    [
+      "extra fields in a colon-form indexed colour",
+      "\u001b[38:5:1:4mX",
+      { color: TERMINAL_PALETTES.dark[1] },
+    ],
+    [
+      "a true colour with its fields after colons",
+      "\u001b[38;2:255:0:0;1mX",
+      { color: "rgb(255,0,0)", fontWeight: "600" },
+    ],
+    [
+      "an indexed colour with its index after a colon",
+      "\u001b[38;5:1;4mX",
+      { color: TERMINAL_PALETTES.dark[1], textDecorationLine: "underline" },
+    ],
+    [
+      "a semicolon-form colour value that looks like a colour code",
+      "\u001b[38;5;38;2:1:2:3mX",
+      { color: "rgb(0,175,215)", opacity: 0.65 },
+    ],
+  ])("reads %s without disturbing the other attributes", (_, input, style) => {
+    const text = normalizeTerminalOutput(input).text;
+    expect(terminalOutputSpans(text, TERMINAL_PALETTES.dark)).toEqual([{ text: "X", style }]);
+    for (let cut = 1; cut < input.length; cut += 1) {
+      expect(feed([input.slice(0, cut), input.slice(cut)]).text).toBe(text);
+    }
+  });
+
   it("leaves the screen and cursor alone for unsupported erase modes", () => {
     for (const sequence of ["3K", "4J", "99K", "99J"]) {
       const input = `a\nb\nc\u001b[${sequence}X`;

packages/shared/src/terminalOutput.ts

@@ -230,9 +230,10 @@ function applySgr(attributes: Attributes, params: ReadonlyArray<number>): void {
       attributes.bg = String(code);
     else if (code === 39) attributes.fg = "";
     else if (code === 49) attributes.bg = "";
-    else if (code === 38 || code === 48) {
+    else if (code === 38 || code === 48 || code === 58) {
+      // 58 (underline colour) is consumed but not kept: nothing renders it.
       const [color, next] = extendedColor(code, params, index);
-      if (color !== undefined) attributes[code === 38 ? "fg" : "bg"] = color;
+      if (color !== undefined && code !== 58) attributes[code === 38 ? "fg" : "bg"] = color;
       index = next;
       continue;
     }
@@ -277,14 +278,44 @@ const penTransitions = new Map<string, string>();
 // Colon color sub-parameters carry an optional color-space id absent from the semicolon form.
 function csiParams(params: string): Array<number> {
   const numbers: Array<number> = [];
+  // After a semicolon-form 38/48/58 the next part is its mode, then that many
+  // colour values; tracking them keeps a value from being read as a code.
+  let pending: "code" | "mode" | number = "code";
   for (const part of params.split(";")) {
-    if (!part.includes(":")) {
-      numbers.push(Number(part));
+    const sub = part.split(":").map(Number);
+    const head = sub[0]!;
+    if (typeof pending === "number") {
+      numbers.push(head);
+      pending = pending > 1 ? pending - 1 : "code";
       continue;
     }
-    const sub = part.split(":").map(Number);
-    if ((sub[0] === 38 || sub[0] === 48) && sub[1] === 2 && sub.length >= 6) sub.splice(2, 1);
-    numbers.push(...sub);
+    if (pending === "mode") {
+      pending = "code";
+      if (sub.length === 1) {
+        numbers.push(head);
+        if (head === 5 || head === 2) pending = head === 5 ? 1 : 3;
+        continue;
+      }
+      // xterm also takes the mode's fields after colons (`38;2:255:0:0`).
+      if (head === 2 && sub.length >= 5) sub.splice(1, 1);
+      numbers.push(...sub.slice(0, head === 5 ? 2 : head === 2 ? 4 : 1));
+      continue;
+    }
+    if (sub.length === 1) {
+      numbers.push(head);
+      if (head === 38 || head === 48 || head === 58) pending = "mode";
+      continue;
+    }
+    if (head !== 38 && head !== 48 && head !== 58) {
+      // Other sub-parameters pick a style variant (`4:3` curly underline); keep
+      // only the head, except that `4:0` turns underline off.
+      numbers.push(head === 4 && sub[1] === 0 ? 24 : head);
+      continue;
+    }
+    if (sub[1] === 2 && sub.length >= 6) sub.splice(2, 1);
+    // The group is one parameter, so fields past its mode's length must not
+    // leak out as SGR codes of their own. Indexed colour has no colour-space id.
+    numbers.push(...sub.slice(0, sub[1] === 5 ? 3 : sub[1] === 2 ? 5 : 2));
   }
   return numbers;
 }