Skip to content

Commit 1e6cfd4

Browse files
committed
fix: address BugBot review comments
- org/list: show both truncation notice and tip hint when results are truncated (was either-or due to else-if) - cli/setup: rename local quiet-aware `log` to `emit` and module-level `cmdLog` to `log` to match the naming convention used by all other commands (avoids shadowing confusion) - trace/list: keep empty-page next-page command inline in hint text instead of splitting it across main output and de-emphasized footer
1 parent 27fe45a commit 1e6cfd4

3 files changed

Lines changed: 48 additions & 52 deletions

File tree

src/commands/cli/setup.ts

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import {
4242
parseInstallationMethod,
4343
} from "../../lib/upgrade.js";
4444

45-
const cmdLog = logger.withTag("cli.setup");
45+
const log = logger.withTag("cli.setup");
4646

4747
type SetupFlags = {
4848
readonly install: boolean;
@@ -74,7 +74,7 @@ async function handleInstall(
7474
execPath: string,
7575
homeDir: string,
7676
env: NodeJS.ProcessEnv,
77-
log: Logger
77+
emit: Logger
7878
): Promise<{ binaryPath: string; binaryDir: string; created: boolean }> {
7979
const installDir = determineInstallDir(homeDir, env);
8080
const targetPath = join(installDir, getBinaryFilename());
@@ -83,7 +83,7 @@ async function handleInstall(
8383
const binaryPath = await installBinary(execPath, installDir);
8484
const binaryDir = dirname(binaryPath);
8585

86-
log(`Binary: Installed to ${binaryPath}`);
86+
emit(`Binary: Installed to ${binaryPath}`);
8787

8888
// Clean up temp binary (Posix only — the inode stays alive for the running process)
8989
if (process.platform !== "win32") {
@@ -104,37 +104,37 @@ async function handlePathModification(
104104
binaryDir: string,
105105
shell: ShellInfo,
106106
env: NodeJS.ProcessEnv,
107-
log: Logger
107+
emit: Logger
108108
): Promise<void> {
109109
const alreadyInPath = isInPath(binaryDir, env.PATH);
110110

111111
if (alreadyInPath) {
112-
log(`PATH: ${binaryDir} is already in PATH`);
112+
emit(`PATH: ${binaryDir} is already in PATH`);
113113
return;
114114
}
115115

116116
if (shell.configFile) {
117117
const result = await addToPath(shell.configFile, binaryDir, shell.type);
118118

119119
if (result.modified) {
120-
log(`PATH: ${result.message}`);
121-
log(` Restart your shell or run: source ${shell.configFile}`);
120+
emit(`PATH: ${result.message}`);
121+
emit(` Restart your shell or run: source ${shell.configFile}`);
122122
} else if (result.manualCommand) {
123-
log(`PATH: ${result.message}`);
124-
log(` Add manually: ${result.manualCommand}`);
123+
emit(`PATH: ${result.message}`);
124+
emit(` Add manually: ${result.manualCommand}`);
125125
} else {
126-
log(`PATH: ${result.message}`);
126+
emit(`PATH: ${result.message}`);
127127
}
128128
} else {
129129
const cmd = getPathCommand(shell.type, binaryDir);
130-
log("PATH: No shell config file found");
131-
log(` Add manually to your shell config: ${cmd}`);
130+
emit("PATH: No shell config file found");
131+
emit(` Add manually to your shell config: ${cmd}`);
132132
}
133133

134134
// Handle GitHub Actions
135135
const addedToGitHub = await addToGitHubPath(binaryDir, env);
136136
if (addedToGitHub) {
137-
log("PATH: Added to $GITHUB_PATH");
137+
emit("PATH: Added to $GITHUB_PATH");
138138
}
139139
}
140140

@@ -235,30 +235,30 @@ async function handleCompletions(
235235
* Only produces output when the skill file is freshly created. Subsequent
236236
* runs (e.g. after upgrade) silently update without printing.
237237
*/
238-
async function handleAgentSkills(homeDir: string, log: Logger): Promise<void> {
238+
async function handleAgentSkills(homeDir: string, emit: Logger): Promise<void> {
239239
const location = await installAgentSkills(homeDir, CLI_VERSION);
240240

241241
if (location?.created) {
242-
log(`Agent skills: Installed to ${location.path}`);
242+
emit(`Agent skills: Installed to ${location.path}`);
243243
}
244244
}
245245

246246
/**
247247
* Print a rich welcome message after fresh install.
248248
*/
249249
function printWelcomeMessage(
250-
log: Logger,
250+
emit: Logger,
251251
version: string,
252252
binaryPath: string
253253
): void {
254-
log("");
255-
log(`Installed sentry v${version} to ${binaryPath}`);
256-
log("");
257-
log("Get started:");
258-
log(" sentry auth login Authenticate with Sentry");
259-
log(" sentry --help See all available commands");
260-
log("");
261-
log("https://cli.sentry.dev");
254+
emit("");
255+
emit(`Installed sentry v${version} to ${binaryPath}`);
256+
emit("");
257+
emit("Get started:");
258+
emit(" sentry auth login Authenticate with Sentry");
259+
emit(" sentry --help See all available commands");
260+
emit("");
261+
emit("https://cli.sentry.dev");
262262
}
263263

264264
type WarnLogger = (step: string, error: unknown) => void;
@@ -291,7 +291,7 @@ type ConfigStepOptions = {
291291
readonly binaryDir: string;
292292
readonly homeDir: string;
293293
readonly env: NodeJS.ProcessEnv;
294-
readonly log: Logger;
294+
readonly emit: Logger;
295295
readonly warn: WarnLogger;
296296
};
297297

@@ -302,7 +302,7 @@ type ConfigStepOptions = {
302302
* error) doesn't prevent the others from running.
303303
*/
304304
async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
305-
const { flags, binaryPath, binaryDir, homeDir, env, log, warn } = opts;
305+
const { flags, binaryPath, binaryDir, homeDir, env, emit, warn } = opts;
306306
const shell = detectShell(env.SHELL, homeDir, env.XDG_CONFIG_HOME);
307307

308308
// 1. Record installation info
@@ -317,7 +317,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
317317
version: CLI_VERSION,
318318
});
319319
if (!flags.install) {
320-
log(`Recorded installation method: ${method}`);
320+
emit(`Recorded installation method: ${method}`);
321321
}
322322
},
323323
warn
@@ -332,7 +332,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
332332
() => {
333333
setReleaseChannel(channel);
334334
if (!flags.install) {
335-
log(`Recorded release channel: ${channel}`);
335+
emit(`Recorded release channel: ${channel}`);
336336
}
337337
},
338338
warn
@@ -343,7 +343,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
343343
if (!flags["no-modify-path"]) {
344344
await bestEffort(
345345
"PATH modification",
346-
() => handlePathModification(binaryDir, shell, env, log),
346+
() => handlePathModification(binaryDir, shell, env, emit),
347347
warn
348348
);
349349
}
@@ -360,7 +360,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
360360
env.PATH
361361
);
362362
for (const line of completionLines) {
363-
log(line);
363+
emit(line);
364364
}
365365
},
366366
warn
@@ -371,7 +371,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions): Promise<void> {
371371
if (!flags["no-agent-skills"]) {
372372
await bestEffort(
373373
"Agent skills",
374-
() => handleAgentSkills(homeDir, log),
374+
() => handleAgentSkills(homeDir, emit),
375375
warn
376376
);
377377
}
@@ -444,16 +444,16 @@ export const setupCommand = buildCommand({
444444
async func(this: SentryContext, flags: SetupFlags): Promise<void> {
445445
const { process, homeDir } = this;
446446

447-
const log: Logger = (msg: string) => {
447+
const emit: Logger = (msg: string) => {
448448
if (!flags.quiet) {
449-
cmdLog.info(msg);
449+
log.info(msg);
450450
}
451451
};
452452

453453
const warn: WarnLogger = (step, error) => {
454454
const msg =
455455
error instanceof Error ? error.message : "Unknown error occurred";
456-
cmdLog.warn(`${step} failed: ${msg}`);
456+
log.warn(`${step} failed: ${msg}`);
457457
};
458458

459459
let binaryPath = process.execPath;
@@ -466,7 +466,7 @@ export const setupCommand = buildCommand({
466466
process.execPath,
467467
homeDir,
468468
process.env,
469-
log
469+
emit
470470
);
471471
binaryPath = result.binaryPath;
472472
binaryDir = result.binaryDir;
@@ -480,14 +480,14 @@ export const setupCommand = buildCommand({
480480
binaryDir,
481481
homeDir,
482482
env: process.env,
483-
log,
483+
emit,
484484
warn,
485485
});
486486

487487
// 5. Print welcome message only on fresh install — upgrades are silent
488488
// since the upgrade command itself prints a success message.
489489
if (!flags.quiet && freshInstall) {
490-
printWelcomeMessage(log, CLI_VERSION, binaryPath);
490+
printWelcomeMessage(emit, CLI_VERSION, binaryPath);
491491
}
492492
},
493493
});

src/commands/org/list.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,14 @@ export const listCommand = buildCommand({
143143
: undefined,
144144
}));
145145

146-
let hint: string | undefined;
146+
const hints: string[] = [];
147147
if (orgs.length > flags.limit) {
148-
hint = `Showing ${flags.limit} of ${orgs.length} organizations`;
149-
} else if (entries.length > 0) {
150-
hint = "Tip: Use 'sentry org view <slug>' for details";
148+
hints.push(`Showing ${flags.limit} of ${orgs.length} organizations`);
149+
}
150+
if (entries.length > 0) {
151+
hints.push("Tip: Use 'sentry org view <slug>' for details");
151152
}
152-
// When entries is empty, hint remains undefined — no tip is shown
153153

154-
return { data: entries, hint };
154+
return { data: entries, hint: hints.join("\n") || undefined };
155155
},
156156
});

src/commands/trace/list.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,19 +118,15 @@ export function parseSort(value: string): SortValue {
118118
/**
119119
* Format trace list data for human-readable terminal output.
120120
*
121-
* Handles three display states:
122-
* - Empty list with more pages → "No traces on this page."
123-
* - Empty list, no more pages → "No traces found."
121+
* Handles two display states:
122+
* - Empty list → "No traces found." (next-page hint is in CommandOutput.hint)
124123
* - Non-empty → header line + formatted table
125-
*
126-
* The hint/footer is handled separately by the {@link CommandOutput.hint}
127-
* field, not by this formatter.
128124
*/
129125
function formatTraceListHuman(result: TraceListResult): string {
130-
const { traces, hasMore, org, project } = result;
126+
const { traces, org, project } = result;
131127

132128
if (traces.length === 0) {
133-
return hasMore ? "No traces on this page." : "No traces found.";
129+
return "No traces found.";
134130
}
135131

136132
return `Recent traces in ${org}/${project}:\n\n${formatTraceTable(traces)}`;
@@ -271,7 +267,7 @@ export const listCommand = buildListCommand("trace", {
271267
// Build footer hint based on result state
272268
let hint: string | undefined;
273269
if (traces.length === 0 && hasMore) {
274-
hint = `Try the next page: ${nextPageHint(org, project, flags)}`;
270+
hint = `No traces on this page. Try the next page: ${nextPageHint(org, project, flags)}`;
275271
} else if (traces.length > 0) {
276272
const countText = `Showing ${traces.length} trace${traces.length === 1 ? "" : "s"}.`;
277273
hint = hasMore

0 commit comments

Comments
 (0)