Repository navigation
fix(util): prevent slice(-0) leak in truncateMiddle on small maxLength - #55953
smartworldarafath wants to merge 2 commits into
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The fix to truncateMiddle in packages/util/src/path.ts is correct. I ran it against the base and the PR: with the PR, maxLength 1 and 2 now return "…" and "h…" instead of the whole string, and longer lengths are unchanged. The util tests and typecheck pass. The public API is unchanged: the exported signature is the same and only the output for maxLength <= 2 changes, which is backwards compatible.
One thing worth addressing: nothing in the repo calls this util function. The truncation users actually see comes from the separate Locale.truncateMiddle in packages/tui/src/util/locale.ts, which has exactly the same slice(-0) bug and still returns "…hello world" for maxLength 1 with this PR. That copy is called from dialog-select.tsx with Math.max(1, width()), the autocomplete and the prompt. Could you fix it there too, ideally by having the TUI reuse the @opencode/util function instead of keeping a second copy? Otherwise this change has no effect users can see.
Related: #44338
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Issue for this PR
Closes #44338
Type of change
What does this PR do?
When
maxLength <= 2intruncateMiddle,Math.floor((maxLength - 1) / 2)evaluates to0.In JavaScript,
slice(-0)is evaluated asslice(0), returning the entire string.For example,
truncateMiddle("some-long-name", 1)returned"…some-long-name"instead of"…".This PR fixes this by:
maxLength <= 1explicitly with an early return"…".slice(-end)is only called whenend > 0, avoidingslice(-0).@opencode/util/path'struncateMiddlein@opencode/tui/src/util/locale.tsso the TUI uses the shared, fixed implementation.How did you verify your code works?
packages/util/src/path.test.tsverifyingmaxLength1, 2, and 3 return"…","a…", and"a…p".packages/tui/test/util/locale.test.tsverifyingLocale.truncateMiddlecorrectly truncates small lengths.packages/util/src/path.test.tspass cleanly.Screenshots / recordings
N/A (text truncation helper)
Checklist