Repository navigation
fix(cli): replace cli-table3 with hand-rolled formatter (#2752) #2813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,3 @@ | ||||||||||||||||||||||||||
| import Table from "cli-table3"; | ||||||||||||||||||||||||||
| import { stringify as csvStringify } from "csv-stringify/sync"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const MASK_RE = /sk-[A-Za-z0-9]{4,}/g; | ||||||||||||||||||||||||||
|
|
@@ -41,25 +40,61 @@ function formatCell(v, col) { | |||||||||||||||||||||||||
| return String(v); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const CYAN = "\x1b[36m"; | ||||||||||||||||||||||||||
| const RESET = "\x1b[0m"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** Truncate a string to `max` chars, appending "…" if trimmed. */ | ||||||||||||||||||||||||||
| function truncateCell(str, max) { | ||||||||||||||||||||||||||
| if (str.length <= max) return str; | ||||||||||||||||||||||||||
| return str.slice(0, max - 1) + "…"; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+51
to
+79
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If We should coerce
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** Pad a string to exactly `width` chars (left-aligned). */ | ||||||||||||||||||||||||||
| function padCell(str, width) { | ||||||||||||||||||||||||||
| return str + " ".repeat(Math.max(0, width - str.length)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function renderTable(rows, schema, opts = {}) { | ||||||||||||||||||||||||||
| if (rows.length === 0) { | ||||||||||||||||||||||||||
| process.stdout.write("(empty)\n"); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| const cols = schema || inferSchema(rows[0]); | ||||||||||||||||||||||||||
| const quiet = opts.quiet === true; | ||||||||||||||||||||||||||
|
Comment on lines
88
to
94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since Please note that this approach has two limitations:
If the CLI is expected to output colored cell values or wide characters in tables, we should consider stripping ANSI codes for length calculations using a simple regex, or document this limitation. |
||||||||||||||||||||||||||
| const widths = cols.map((c) => c.width || null); | ||||||||||||||||||||||||||
| const hasWidths = widths.some((w) => w !== null); | ||||||||||||||||||||||||||
| const tableOpts = { | ||||||||||||||||||||||||||
| head: quiet ? [] : cols.map((c) => c.header), | ||||||||||||||||||||||||||
| style: { head: quiet ? [] : ["cyan"] }, | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Compute column widths: max(header.length, max cell length), capped by explicit c.width. | ||||||||||||||||||||||||||
| const colWidths = cols.map((c) => { | ||||||||||||||||||||||||||
| const headerLen = c.header.length; | ||||||||||||||||||||||||||
| const maxData = rows.reduce((m, row) => Math.max(m, formatCell(row[c.key], c).length), 0); | ||||||||||||||||||||||||||
| const natural = Math.max(headerLen, maxData); | ||||||||||||||||||||||||||
| return c.width ? Math.max(c.width, 1) : natural; | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
Comment on lines
+97
to
+105
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a few potential issues here:
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const separator = colWidths.map((w) => "-".repeat(w + 2)).join("-+-"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const renderRow = (cells, cyan) => { | ||||||||||||||||||||||||||
| const parts = cells.map((cell, i) => { | ||||||||||||||||||||||||||
| const truncated = truncateCell(cell, colWidths[i]); | ||||||||||||||||||||||||||
| const padded = padCell(truncated, colWidths[i]); | ||||||||||||||||||||||||||
| return cyan ? ` ${CYAN}${padded}${RESET} ` : ` ${padded} `; | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| return `|${parts.join("|")}|`; | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| if (hasWidths) tableOpts.colWidths = widths; | ||||||||||||||||||||||||||
| const table = new Table(tableOpts); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const lines = []; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!quiet) { | ||||||||||||||||||||||||||
| lines.push(separator); | ||||||||||||||||||||||||||
| lines.push(renderRow(cols.map((c) => c.header), true)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| lines.push(separator); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| for (const row of rows) { | ||||||||||||||||||||||||||
| table.push(cols.map((c) => formatCell(row[c.key], c))); | ||||||||||||||||||||||||||
| lines.push(renderRow(cols.map((c) => formatCell(row[c.key], c)), false)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| process.stdout.write(table.toString() + "\n"); | ||||||||||||||||||||||||||
| lines.push(separator); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| process.stdout.write(lines.join("\n") + "\n"); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function renderCsv(rows, schema) { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /** | ||
| * Regression tests for renderTable (exposed via emit({ output: "table" })). | ||
| * Verifies behaviour is preserved after replacing cli-table3 with the hand-rolled formatter. | ||
| */ | ||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
|
|
||
| function captureStdout(fn: () => void): string { | ||
| const chunks: string[] = []; | ||
| const originalWrite = process.stdout.write.bind(process.stdout); | ||
| process.stdout.write = (chunk: string | Uint8Array) => { | ||
| chunks.push(typeof chunk === "string" ? chunk : chunk.toString()); | ||
| return true; | ||
| }; | ||
| try { | ||
| fn(); | ||
| } finally { | ||
| process.stdout.write = originalWrite; | ||
| } | ||
| return chunks.join(""); | ||
| } | ||
|
|
||
| // ─── renderTable via emit ──────────────────────────────────────────────────── | ||
|
|
||
| test("renderTable: header text appears in output", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [ | ||
| { key: "a", header: "A" }, | ||
| { key: "b", header: "B" }, | ||
| ]; | ||
| const out = captureStdout(() => emit([{ a: "foo", b: "bar" }], { output: "table" }, schema)); | ||
| assert.ok(out.includes("A"), `expected "A" in output, got: ${out}`); | ||
| assert.ok(out.includes("B"), `expected "B" in output, got: ${out}`); | ||
| }); | ||
|
|
||
| test("renderTable: row values appear in output", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [ | ||
| { key: "a", header: "A" }, | ||
| { key: "b", header: "B" }, | ||
| ]; | ||
| const out = captureStdout(() => emit([{ a: "foo", b: "bar" }], { output: "table" }, schema)); | ||
| assert.ok(out.includes("foo"), `expected "foo" in output, got: ${out}`); | ||
| assert.ok(out.includes("bar"), `expected "bar" in output, got: ${out}`); | ||
| }); | ||
|
|
||
| test("renderTable: output contains newline separation", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [{ key: "a", header: "A" }]; | ||
| const out = captureStdout(() => emit([{ a: "foo" }], { output: "table" }, schema)); | ||
| assert.ok(out.includes("\n"), "expected newline in output"); | ||
| }); | ||
|
|
||
| test("renderTable: empty rows prints (empty)", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [{ key: "a", header: "A" }]; | ||
| const out = captureStdout(() => emit([], { output: "table" }, schema)); | ||
| assert.ok(out.includes("empty"), `expected "(empty)" in output, got: ${out}`); | ||
| }); | ||
|
|
||
| test("renderTable: quiet:true suppresses headers", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [ | ||
| { key: "a", header: "Alpha" }, | ||
| { key: "b", header: "Beta" }, | ||
| ]; | ||
| const out = captureStdout(() => | ||
| emit([{ a: "val1", b: "val2" }], { output: "table", quiet: true }, schema), | ||
| ); | ||
| // headers must NOT appear in quiet mode | ||
| assert.ok(!out.includes("Alpha"), `header "Alpha" should be hidden in quiet mode, got: ${out}`); | ||
| assert.ok(!out.includes("Beta"), `header "Beta" should be hidden in quiet mode, got: ${out}`); | ||
| // data rows still present | ||
| assert.ok(out.includes("val1"), `expected "val1" in quiet output, got: ${out}`); | ||
| assert.ok(out.includes("val2"), `expected "val2" in quiet output, got: ${out}`); | ||
| }); | ||
|
|
||
| test("renderTable: multiple rows all appear", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [{ key: "name", header: "Name" }]; | ||
| const data = [{ name: "alice" }, { name: "bob" }, { name: "charlie" }]; | ||
| const out = captureStdout(() => emit(data, { output: "table" }, schema)); | ||
| assert.ok(out.includes("alice"), "expected 'alice'"); | ||
| assert.ok(out.includes("bob"), "expected 'bob'"); | ||
| assert.ok(out.includes("charlie"), "expected 'charlie'"); | ||
| }); | ||
|
|
||
| test("renderTable: infers schema from data keys when no schema given", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const data = [{ id: "x1", status: "ok" }]; | ||
| const out = captureStdout(() => emit(data, { output: "table" })); | ||
| assert.ok(out.includes("id"), "expected key 'id' as header"); | ||
| assert.ok(out.includes("status"), "expected key 'status' as header"); | ||
| assert.ok(out.includes("x1"), "expected value 'x1'"); | ||
| assert.ok(out.includes("ok"), "expected value 'ok'"); | ||
| }); | ||
|
|
||
| test("renderTable: column separator characters present", async () => { | ||
| const { emit } = await import("../../bin/cli/output.mjs"); | ||
| const schema = [ | ||
| { key: "col1", header: "Col1" }, | ||
| { key: "col2", header: "Col2" }, | ||
| ]; | ||
| const out = captureStdout(() => emit([{ col1: "a", col2: "b" }], { output: "table" }, schema)); | ||
| // Output must contain some column delimiter — either "|" (hand-rolled) or "│" (cli-table3 box-drawing) | ||
| const hasSeparator = out.includes("|") || out.includes("│"); | ||
| assert.ok(hasSeparator, `expected column separator in output, got: ${out}`); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a table column uses one of the existing colored formatters with an explicit width, this raw-string truncation counts and slices ANSI control sequences instead of visible characters. For example
providerListSchemaformatsisActiveas\x1b[32m✓\x1b[0mwithwidth: 8(bin/cli/schemas/output-schemas.mjs:14), so the visible one-character checkmark is treated as length 10 and is truncated into a partial escape/reset sequence, corrupting the table and potentially leaking color into following output.cli-table3handled this via ANSI-aware width/strip logic, so the replacement needs similar handling before truncating/padding.Useful? React with 👍 / 👎.