Skip to content

Commit 92e1103

Browse files
committed
refactor: extract parseDualModeArgs from duplicated disambiguation logic
Both parseSpanListArgs (span/list.ts) and parseLogListArgs (log/list.ts) had near-identical control flow for detecting whether positional args contain a trace ID. Extracted the shared logic into parseDualModeArgs() in trace-target.ts, parameterized by the usage hint string. Both commands now delegate to this shared utility via thin wrappers. Addresses BugBot review feedback about duplicated argument disambiguation.
1 parent 313fe00 commit 92e1103

File tree

3 files changed

+91
-125
lines changed

3 files changed

+91
-125
lines changed

src/commands/log/list.ts

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ import {
3434
import { logger } from "../../lib/logger.js";
3535
import { withProgress } from "../../lib/polling.js";
3636
import { resolveOrgProjectFromArg } from "../../lib/resolve-target.js";
37-
import { isTraceId } from "../../lib/trace-id.js";
3837
import {
39-
type ParsedTraceTarget,
40-
parseTraceTarget,
38+
parseDualModeArgs,
4139
resolveTraceOrg,
4240
warnIfNormalized,
4341
} from "../../lib/trace-target.js";
@@ -144,69 +142,16 @@ type FetchResult = {
144142
// Positional argument disambiguation
145143
// ---------------------------------------------------------------------------
146144

147-
/**
148-
* Parsed result from log list positional arguments.
149-
*
150-
* Discriminated on `mode`:
151-
* - `"project"` — standard project-scoped log listing (existing path)
152-
* - `"trace"` — trace-filtered log listing via trace-logs endpoint
153-
*/
154-
type ParsedLogArgs =
155-
| { mode: "project"; target?: string }
156-
| { mode: "trace"; parsed: ParsedTraceTarget };
157-
158145
/**
159146
* Disambiguate log list positional arguments.
160147
*
161-
* Detects trace mode by checking whether any argument segment looks like
162-
* a 32-char hex trace ID:
163-
*
164-
* - **Single arg**: checks the tail segment (last part after `/`, or the
165-
* entire arg). `<trace-id>`, `<org>/<trace-id>`, `<org>/<project>/<trace-id>`.
166-
* - **Two+ args**: checks the last positional (`<org> <trace-id>` or
167-
* `<org>/<project> <trace-id>` space-separated forms).
168-
* - **No match**: treats the argument as a project target.
169-
*
170-
* When trace mode is detected, delegates to {@link parseTraceTarget} for
171-
* full parsing and validation.
172-
*
173-
* @param args - Positional arguments from CLI
174-
* @returns Parsed args with mode discrimination
148+
* Thin wrapper around {@link parseDualModeArgs} that binds the
149+
* trace-mode usage hint for log list.
175150
*/
176-
function parseLogListArgs(args: string[]): ParsedLogArgs {
177-
if (args.length === 0) {
178-
return { mode: "project" };
179-
}
180-
181-
const first = args[0];
182-
if (first === undefined) {
183-
return { mode: "project" };
184-
}
185-
186-
// Two+ args: check if the last arg is a trace ID (space-separated form)
187-
// e.g., `sentry log list my-org abc123...` or `sentry log list my-org/proj abc123...`
188-
if (args.length >= 2) {
189-
const last = args.at(-1);
190-
if (last && isTraceId(last)) {
191-
return {
192-
mode: "trace",
193-
parsed: parseTraceTarget(args, TRACE_USAGE_HINT),
194-
};
195-
}
196-
}
197-
198-
// Single arg: check the tail segment (last part after `/`, or the entire arg)
199-
const lastSlash = first.lastIndexOf("/");
200-
const tail = lastSlash === -1 ? first : first.slice(lastSlash + 1);
201-
202-
if (isTraceId(tail)) {
203-
return {
204-
mode: "trace",
205-
parsed: parseTraceTarget(args, TRACE_USAGE_HINT),
206-
};
207-
}
208-
209-
return { mode: "project", target: first };
151+
function parseLogListArgs(
152+
args: string[]
153+
): ReturnType<typeof parseDualModeArgs> {
154+
return parseDualModeArgs(args, TRACE_USAGE_HINT);
210155
}
211156

212157
/** Default time period for project-scoped log queries */

src/commands/span/list.ts

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,9 @@ import {
3737
} from "../../lib/list-command.js";
3838
import { withProgress } from "../../lib/polling.js";
3939
import { resolveOrgProjectFromArg } from "../../lib/resolve-target.js";
40-
import { isTraceId } from "../../lib/trace-id.js";
4140
import {
4241
type ParsedTraceTarget,
43-
parseTraceTarget,
42+
parseDualModeArgs,
4443
resolveTraceOrgProject,
4544
warnIfNormalized,
4645
} from "../../lib/trace-target.js";
@@ -109,70 +108,16 @@ export function parseSort(value: string): SpanSortValue {
109108
return value as SpanSortValue;
110109
}
111110

112-
// ---------------------------------------------------------------------------
113-
// Positional argument disambiguation
114-
// ---------------------------------------------------------------------------
115-
116-
/**
117-
* Parsed result from span list positional arguments.
118-
*
119-
* Discriminated on `mode`:
120-
* - `"project"` — project-scoped span listing (no trace ID)
121-
* - `"trace"` — trace-scoped span listing (trace ID provided)
122-
*/
123-
type ParsedSpanListArgs =
124-
| { mode: "project"; target?: string }
125-
| { mode: "trace"; parsed: ParsedTraceTarget };
126-
127111
/**
128112
* Disambiguate span list positional arguments.
129113
*
130-
* Detects trace mode by checking whether any argument segment looks like
131-
* a 32-char hex trace ID:
132-
*
133-
* - **No args**: project mode (auto-detect org/project)
134-
* - **Single arg**: checks the tail segment (last part after `/`). If it
135-
* looks like a trace ID → trace mode. Otherwise → project target.
136-
* - **Two+ args**: checks the last positional. If it's a trace ID → trace
137-
* mode (space-separated form). Otherwise → project target (first arg only).
138-
*
139-
* @param args - Positional arguments from CLI
140-
* @returns Parsed args with mode discrimination
114+
* Thin wrapper around {@link parseDualModeArgs} that binds the
115+
* trace-mode usage hint for span list.
141116
*/
142-
export function parseSpanListArgs(args: string[]): ParsedSpanListArgs {
143-
if (args.length === 0) {
144-
return { mode: "project" };
145-
}
146-
147-
const first = args[0];
148-
if (first === undefined) {
149-
return { mode: "project" };
150-
}
151-
152-
// Two+ args: check if the last arg is a trace ID (space-separated form)
153-
// e.g., `sentry span list my-project abc123...`
154-
if (args.length >= 2) {
155-
const last = args.at(-1);
156-
if (last && isTraceId(last)) {
157-
return {
158-
mode: "trace",
159-
parsed: parseTraceTarget(args, TRACE_USAGE_HINT),
160-
};
161-
}
162-
}
163-
164-
// Single arg: check the tail segment (last part after "/", or entire arg)
165-
const segments = first.split("/");
166-
const tail = segments.at(-1) ?? first;
167-
if (isTraceId(tail)) {
168-
return {
169-
mode: "trace",
170-
parsed: parseTraceTarget(args, TRACE_USAGE_HINT),
171-
};
172-
}
173-
174-
// Not a trace ID → treat as project target
175-
return { mode: "project", target: first };
117+
export function parseSpanListArgs(
118+
args: string[]
119+
): ReturnType<typeof parseDualModeArgs> {
120+
return parseDualModeArgs(args, TRACE_USAGE_HINT);
176121
}
177122

178123
// ---------------------------------------------------------------------------

src/lib/trace-target.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
resolveOrgAndProject,
2525
resolveProjectBySlug,
2626
} from "./resolve-target.js";
27-
import { validateTraceId } from "./trace-id.js";
27+
import { isTraceId, validateTraceId } from "./trace-id.js";
2828

2929
/** Match `[<prefix>]<trail>` in usageHint — captures bracket content + trailing placeholder */
3030
const USAGE_TARGET_RE = /\[.*\]<[^>]+>/;
@@ -397,3 +397,79 @@ export async function resolveTraceOrg(
397397
}
398398
}
399399
}
400+
401+
// ---------------------------------------------------------------------------
402+
// Dual-mode argument disambiguation (project vs trace)
403+
// ---------------------------------------------------------------------------
404+
405+
/**
406+
* Result from dual-mode argument disambiguation.
407+
*
408+
* Used by commands that support both project-scoped listing (no trace ID)
409+
* and trace-scoped listing (trace ID provided), like `span list` and
410+
* `log list`.
411+
*
412+
* Discriminated on `mode`:
413+
* - `"project"` — no trace ID detected; `target` is the optional org/project arg
414+
* - `"trace"` — a 32-char hex trace ID was found; `parsed` contains the full target
415+
*/
416+
export type ParsedDualModeArgs =
417+
| { mode: "project"; target?: string }
418+
| { mode: "trace"; parsed: ParsedTraceTarget };
419+
420+
/**
421+
* Disambiguate positional arguments for dual-mode list commands.
422+
*
423+
* Detects trace mode by checking whether any argument segment looks like
424+
* a 32-char hex trace ID via {@link isTraceId}:
425+
*
426+
* - **No args**: project mode (auto-detect org/project)
427+
* - **Two+ args**: checks the last positional. If it's a trace ID → trace
428+
* mode (space-separated form like `<project> <trace-id>`).
429+
* - **Single arg**: checks the tail segment (last part after `/`). If it
430+
* looks like a trace ID → trace mode. Otherwise → project target.
431+
*
432+
* When trace mode is detected, delegates to {@link parseTraceTarget} for
433+
* full parsing and validation.
434+
*
435+
* @param args - Positional arguments from CLI
436+
* @param traceUsageHint - Usage hint for trace-mode error messages
437+
* @returns Parsed args with mode discrimination
438+
*/
439+
export function parseDualModeArgs(
440+
args: string[],
441+
traceUsageHint: string
442+
): ParsedDualModeArgs {
443+
if (args.length === 0) {
444+
return { mode: "project" };
445+
}
446+
447+
const first = args[0];
448+
if (first === undefined) {
449+
return { mode: "project" };
450+
}
451+
452+
// Two+ args: check if the last arg is a trace ID (space-separated form)
453+
if (args.length >= 2) {
454+
const last = args.at(-1);
455+
if (last && isTraceId(last)) {
456+
return {
457+
mode: "trace",
458+
parsed: parseTraceTarget(args, traceUsageHint),
459+
};
460+
}
461+
}
462+
463+
// Single arg: check the tail segment (last part after "/", or entire arg)
464+
const lastSlash = first.lastIndexOf("/");
465+
const tail = lastSlash === -1 ? first : first.slice(lastSlash + 1);
466+
if (isTraceId(tail)) {
467+
return {
468+
mode: "trace",
469+
parsed: parseTraceTarget(args, traceUsageHint),
470+
};
471+
}
472+
473+
// Not a trace ID → treat as project target
474+
return { mode: "project", target: first };
475+
}

0 commit comments

Comments
 (0)