Skip to content

fix: preserve Unicode code points at acronym boundaries - #144

Closed
xiehuanyi wants to merge 1 commit into
unjs:mainfrom
xiehuanyi:fix/unicode-acronym-boundaries
Closed

xiehuanyi wants to merge 1 commit into
unjs:mainfrom
xiehuanyi:fix/unicode-acronym-boundaries

Conversation

@xiehuanyi

@xiehuanyi xiehuanyi commented Oct 6, 2026 •

Copy link
Copy Markdown

Description

splitByCase("𐐀foo") splits an astral uppercase letter into invalid surrogate halves. Match the loop's Unicode code-point iteration when moving the last acronym character into the next word. A single uppercase code point followed by lowercase text remains intact.

Resolves #143.

Checks

All four new regressions fail on unchanged main. All 79 tests pass, with no type errors, 100% statement/function/line coverage, lint, formatting, build, and git diff --check passing. Verification completed in under one minute on Node 24.19.0.

AI-assisted; source and results reviewed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved case-based text splitting for Unicode characters, keeping supplementary characters intact when they appear next to lowercase letters.

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 was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d12358e6-c482-46d2-9c1f-128e0b8f1a70
📥 Commits

Reviewing files that changed from the base of the PR and between 5bc6804 and e6c8f7e.

📒 Files selected for processing (2)
  • src/index.ts
  • test/scule.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

splitByCase now identifies the final character of an uppercase run by Unicode code point. New tests check that astral characters remain intact when the function splits text by case.

Changes

Unicode-safe case splitting

Layer / File(s) Summary
Preserve Unicode code points at case boundaries
src/index.ts, test/scule.test.ts
splitByCase now checks the final code point’s UTF-16 length before splitting. Tests cover astral characters at the start, repeated at the start, after an ASCII uppercase letter, and after lowercase text.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to e6c8f

Astral characters remain intact at the covered case boundaries while existing BMP acronym behavior is preserved. No merge-blocking risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to e6c8f

The change affects 2 systems.

Changed systems: src, test

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — test (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/index.ts: splitByCase now obtains the final Unicode code point of the buffer and compares its UTF-16 length with the buffer length before splitting off that character. This replaces the prior buff.length > 1 check and one-code-unit extraction/slicing.
  • observed — Modified behavior in test/scule.test.ts: Added splitByCase cases for astral characters at the start, repeated at the start, after an ASCII uppercase letter, and after lowercase text; each expects complete Unicode code points in the resulting parts.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: preserving Unicode code points at acronym boundaries.
Linked Issues check ✅ Passed Issue #143 requires splitByCase to preserve astral uppercase code points at acronym boundaries. src/index.ts uses the final code point from [...buff] and compares its UTF-16 length before slicin…
Out of Scope Changes check ✅ Passed The reported source change and added tests address issue #143. No unrelated changes are identified in the available change summary or reviewed files.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/index.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

test/scule.test.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pi0 pi0 closed this Oct 7, 2026
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.

splitByCase cuts astral uppercase characters into invalid surrogate halves

3 participants