From 8127a48dab43056a0e0a0758c19a97452406dbd3 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 10 Mar 2026 09:50:05 +0000 Subject: [PATCH] refactor: converge Tier 1 commands to writeOutput helper Enhance writeOutput with footer and jsonData options, migrate auth/whoami, auth/refresh, and issue/explain to use it. - footer: muted hint after human output, suppressed in JSON mode - jsonData: separate JSON data when shapes diverge from human formatter - 16 new tests in test/lib/formatters/output.test.ts --- src/commands/auth/refresh.ts | 22 ++- src/commands/auth/whoami.ts | 28 ++-- src/commands/issue/explain.ts | 19 +-- src/lib/formatters/output.ts | 53 ++++++- test/lib/formatters/output.test.ts | 240 +++++++++++++++++++++++++++++ 5 files changed, 314 insertions(+), 48 deletions(-) create mode 100644 test/lib/formatters/output.test.ts diff --git a/src/commands/auth/refresh.ts b/src/commands/auth/refresh.ts index d711062c7..9eba3f16a 100644 --- a/src/commands/auth/refresh.ts +++ b/src/commands/auth/refresh.ts @@ -15,7 +15,7 @@ import { import { AuthError } from "../../lib/errors.js"; import { success } from "../../lib/formatters/colors.js"; import { formatDuration } from "../../lib/formatters/human.js"; -import { writeJson } from "../../lib/formatters/index.js"; +import { writeOutput } from "../../lib/formatters/index.js"; type RefreshFlags = { readonly json: boolean; @@ -100,17 +100,13 @@ Examples: : undefined, }; - if (flags.json) { - writeJson(stdout, output, flags.fields); - } else if (result.refreshed) { - stdout.write( - `${success("✓")} Token refreshed successfully. Expires in ${formatDuration(result.expiresIn ?? 0)}.\n` - ); - } else { - stdout.write( - `Token still valid (expires in ${formatDuration(result.expiresIn ?? 0)}).\n` + - "Use --force to refresh anyway.\n" - ); - } + writeOutput(stdout, output, { + json: flags.json, + fields: flags.fields, + formatHuman: (data) => + data.refreshed + ? `${success("✓")} Token refreshed successfully. Expires in ${formatDuration(data.expiresIn ?? 0)}.` + : `Token still valid (expires in ${formatDuration(data.expiresIn ?? 0)}).\nUse --force to refresh anyway.`, + }); }, }); diff --git a/src/commands/auth/whoami.ts b/src/commands/auth/whoami.ts index 4c2a678cd..49521bd43 100644 --- a/src/commands/auth/whoami.ts +++ b/src/commands/auth/whoami.ts @@ -12,7 +12,7 @@ import { buildCommand } from "../../lib/command.js"; import { isAuthenticated } from "../../lib/db/auth.js"; import { setUserInfo } from "../../lib/db/user.js"; import { AuthError } from "../../lib/errors.js"; -import { formatUserIdentity, writeJson } from "../../lib/formatters/index.js"; +import { formatUserIdentity, writeOutput } from "../../lib/formatters/index.js"; import { applyFreshFlag, FRESH_ALIASES, @@ -63,20 +63,16 @@ export const whoamiCommand = buildCommand({ // Cache update failure is non-essential — user identity was already fetched. } - if (flags.json) { - writeJson( - stdout, - { - id: user.id, - name: user.name ?? null, - username: user.username ?? null, - email: user.email ?? null, - }, - flags.fields - ); - return; - } - - stdout.write(`${formatUserIdentity(user)}\n`); + writeOutput(stdout, user, { + json: flags.json, + fields: flags.fields, + jsonData: { + id: user.id, + name: user.name ?? null, + username: user.username ?? null, + email: user.email ?? null, + }, + formatHuman: formatUserIdentity, + }); }, }); diff --git a/src/commands/issue/explain.ts b/src/commands/issue/explain.ts index 6f52432f5..b9f5f6a8f 100644 --- a/src/commands/issue/explain.ts +++ b/src/commands/issue/explain.ts @@ -7,7 +7,7 @@ import type { SentryContext } from "../../context.js"; import { buildCommand } from "../../lib/command.js"; import { ApiError } from "../../lib/errors.js"; -import { writeFooter, writeJson } from "../../lib/formatters/index.js"; +import { writeOutput } from "../../lib/formatters/index.js"; import { formatRootCauseList, handleSeerApiError, @@ -111,17 +111,12 @@ export const explainCommand = buildCommand({ } // Output results - if (flags.json) { - writeJson(stdout, causes, flags.fields); - return; - } - - // Human-readable output - stdout.write(`${formatRootCauseList(causes)}\n`); - writeFooter( - stdout, - `To create a plan, run: sentry issue plan ${issueArg}` - ); + writeOutput(stdout, causes, { + json: flags.json, + fields: flags.fields, + formatHuman: formatRootCauseList, + footer: `To create a plan, run: sentry issue plan ${issueArg}`, + }); } catch (error) { // Handle API errors with friendly messages if (error instanceof ApiError) { diff --git a/src/lib/formatters/output.ts b/src/lib/formatters/output.ts index eaaf0243b..35ae7f320 100644 --- a/src/lib/formatters/output.ts +++ b/src/lib/formatters/output.ts @@ -9,6 +9,12 @@ import type { Writer } from "../../types/index.js"; import { muted } from "./colors.js"; import { writeJson } from "./json.js"; +/** + * Options for {@link writeOutput} when JSON and human data share the same type. + * + * Most commands fetch data and then either serialize it to JSON or format it + * for the terminal — use this form when the same object works for both paths. + */ type WriteOutputOptions = { /** Output JSON format instead of human-readable */ json: boolean; @@ -18,23 +24,52 @@ type WriteOutputOptions = { formatHuman: (data: T) => string; /** Optional source description if data was auto-detected */ detectedFrom?: string; + /** Footer hint shown after human output (suppressed in JSON mode) */ + footer?: string; +}; + +/** + * Options for {@link writeOutput} when JSON needs a different data shape. + * + * Some commands build a richer or narrower object for JSON than the one + * the human formatter receives. Supply `jsonData` to decouple the two. + * + * @typeParam T - Type of data used by the human formatter + * @typeParam J - Type of data serialized to JSON (defaults to T) + */ +type WriteOutputDivergentOptions = WriteOutputOptions & { + /** + * Separate data object to serialize when `json: true`. + * When provided, `data` is only used by `formatHuman` and + * `jsonData` is passed to `writeJson`. + */ + jsonData: J; }; /** * Write formatted output to stdout based on output format. - * Handles the common JSON vs human-readable pattern used across commands. * - * @param stdout - Writer to output to - * @param data - Data to output - * @param options - Output options including format and formatters + * Handles the common JSON-vs-human pattern used across commands: + * - JSON mode: serialize data (or `jsonData` if provided) with optional field filtering + * - Human mode: call `formatHuman`, then optionally print `detectedFrom` and `footer` + * + * When JSON and human paths need different data shapes, pass `jsonData`: + * ```ts + * writeOutput(stdout, fullUser, { + * json: true, + * jsonData: { id: fullUser.id, email: fullUser.email }, + * formatHuman: formatUserIdentity, + * }); + * ``` */ -export function writeOutput( +export function writeOutput( stdout: Writer, data: T, - options: WriteOutputOptions + options: WriteOutputOptions | WriteOutputDivergentOptions ): void { if (options.json) { - writeJson(stdout, data, options.fields); + const jsonPayload = "jsonData" in options ? options.jsonData : data; + writeJson(stdout, jsonPayload, options.fields); return; } @@ -44,6 +79,10 @@ export function writeOutput( if (options.detectedFrom) { stdout.write(`\nDetected from ${options.detectedFrom}\n`); } + + if (options.footer) { + writeFooter(stdout, options.footer); + } } /** diff --git a/test/lib/formatters/output.test.ts b/test/lib/formatters/output.test.ts new file mode 100644 index 000000000..02262865b --- /dev/null +++ b/test/lib/formatters/output.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, test } from "bun:test"; +import { + writeFooter, + writeOutput, +} from "../../../src/lib/formatters/output.js"; + +/** Collect all writes to a string array for assertions. */ +function createTestWriter() { + const chunks: string[] = []; + return { + write(data: string) { + chunks.push(data); + return true; + }, + chunks, + /** Full concatenated output */ + get output() { + return chunks.join(""); + }, + }; +} + +describe("writeOutput", () => { + describe("json mode", () => { + test("writes JSON with fields filtering", () => { + const w = createTestWriter(); + writeOutput( + w, + { id: 1, name: "Alice", secret: "x" }, + { + json: true, + fields: ["id", "name"], + formatHuman: () => "should not be called", + } + ); + const parsed = JSON.parse(w.output); + expect(parsed).toEqual({ id: 1, name: "Alice" }); + }); + + test("writes full JSON when no fields specified", () => { + const w = createTestWriter(); + writeOutput( + w, + { a: 1, b: 2 }, + { + json: true, + formatHuman: () => "unused", + } + ); + expect(JSON.parse(w.output)).toEqual({ a: 1, b: 2 }); + }); + + test("does not call formatHuman in json mode", () => { + const w = createTestWriter(); + let called = false; + writeOutput( + w, + { x: 1 }, + { + json: true, + formatHuman: () => { + called = true; + return "nope"; + }, + } + ); + expect(called).toBe(false); + }); + + test("does not write footer in json mode", () => { + const w = createTestWriter(); + writeOutput( + w, + { x: 1 }, + { + json: true, + formatHuman: () => "unused", + footer: "Should not appear", + } + ); + expect(w.output).not.toContain("Should not appear"); + }); + + test("does not write detectedFrom in json mode", () => { + const w = createTestWriter(); + writeOutput( + w, + { x: 1 }, + { + json: true, + formatHuman: () => "unused", + detectedFrom: ".env", + } + ); + expect(w.output).not.toContain(".env"); + }); + }); + + describe("human mode", () => { + test("calls formatHuman and writes with trailing newline", () => { + const w = createTestWriter(); + writeOutput( + w, + { name: "Alice" }, + { + json: false, + formatHuman: (data) => `Hello ${data.name}`, + } + ); + expect(w.output).toBe("Hello Alice\n"); + }); + + test("appends detectedFrom when provided", () => { + const w = createTestWriter(); + writeOutput(w, "data", { + json: false, + formatHuman: () => "Result", + detectedFrom: ".env.local", + }); + expect(w.output).toContain("Result\n"); + expect(w.output).toContain("Detected from .env.local"); + }); + + test("appends footer when provided", () => { + const w = createTestWriter(); + writeOutput(w, "data", { + json: false, + formatHuman: () => "Main output", + footer: "Tip: try something", + }); + expect(w.output).toContain("Main output\n"); + expect(w.output).toContain("Tip: try something"); + }); + + test("writes detectedFrom before footer", () => { + const w = createTestWriter(); + writeOutput(w, "data", { + json: false, + formatHuman: () => "Body", + detectedFrom: "DSN", + footer: "Hint", + }); + const detectedIdx = w.output.indexOf("Detected from DSN"); + const footerIdx = w.output.indexOf("Hint"); + expect(detectedIdx).toBeGreaterThan(-1); + expect(footerIdx).toBeGreaterThan(detectedIdx); + }); + + test("omits detectedFrom when not provided", () => { + const w = createTestWriter(); + writeOutput(w, 42, { + json: false, + formatHuman: (n) => `Number: ${n}`, + }); + expect(w.output).toBe("Number: 42\n"); + expect(w.output).not.toContain("Detected from"); + }); + + test("omits footer when not provided", () => { + const w = createTestWriter(); + writeOutput(w, 42, { + json: false, + formatHuman: (n) => `Number: ${n}`, + }); + // Only the main output + newline + expect(w.chunks).toHaveLength(1); + }); + }); + + describe("jsonData (divergent data)", () => { + test("uses jsonData for JSON output instead of data", () => { + const w = createTestWriter(); + const fullUser = { id: 1, name: "Alice", internalSecret: "s3cret" }; + writeOutput(w, fullUser, { + json: true, + jsonData: { id: fullUser.id, name: fullUser.name }, + formatHuman: () => "unused", + }); + const parsed = JSON.parse(w.output); + expect(parsed).toEqual({ id: 1, name: "Alice" }); + expect(w.output).not.toContain("s3cret"); + }); + + test("uses data for formatHuman even when jsonData is provided", () => { + const w = createTestWriter(); + const fullUser = { id: 1, name: "Alice", role: "admin" }; + writeOutput(w, fullUser, { + json: false, + jsonData: { id: fullUser.id }, + formatHuman: (user) => `${user.name} (${user.role})`, + }); + expect(w.output).toBe("Alice (admin)\n"); + }); + + test("applies fields filtering to jsonData", () => { + const w = createTestWriter(); + writeOutput( + w, + { full: true }, + { + json: true, + fields: ["id"], + jsonData: { id: 1, name: "Alice", extra: "x" }, + formatHuman: () => "unused", + } + ); + expect(JSON.parse(w.output)).toEqual({ id: 1 }); + }); + + test("works with footer and detectedFrom in human mode", () => { + const w = createTestWriter(); + writeOutput( + w, + { name: "Alice" }, + { + json: false, + jsonData: { id: 1 }, + formatHuman: (data) => `User: ${data.name}`, + footer: "Done", + detectedFrom: ".env", + } + ); + expect(w.output).toContain("User: Alice\n"); + expect(w.output).toContain("Detected from .env"); + expect(w.output).toContain("Done"); + }); + }); +}); + +describe("writeFooter", () => { + test("writes empty line followed by muted text", () => { + const w = createTestWriter(); + writeFooter(w, "Some hint"); + // First chunk is the empty line separator + expect(w.chunks[0]).toBe("\n"); + // Second chunk contains the hint text with trailing newline + expect(w.chunks[1]).toContain("Some hint"); + expect(w.chunks[1]).toEndWith("\n"); + }); +});