Skip to content

Commit 0a4083b

Browse files
committed
refactor: address round 4 review comments
1. Remove isCommandOutput helper — inline instanceof check directly in handleYieldedValue. 2. Remove onPolling callback from DeviceFlowCallbacks in oauth.ts — spinner handles visual feedback now. 3. Convert 11 "yield new CommandOutput(..); return;" patterns to "return yield new CommandOutput(..)" one-liners across 9 files. 4. Add output config to auth/token — emits token as structured data through the framework. JSON: {"token": "..."}, human: raw token. 5. Add output config to cli/setup — collects messages and warnings into SetupResult, yields through framework. JSON output includes structured messages/warnings/binaryPath/version. 6. Remove hasJsonOutput intermediate — use outputConfig directly. Only init.ts remains without output config (interactive wizard, deferred to separate PR).
1 parent b08a507 commit 0a4083b

14 files changed

Lines changed: 72 additions & 60 deletions

File tree

src/commands/api.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,6 @@ export const apiCommand = buildCommand({
12101210
throw new OutputError(response.body);
12111211
}
12121212

1213-
yield new CommandOutput(response.body);
1214-
return;
1213+
return yield new CommandOutput(response.body);
12151214
},
12161215
});

src/commands/auth/login.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,7 @@ export const loginCommand = buildCommand({
182182
// Non-fatal: user info is supplementary. Token remains stored and valid.
183183
}
184184

185-
yield new CommandOutput(result);
186-
return;
185+
return yield new CommandOutput(result);
187186
}
188187

189188
// OAuth device flow

src/commands/auth/refresh.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ Examples:
105105
: undefined,
106106
};
107107

108-
yield new CommandOutput(payload);
109-
return;
108+
return yield new CommandOutput(payload);
110109
},
111110
});

src/commands/auth/status.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ export const statusCommand = buildCommand({
190190
verification: await verifyCredentials(),
191191
};
192192

193-
yield new CommandOutput(data);
194-
return;
193+
return yield new CommandOutput(data);
195194
},
196195
});

src/commands/auth/token.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,25 @@ import type { SentryContext } from "../../context.js";
99
import { buildCommand } from "../../lib/command.js";
1010
import { getAuthToken } from "../../lib/db/auth.js";
1111
import { AuthError } from "../../lib/errors.js";
12+
import { CommandOutput, stateless } from "../../lib/formatters/output.js";
1213

1314
export const tokenCommand = buildCommand({
1415
docs: {
1516
brief: "Print the stored authentication token",
1617
fullDescription:
1718
"Print the stored authentication token to stdout.\n\n" +
1819
"This outputs the raw token without any formatting, making it suitable for " +
19-
"piping to other commands or scripts. The token is printed without a trailing newline " +
20-
"when stdout is not a TTY (e.g., when piped).",
20+
"piping to other commands or scripts.",
2121
},
2222
parameters: {},
23-
// biome-ignore lint/correctness/useYield: void generator — writes to stdout directly
23+
output: { human: stateless((token: string) => token) },
2424
// biome-ignore lint/suspicious/useAwait: sync body but async generator required by buildCommand
2525
async *func(this: SentryContext) {
26-
const { stdout } = this;
27-
2826
const token = getAuthToken();
2927
if (!token) {
3028
throw new AuthError("not_authenticated");
3129
}
3230

33-
// Add newline only if stdout is a TTY (interactive terminal)
34-
// When piped, omit newline for cleaner output
35-
const suffix = process.stdout.isTTY ? "\n" : "";
36-
stdout.write(`${token}${suffix}`);
31+
return yield new CommandOutput(token);
3732
},
3833
});

src/commands/auth/whoami.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ export const whoamiCommand = buildCommand({
6565
// Cache update failure is non-essential — user identity was already fetched.
6666
}
6767

68-
yield new CommandOutput(user);
69-
return;
68+
return yield new CommandOutput(user);
7069
},
7170
});

src/commands/cli/fix.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,6 @@ export const fixCommand = buildCommand({
735735
throw new OutputError(result);
736736
}
737737

738-
yield new CommandOutput(result);
739-
return;
738+
return yield new CommandOutput(result);
740739
},
741740
});

src/commands/cli/setup.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
type ReleaseChannel,
2828
setReleaseChannel,
2929
} from "../../lib/db/release-channel.js";
30-
import { logger } from "../../lib/logger.js";
30+
import { CommandOutput, stateless } from "../../lib/formatters/output.js";
3131
import {
3232
addToGitHubPath,
3333
addToPath,
@@ -42,8 +42,6 @@ import {
4242
parseInstallationMethod,
4343
} from "../../lib/upgrade.js";
4444

45-
const log = logger.withTag("cli.setup");
46-
4745
type SetupFlags = {
4846
readonly install: boolean;
4947
readonly method?: InstallationMethod;
@@ -56,6 +54,31 @@ type SetupFlags = {
5654

5755
type Logger = (msg: string) => void;
5856

57+
/** Structured result of the setup operation */
58+
type SetupResult = {
59+
/** Status messages collected during setup */
60+
messages: string[];
61+
/** Warning messages from best-effort steps */
62+
warnings: string[];
63+
/** Whether a fresh binary was installed */
64+
freshInstall: boolean;
65+
/** Path to the installed binary */
66+
binaryPath: string;
67+
/** CLI version */
68+
version: string;
69+
};
70+
71+
/** Format setup result for human-readable output */
72+
function formatSetupResult(result: SetupResult): string {
73+
const lines: string[] = [...result.messages];
74+
if (result.warnings.length > 0) {
75+
for (const w of result.warnings) {
76+
lines.push(`⚠ ${w}`);
77+
}
78+
}
79+
return lines.join("\n");
80+
}
81+
5982
/**
6083
* Handle binary installation from a temp location.
6184
*
@@ -441,19 +464,23 @@ export const setupCommand = buildCommand({
441464
},
442465
},
443466
},
467+
output: { human: stateless(formatSetupResult) },
444468
async *func(this: SentryContext, flags: SetupFlags) {
445469
const { process, homeDir } = this;
446470

471+
const messages: string[] = [];
472+
const warnings: string[] = [];
473+
447474
const emit: Logger = (msg: string) => {
448475
if (!flags.quiet) {
449-
log.info(msg);
476+
messages.push(msg);
450477
}
451478
};
452479

453480
const warn: WarnLogger = (step, error) => {
454481
const msg =
455482
error instanceof Error ? error.message : "Unknown error occurred";
456-
log.warn(`${step} failed: ${msg}`);
483+
warnings.push(`${step} failed: ${msg}`);
457484
};
458485

459486
let binaryPath = process.execPath;
@@ -489,5 +516,13 @@ export const setupCommand = buildCommand({
489516
if (!flags.quiet && freshInstall) {
490517
printWelcomeMessage(emit, CLI_VERSION, binaryPath);
491518
}
519+
520+
return yield new CommandOutput<SetupResult>({
521+
messages,
522+
warnings,
523+
freshInstall,
524+
binaryPath,
525+
version: CLI_VERSION,
526+
});
492527
},
493528
});

src/commands/cli/upgrade.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,8 +494,7 @@ export const upgradeCommand = buildCommand({
494494
flags,
495495
});
496496
if (resolved.kind === "done") {
497-
yield new CommandOutput(resolved.result);
498-
return;
497+
return yield new CommandOutput(resolved.result);
499498
}
500499

501500
const { target } = resolved;

src/commands/issue/plan.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,7 @@ export const planCommand = buildCommand({
225225
if (!flags.force) {
226226
const existingSolution = extractSolution(state);
227227
if (existingSolution) {
228-
yield new CommandOutput(buildPlanData(state));
229-
return;
228+
return yield new CommandOutput(buildPlanData(state));
230229
}
231230
}
232231

@@ -261,8 +260,7 @@ export const planCommand = buildCommand({
261260
throw new Error("Plan creation was cancelled.");
262261
}
263262

264-
yield new CommandOutput(buildPlanData(finalState));
265-
return;
263+
return yield new CommandOutput(buildPlanData(finalState));
266264
} catch (error) {
267265
// Handle API errors with friendly messages
268266
if (error instanceof ApiError) {

0 commit comments

Comments
 (0)