Skip to content

fix(tui): truncate helpers can return a longer string than they got - #41060

Open
Mute-404 wants to merge 1 commit into
anomalyco:devfrom
Mute-404:truncate-clamp
Open

fix(tui): truncate helpers can return a longer string than they got#41060
Mute-404 wants to merge 1 commit into
anomalyco:devfrom
Mute-404:truncate-clamp

Conversation

@Mute-404

@Mute-404 Mute-404 commented Aug 7, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #37458

(I opened #41054 before finding #37458. Closing mine as a duplicate — this PR targets the existing one. Same bug was also reported in #20880 and #34395, and a fix in #34482 was closed by the automated PR cleanup in July rather than reviewed.)

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

truncateLeft and truncateMiddle in packages/tui/src/util/locale.ts compute a slice length that reaches zero or goes negative on small budgets. String.prototype.slice(-0) is slice(0), which returns the whole string, so both helpers hand back something longer than the input:

truncateMiddle("abcdefghij", 0) -> "…bcdefghij"
truncateMiddle("abcdefghij", 1) -> "…abcdefghij"
truncateLeft("abcdefghij", 1)   -> "…abcdefghij"

They grow the text exactly when the terminal has the least room for it.

Clamped inside both helpers so callers don't each need their own guard. truncateMiddle also got restructured slightly: keepEnd is now checked before slicing, which is what removes the slice(-0) path.

Worth noting for review: the Math.max(1, ...) at dialog-select.tsx:702 and dialog-move-session.tsx:151 looks like a guard against this, but 1 is inside the broken range, so it clamps straight into it. files.tsx:35 uses Math.max(2, ...) and happens to land just outside. After this change those clamps are redundant but harmless, so I left them alone to keep the diff focused.

How did you verify your code works?

Ran the actual call site expressions from the TUI against the real module, on dev and on this branch:

term | callsite                     | budget | before | after
  10 | dialog-select.tsx:702        |      1 |     40 |     1
  10 | dialog-move-session.tsx:151  |      1 |     40 |     1
  12 | dialog-select.tsx:702        |      1 |     40 |     1
  14 | dialog-select.tsx:702        |      2 |     41 |     2
  16 | dialog-move-session.tsx:151  |      2 |     41 |     2

9 call sites returned more than their budget before, 0 after. Output stays correct for normal widths (truncateMiddle("...", 5) gives "ab…ij").

$ bun test test/util-locale.test.ts
 4 pass
 0 fail
 20 expect() calls

util/locale.ts had no tests, which is presumably how this survived three reports. The tests assert the budget is never exceeded across the whole small range rather than checking specific strings.

Two things I deliberately left out:

  • packages/core/src/util/path.ts:31 has a byte-identical copy of the same bug. Different package, seemed like it should be its own PR — happy to include it here if you'd prefer.
  • These helpers budget in str.length (UTF-16 code units) while callers are budgeting terminal columns, so CJK and emoji still under-truncate. packages/tui/src/prompt/display.ts:7 shows the house approach with Bun.stringWidth + Intl.Segmenter. That's a behavior change and needs its own discussion.

Screenshots / recordings

This only shows up below roughly 16 columns, where the layout is already degraded. The table above is the measurable version.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Both helpers compute a slice length that goes to zero or negative on
small budgets, and slice(-0) hands back the whole string. So they grow
the text right when the terminal has the least room for it:

  truncateMiddle("abcdefghij", 0) === "…bcdefghij"
  truncateLeft("abcdefghij", 1)   === "…abcdefghij"

At 10 columns dialog-select.tsx:702 asks for 1 char and gets 40 back.
Same for dialog-move-session.tsx:151. The Math.max(1, ...) at those call
sites reads like a guard but clamps straight into the broken range.

Clamped inside the helpers so callers don't each need their own guard.
Adds tests for util/locale, which had none.
@github-actions github-actions Bot added needs:compliance This means the issue will auto-close after 2 hours. and removed needs:compliance This means the issue will auto-close after 2 hours. labels Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

truncateMiddle returns a string longer than maxLength when maxLength is 1 or 2

1 participant