Skip to content

fix: avoid splitting surrogate pairs in trimEnd - #2423

Merged
Dmitriy Vasyura (dmitrivMS) merged 1 commit into
microsoft:mainfrom
kwy404:fix/trim-end-surrogate-pair
Oct 5, 2026
Merged

Dmitriy Vasyura (dmitrivMS) merged 1 commit into
microsoft:mainfrom
kwy404:fix/trim-end-surrogate-pair

Conversation

@kwy404

Copy link
Copy Markdown
Contributor

Root cause: trimEnd in src/common/stringUtils.ts cuts the string at maxLength - 1 UTF-16 code units. When that cut lands between the two halves of a surrogate pair (an emoji or any other astral character), the result keeps a lone high surrogate before the ellipsis. Truncated object previews, keys, descriptions and console.table rows then show a replacement character. For example trimEnd('ab\u{1F600}cd', 4) returns 'ab\uD83D…'. trimMiddle, right below it, already guards against this case, but trimEnd did not.

Fix: if the last kept code unit starts a surrogate pair, drop it too, using the same codePointAt(...) >= 0x10000 check that trimMiddle uses.

Test: added trimEnd does not split surrogate pairs to src/common/stringUtils.test.ts. It fails before the change (expected 'ab\uD83D…' to equal 'ab…') and passes after. The rest of the unit suite is unchanged.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation matches the established trimMiddle approach and the regression test covers both boundary cases.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes trimEnd so truncation does not split UTF-16 surrogate pairs.

Changes:

  • Adjusts truncation when the boundary intersects an astral character.
  • Adds regression coverage for safe and unsafe truncation boundaries.
File Description
src/​common/​stringUtils.ts Prevents retaining a lone high surrogate.
src/​common/​stringUtils.test.ts Tests emoji truncation boundaries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@connor4312 Connor Peet (connor4312) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit 7980b0d into microsoft:main Oct 5, 2026
6 checks passed
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.

5 participants