Skip to content

Commit 68cae8a

Browse files
committed
Address review: fix clipAnsi reset, section/path width floors, normalizeScrollOffset fixed-point
Four follow-up fixes from code review on PR #106: 1. clipAnsi: replace \x1b[0m (full SGR reset) with \x1b[22;39m (reset bold + fg only). A full reset clears any background set by the caller, so renderActiveLine's dark-purple bg was wiped mid-line on narrow terminals when the repo name or file path was clipped. 2. Section label width: change Math.max(4, termWidth - SECTION_FIXED) to Math.max(0, …). The old floor of 4 could exceed the actual available width on very narrow terminals, forcing the section header line wider than termWidth and re-introducing wrapping. When maxLabelChars === 0 the section is counted toward usedLines but not pushed to output. 3. Extract path width: change Math.max(10, …) to Math.max(1, …). Same reasoning as above — the floor of 10 could exceed the available width on very narrow terminals. 4. normalizeScrollOffset fixed-point loop: replace single-pass call with a loop that iterates until (scrollOffset, viewportHeight) stabilises. getViewportHeight depends on scrollOffset (sticky-header condition), so decreasing scrollOffset can toggle the sticky header, changing viewportHeight by 1 and causing a single pass to stop 1 row early.
1 parent b5c4382 commit 68cae8a

2 files changed

Lines changed: 32 additions & 5 deletions

File tree

src/render.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,12 @@ function clipAnsi(str: string, maxVisible: number): string {
226226
}
227227
visCount++;
228228
if (visCount === maxVisible) {
229-
// Cut after this visible char and reset SGR.
230-
return str.slice(0, i + 1) + "\x1b[0m";
229+
// Cut after this visible char. Use a targeted SGR reset that clears bold
230+
// and foreground color (22;39) but deliberately leaves background color
231+
// (49) untouched. A full \x1b[0m reset would clear any background set
232+
// by the *caller* (e.g. renderActiveLine's dark-purple bg), causing the
233+
// remainder of the active row to lose its highlight — see issue #105.
234+
return str.slice(0, i + 1) + "\x1b[22;39m";
231235
}
232236
i++;
233237
}
@@ -499,7 +503,16 @@ export function renderGroups(
499503
// Fix: clip section label to termWidth so the label line never wraps.
500504
// "── " prefix is 3 visible chars + 1 trailing space = 4 chars total.
501505
const SECTION_FIXED = 4; // "── " (3) + trailing " " (1)
502-
const maxLabelChars = Math.max(4, termWidth - SECTION_FIXED);
506+
// Use Math.max(0, …) — not Math.max(4, …) — so that on very narrow
507+
// terminals the label budget never exceeds the actual available width
508+
// and forces a line wider than termWidth. When the budget is 0 or 1
509+
// we skip rendering the section entirely to avoid wrapping.
510+
const maxLabelChars = Math.max(0, termWidth - SECTION_FIXED);
511+
if (maxLabelChars === 0) {
512+
usedLines += sectionCost;
513+
if (usedLines >= viewportHeight) break;
514+
continue;
515+
}
503516
const label =
504517
row.sectionLabel.length > maxLabelChars
505518
? row.sectionLabel.slice(0, maxLabelChars - 1) + "…"
@@ -571,7 +584,11 @@ export function renderGroups(
571584
const prefixWidth = isCursor
572585
? ACTIVE_BAR_WIDTH + INDENT.length + 1 + 1 // bar + " " + checkbox + space = 5
573586
: INDENT.length * 2 + 1 + 1; // " " + " " + checkbox + space = 6
574-
const maxPathVisible = Math.max(10, termWidth - prefixWidth - locSuffix.length);
587+
// Use Math.max(1, …) so that on very narrow terminals the floor of 1
588+
// never exceeds the available width (unlike Math.max(10, …) which can
589+
// produce a maxPathVisible wider than termWidth - prefixWidth,
590+
// reintroducing line wrapping — see review on #106).
591+
const maxPathVisible = Math.max(1, termWidth - prefixWidth - locSuffix.length);
575592
const rawPath = isCursor
576593
? `${highlightText(match.path, "path", (s) => pc.bold(pc.white(s)))}${styledLocSuffix}`
577594
: `${highlightText(match.path, "path", pc.cyan)}${styledLocSuffix}`;

src/tui.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,17 @@ export async function runInteractive(
197197
// list, the rows visible from scrollOffset onwards can be fewer than
198198
// viewportHeight — leaving blank space above the footer even though rows
199199
// above scrollOffset could fill it. See issue #105.
200-
scrollOffset = normalizeScrollOffset(scrollOffset, rows, groups, getViewportHeight(rows));
200+
//
201+
// Iterate to a fixed point: getViewportHeight depends on scrollOffset (via
202+
// the sticky-header condition). Decreasing scrollOffset can change whether
203+
// the sticky header is shown, which changes viewportHeight by 1, so a
204+
// single pass can stop 1 row early. Loop until both values stabilise.
205+
for (;;) {
206+
const vh = getViewportHeight(rows);
207+
const next = normalizeScrollOffset(scrollOffset, rows, groups, vh);
208+
if (next === scrollOffset) break;
209+
scrollOffset = next;
210+
}
201211
const rendered = renderGroups(groups, cursor, rows, termHeight, scrollOffset, query, org, {
202212
filterPath,
203213
filterMode,

0 commit comments

Comments
 (0)