Public: anyone can read this; the crew can change it.
A test pins what the forge CLI prints: columns line up with wide characters, and piped output carries no colour #7254
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6875-a-test-pins-what-the-forge-cli-prints-co → main 1 commit fork 2 threads · 2 unresolved
2 files
changed +145
−4
src/cli/out.ts +12 −4
| @@ -45,6 +45,11 @@ export function cell(text: string, fg?: Color, attr?: number): Cell { | ||
| 45 | 45 | }; |
| 46 | 46 | } |
| 47 | 47 | |
| 48 | /** A cell is data, not instructions: control characters become U+FFFD. */ | |
| 49 | function inert(text: string): string { | |
| 50 | return text.replace(/[\x00-\x1f\x7f-\x9f]/g, "\ufffd"); | |
| 51 | } | |
| 52 | ||
| 48 | 53 | function chosenTheme(): ThemeName { |
| 49 | 54 | const name = process.env["FORGE_THEME"]; |
| 50 | 55 | return name !== undefined && (THEME_NAMES as readonly string[]).includes(name) |
| @@ -77,12 +82,15 @@ export function createOut( | ||
| 77 | 82 | paint, |
| 78 | 83 | line: (text = "") => write(`${text}\n`), |
| 79 | 84 | err: (text) => writeErr(`${text}\n`), |
| 80 | table(columns, rows) { | |
| 81 | if (rows.length === 0) return; | |
| 85 | table(columns, rows_) { | |
| 86 | if (rows_.length === 0) return; | |
| 87 | const rows = rows_.map((row) => | |
| 88 | row.map((x) => ({ ...x, text: inert(x.text) })), | |
| 89 | ); | |
| 82 | 90 | // Size from the content, then cap: a table that reflows on every run |
| 83 | 91 | // because one branch name grew is a table you cannot scan. |
| 84 | 92 | const widths = columns.map((col, i) => { |
| 85 | let w = displayWidth(col.header); | |
| 93 | let w = displayWidth(inert(col.header)); | |
| 86 | 94 | for (const row of rows) |
| 87 | 95 | w = Math.max(w, displayWidth(row[i]?.text ?? "")); |
| 88 | 96 | return Math.min(w, col.max ?? w); |
| @@ -98,7 +106,7 @@ export function createOut( | ||
| 98 | 106 | }; |
| 99 | 107 | write( |
| 100 | 108 | `${render( |
| 101 | columns.map((col) => cell(col.header)), | |
| 109 | columns.map((col) => cell(inert(col.header))), | |
| 102 | 110 | true, |
| 103 | 111 | )}\n`, |
| 104 | 112 | ); |
test/cli-out.test.ts added +133
| @@ -0,0 +1,133 @@ | ||
| 1 | import { afterEach, describe, expect, test } from "bun:test"; | |
| 2 | import { cell, createOut } from "../src/cli/out.ts"; | |
| 3 | import { displayWidth } from "../src/tui/term/width.ts"; | |
| 4 | ||
| 5 | const ESC = "\x1b"; | |
| 6 | const saved = { | |
| 7 | noColor: process.env["NO_COLOR"], | |
| 8 | forgeColor: process.env["FORGE_COLOR"], | |
| 9 | isTTY: process.stdout.isTTY, | |
| 10 | columns: process.stdout.columns, | |
| 11 | }; | |
| 12 | ||
| 13 | function terminal(tty: boolean, columns?: number): void { | |
| 14 | Object.defineProperty(process.stdout, "isTTY", { value: tty, configurable: true }); | |
| 15 | Object.defineProperty(process.stdout, "columns", { value: columns, configurable: true }); | |
| 16 | } | |
| 17 | ||
| 18 | function restore(key: string, value: string | undefined): void { | |
| 19 | if (value === undefined) delete process.env[key]; | |
| 20 | else process.env[key] = value; | |
| 21 | } | |
| 22 | ||
| 23 | afterEach(() => { | |
| 24 | restore("NO_COLOR", saved.noColor); | |
| 25 | restore("FORGE_COLOR", saved.forgeColor); | |
| 26 | terminal(saved.isTTY as boolean, saved.columns); | |
| 27 | }); | |
| 28 | ||
| 29 | function capture(): { out: ReturnType<typeof createOut>; stdout: string[]; stderr: string[] } { | |
| 30 | const stdout: string[] = []; | |
| 31 | const stderr: string[] = []; | |
| 32 | const out = createOut((t) => void stdout.push(t), (t) => void stderr.push(t)); | |
| 33 | return { out, stdout, stderr }; | |
| 34 | } | |
| 35 | ||
| 36 | describe("colour", () => { | |
| 37 | test("piped output carries no ANSI escape", () => { | |
| 38 | delete process.env["NO_COLOR"]; | |
| 39 | delete process.env["FORGE_COLOR"]; | |
| 40 | terminal(false); | |
| 41 | const { out, stdout } = capture(); | |
| 42 | out.line(out.paint("hello", 0xff0000)); | |
| 43 | out.table([{ header: "name" }], [[cell("a", 0x00ff00, 1)]]); | |
| 44 | expect(out.color).toBe(false); | |
| 45 | expect(stdout.join("")).not.toContain(ESC); | |
| 46 | }); | |
| 47 | ||
| 48 | test("NO_COLOR silences a terminal", () => { | |
| 49 | process.env["NO_COLOR"] = "1"; | |
| 50 | terminal(true, 100); | |
| 51 | const { out } = capture(); | |
| 52 | expect(out.color).toBe(false); | |
| 53 | expect(out.paint("x", 0xff0000)).toBe("x"); | |
| 54 | }); | |
| 55 | ||
| 56 | test("a terminal without NO_COLOR paints", () => { | |
| 57 | delete process.env["NO_COLOR"]; | |
| 58 | delete process.env["FORGE_COLOR"]; | |
| 59 | terminal(true, 100); | |
| 60 | const { out } = capture(); | |
| 61 | expect(out.color).toBe(true); | |
| 62 | const painted = out.paint("x", 0xff0000); | |
| 63 | expect(painted).toContain(ESC); | |
| 64 | expect(painted.endsWith(`${ESC}[0m`)).toBe(true); | |
| 65 | }); | |
| 66 | }); | |
| 67 | ||
| 68 | describe("streams and width", () => { | |
| 69 | test("width is 80 without a terminal, the terminal's otherwise", () => { | |
| 70 | terminal(false, undefined); | |
| 71 | expect(capture().out.width).toBe(80); | |
| 72 | terminal(true, 132); | |
| 73 | expect(capture().out.width).toBe(132); | |
| 74 | }); | |
| 75 | ||
| 76 | test("err goes to stderr, line to stdout", () => { | |
| 77 | terminal(false); | |
| 78 | const { out, stdout, stderr } = capture(); | |
| 79 | out.err("bad"); | |
| 80 | out.line("good"); | |
| 81 | expect(stderr).toEqual(["bad\n"]); | |
| 82 | expect(stdout).toEqual(["good\n"]); | |
| 83 | }); | |
| 84 | }); | |
| 85 | ||
| 86 | describe("table", () => { | |
| 87 | function render( | |
| 88 | cols: Parameters<ReturnType<typeof createOut>["table"]>[0], | |
| 89 | rows: Parameters<ReturnType<typeof createOut>["table"]>[1], | |
| 90 | ): string[] { | |
| 91 | terminal(false); | |
| 92 | const { out, stdout } = capture(); | |
| 93 | out.table(cols, rows); | |
| 94 | return stdout.join("").split("\n").filter((l) => l !== ""); | |
| 95 | } | |
| 96 | ||
| 97 | test("a column with CJK and emoji lines up with ASCII", () => { | |
| 98 | const lines = render( | |
| 99 | [{ header: "repo" }, { header: "n" }], | |
| 100 | [[cell("plain"), cell("1")], [cell("日本語"), cell("2")], [cell("🔥x"), cell("3")]], | |
| 101 | ); | |
| 102 | const start = lines.map((l) => displayWidth(l.slice(0, l.lastIndexOf(" ")))); | |
| 103 | expect(new Set(start).size).toBe(1); | |
| 104 | expect(start[0]).toBe(displayWidth("日本語") > 5 ? 6 : 5); | |
Nit, not blocking: the expected value depends on what |
||
| 105 | expect(lines.map((l) => l.at(-1))).toEqual(["n", "1", "2", "3"]); | |
| 106 | }); | |
| 107 | ||
| 108 | test("max cuts a cell", () => { | |
| 109 | const lines = render([{ header: "name", max: 5 }], [[cell("abcdefghij")]]); | |
| 110 | expect(lines[1]).toBe("abcd…"); | |
| 111 | expect(displayWidth(lines[1]!)).toBe(5); | |
| 112 | }); | |
| 113 | ||
| 114 | test("right-aligned numbers end at the same column", () => { | |
| 115 | const lines = render( | |
| 116 | [{ header: "n", align: "right" }, { header: "x" }], | |
| 117 | [[cell("1"), cell("a")], [cell("1234"), cell("b")], [cell("56"), cell("c")]], | |
| 118 | ); | |
| 119 | const ends = lines.map((l) => l.lastIndexOf(" ")); | |
| 120 | expect(new Set(ends).size).toBe(1); | |
| 121 | expect(lines[1]).toBe(" 1 a"); | |
| 122 | }); | |
| 123 | ||
| 124 | test("an escape sequence in a cell is printed inert", () => { | |
| 125 | const lines = render( | |
| 126 | [{ header: "repo" }], | |
| 127 | [[cell(`evil${ESC}[2J${ESC}]0;pwned\x07name`)]], | |
| 128 | ); | |
| 129 | const text = lines.join("\n"); | |
| 130 | expect(text).not.toContain(ESC); | |
| 131 | expect(text).not.toContain("\x07"); | |
| 132 | }); | |
| 133 | }); |
new above
offline
Good. Follow-up, not blocking: the same treatment is needed for interpolated server text that goes through
line. Bidi overrides (U+202A–202E, U+2066–2069) could join this class later.