Skip to content

Commit 343b3ef

Browse files
committed
refactor: extract startSpinner helper, use running total for progress, report NaN counts
- Extract startSpinner() from duplicated spinner code in poll() and withProgress() — eliminates ~30 lines of duplication - Set stopped = true unconditionally in poll() finally block, not just in non-JSON mode - Replace fetchedCounts array + reduce with running total + delta tracking in multi-target progress (onPage reports cumulative, not increments) - Use '?'.padStart(COL_COUNT) for NaN placeholder instead of hardcoded spaces, and report to Sentry as a warning for unexpected non-numeric input
1 parent 65b3acf commit 343b3ef

File tree

3 files changed

+59
-45
lines changed

3 files changed

+59
-45
lines changed

src/commands/issue/list.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,11 @@ async function handleResolvedTargets(
600600
const results = await withProgress(
601601
{ stderr, message: "Fetching issues..." },
602602
(setMessage) => {
603-
const fetchedCounts = new Array<number>(targets.length).fill(0);
603+
// Track per-target previous counts to compute deltas — onPage reports the
604+
// running total for each target, not increments, so we need the previous
605+
// value to derive how many new items arrived per callback.
606+
let totalFetched = 0;
607+
const prevFetched = new Array<number>(targets.length).fill(0);
604608
const totalLimit = flags.limit * targets.length;
605609
return Promise.all(
606610
targets.map((t, i) =>
@@ -610,8 +614,8 @@ async function handleResolvedTargets(
610614
sort: flags.sort,
611615
statsPeriod: flags.period,
612616
onPage: (fetched) => {
613-
fetchedCounts[i] = fetched;
614-
const totalFetched = fetchedCounts.reduce((a, b) => a + b, 0);
617+
totalFetched += fetched - (prevFetched[i] ?? 0);
618+
prevFetched[i] = fetched;
615619
setMessage(`Fetching issues... ${totalFetched}/${totalLimit}`);
616620
},
617621
})

src/lib/formatters/human.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* Follows gh cli patterns for alignment and presentation.
66
*/
77

8+
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
9+
import * as Sentry from "@sentry/bun";
810
import prettyMs from "pretty-ms";
911
import type {
1012
Breadcrumb,
@@ -333,7 +335,11 @@ function abbreviateCount(raw: string): string {
333335
if (Number.isNaN(n)) {
334336
// Non-numeric input: use a placeholder rather than passing through an
335337
// arbitrarily wide string that would break column alignment
336-
return " ?";
338+
Sentry.captureMessage(
339+
`Unexpected non-numeric issue count: ${raw}`,
340+
"warning"
341+
);
342+
return "?".padStart(COL_COUNT);
337343
}
338344
if (n < 10_000) {
339345
return raw.padStart(COL_COUNT);

src/lib/polling.ts

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -83,31 +83,17 @@ export async function poll<T>(options: PollOptions<T>): Promise<T> {
8383
} = options;
8484

8585
const startTime = Date.now();
86-
let tick = 0;
87-
let currentMessage = initialMessage;
88-
89-
// Animation timer runs independently of polling for smooth spinner
90-
let stopped = false;
91-
if (!json) {
92-
const scheduleFrame = () => {
93-
if (stopped) {
94-
return;
95-
}
96-
const display = truncateProgressMessage(currentMessage);
97-
stderr.write(`\r\x1b[K${formatProgressLine(display, tick)}`);
98-
tick += 1;
99-
setTimeout(scheduleFrame, ANIMATION_INTERVAL_MS).unref();
100-
};
101-
scheduleFrame();
102-
}
86+
const spinner = json ? null : startSpinner(stderr, initialMessage);
10387

10488
try {
10589
while (Date.now() - startTime < timeoutMs) {
10690
const state = await fetchState();
10791

10892
if (state) {
109-
// Update message for animation loop to display
110-
currentMessage = getProgressMessage(state);
93+
// Always call getProgressMessage (callers may rely on the callback
94+
// being invoked), but only forward the result to the spinner.
95+
const msg = getProgressMessage(state);
96+
spinner?.setMessage(msg);
11197

11298
if (shouldStop(state)) {
11399
return state;
@@ -119,14 +105,48 @@ export async function poll<T>(options: PollOptions<T>): Promise<T> {
119105

120106
throw new Error(timeoutMessage);
121107
} finally {
122-
// Clean up animation timer
108+
spinner?.stop();
123109
if (!json) {
124-
stopped = true;
125110
stderr.write("\n");
126111
}
127112
}
128113
}
129114

115+
/**
116+
* Start an animated spinner that writes progress to stderr.
117+
*
118+
* Returns a controller with `setMessage` to update the displayed text
119+
* and `stop` to halt the animation.
120+
*/
121+
function startSpinner(
122+
stderr: Writer,
123+
initialMessage: string
124+
): { setMessage: (msg: string) => void; stop: () => void } {
125+
let currentMessage = initialMessage;
126+
let tick = 0;
127+
let stopped = false;
128+
129+
const scheduleFrame = () => {
130+
if (stopped) {
131+
return;
132+
}
133+
const display = truncateProgressMessage(currentMessage);
134+
stderr.write(`\r\x1b[K${formatProgressLine(display, tick)}`);
135+
tick += 1;
136+
setTimeout(scheduleFrame, ANIMATION_INTERVAL_MS).unref();
137+
};
138+
scheduleFrame();
139+
140+
return {
141+
setMessage: (msg: string) => {
142+
currentMessage = msg;
143+
},
144+
stop: () => {
145+
stopped = true;
146+
},
147+
};
148+
}
149+
130150
/**
131151
* Options for {@link withProgress}.
132152
*/
@@ -169,28 +189,12 @@ export async function withProgress<T>(
169189
options: WithProgressOptions,
170190
fn: (setMessage: (msg: string) => void) => Promise<T>
171191
): Promise<T> {
172-
const { stderr } = options;
173-
let currentMessage = options.message;
174-
let tick = 0;
175-
let stopped = false;
176-
177-
const scheduleFrame = () => {
178-
if (stopped) {
179-
return;
180-
}
181-
const display = truncateProgressMessage(currentMessage);
182-
stderr.write(`\r\x1b[K${formatProgressLine(display, tick)}`);
183-
tick += 1;
184-
setTimeout(scheduleFrame, ANIMATION_INTERVAL_MS).unref();
185-
};
186-
scheduleFrame();
192+
const spinner = startSpinner(options.stderr, options.message);
187193

188194
try {
189-
return await fn((msg) => {
190-
currentMessage = msg;
191-
});
195+
return await fn(spinner.setMessage);
192196
} finally {
193-
stopped = true;
194-
stderr.write("\r\x1b[K");
197+
spinner.stop();
198+
options.stderr.write("\r\x1b[K");
195199
}
196200
}

0 commit comments

Comments
 (0)