Skip to content

Commit 557c7f4

Browse files
committed
Address review: fix clipAnsi docstring/surrogates, section blank sync, repo row floor
Four follow-up fixes from code review on PR #106: 1. clipAnsi docstring: updated to document the partial SGR reset (\x1b[22;39m) and explain why the background is deliberately preserved. Added note about code-point iteration to match the new implementation. 2. clipAnsi surrogate pairs: the loop now advances i by codePointAt() width (2 UTF-16 units for non-BMP code points, 1 otherwise) instead of always i++. This prevents clipping in the middle of a surrogate pair when maxVisible falls between the two halves of an emoji (e.g. the 🔍 prefix used in filter lines). 3. Section row maxLabelChars === 0: instead of incrementing usedLines without pushing to lines[], the code now pushes blank placeholder strings (same count as sectionCost) to keep lines[] in sync with usedLines. Without this, the footer-padding loop adds too few blank lines and the output is sectionCost lines shorter than termHeight. 4. Repo row maxLeftVisible floor: Math.max(4, …) → Math.max(1, …). The old floor of 4 could exceed the available width on very narrow terminals (termWidth < 4 + countLen + barAdjust), reintroducing wrapping.
1 parent 68cae8a commit 557c7f4

1 file changed

Lines changed: 27 additions & 12 deletions

File tree

src/render.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,15 @@ function stripAnsi(str: string): string {
203203
/**
204204
* Clip a string (which may contain ANSI escape sequences) to at most
205205
* `maxVisible` visible characters. Correctly skips over escape sequences
206-
* when counting, and appends a SGR reset (`\x1b[0m`) at the cut point so
207-
* the terminal is left in a clean state.
206+
* when counting, and iterates by Unicode code point (not UTF-16 code unit)
207+
* so clipping never splits a surrogate pair (e.g. emoji like 🔍).
208+
*
209+
* At the cut point a partial SGR reset (`\x1b[22;39m`) is appended to
210+
* clear bold and foreground colour while deliberately **leaving background
211+
* colour intact** (no `\x1b[49m`). A full `\x1b[0m` reset would undo any
212+
* background applied by the caller (e.g. renderActiveLine's dark-purple
213+
* highlight), causing the remainder of the active row to lose its colour
214+
* on narrow terminals — see issue #105.
208215
*
209216
* If the string already fits within `maxVisible` visible chars it is
210217
* returned unchanged.
@@ -224,16 +231,17 @@ function clipAnsi(str: string, maxVisible: number): string {
224231
i = j + 1; // skip the terminating letter too
225232
continue;
226233
}
234+
// Advance by code-point width (2 UTF-16 units for non-BMP chars like emoji)
235+
// so we never split a surrogate pair at a cut boundary.
236+
const cp = str.codePointAt(i)!;
237+
const cpLen = cp > 0xffff ? 2 : 1;
227238
visCount++;
228239
if (visCount === maxVisible) {
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";
240+
// Cut after this visible char. Use a partial SGR reset (bold + fg only)
241+
// so that the caller's background colour is preserved.
242+
return str.slice(0, i + cpLen) + "\x1b[22;39m";
235243
}
236-
i++;
244+
i += cpLen;
237245
}
238246
return str;
239247
}
@@ -509,6 +517,11 @@ export function renderGroups(
509517
// we skip rendering the section entirely to avoid wrapping.
510518
const maxLabelChars = Math.max(0, termWidth - SECTION_FIXED);
511519
if (maxLabelChars === 0) {
520+
// Terminal too narrow to render even a minimal label. Push blank
521+
// placeholder lines so that lines[] stays in sync with usedLines
522+
// and the footer-padding arithmetic remains correct — see review #106.
523+
if (usedLines > 0) lines.push(""); // blank separator when not first
524+
lines.push(""); // empty label placeholder
512525
usedLines += sectionCost;
513526
if (usedLines >= viewportHeight) break;
514527
continue;
@@ -554,9 +567,11 @@ export function renderGroups(
554567
const leftPartRaw = `${arrow} ${checkbox} ${repoName}`;
555568
const countLen = stripAnsi(count).length;
556569
const barAdjust = isCursor ? ACTIVE_BAR_WIDTH : 0;
557-
// Fix: ensure leftPart + count fits within termWidth — clip repoName if needed.
558-
// The maximum visible width available for leftPart: termWidth - countLen - barAdjust.
559-
const maxLeftVisible = Math.max(4, termWidth - countLen - barAdjust);
570+
// Use Math.max(1, …) so that on very narrow terminals the floor of 1
571+
// never exceeds the available width (unlike Math.max(4, …) which can
572+
// produce a maxLeftVisible wider than the actual space and reintroduce
573+
// wrapping — see review on #106).
574+
const maxLeftVisible = Math.max(1, termWidth - countLen - barAdjust);
560575
const leftPart =
561576
stripAnsi(leftPartRaw).length > maxLeftVisible
562577
? clipAnsi(leftPartRaw, maxLeftVisible)

0 commit comments

Comments
 (0)