Skip to content

Fix another bug in diffWords's "intlSegmenter" mode - #667

Merged
ExplodingCabbage merged 6 commits into
masterfrom
fix-another-segmenter-bug
Feb 18, 2026
Merged

Fix another bug in diffWords's "intlSegmenter" mode#667
ExplodingCabbage merged 6 commits into
masterfrom
fix-another-segmenter-bug

Conversation

@ExplodingCabbage

Copy link
Copy Markdown
Collaborator

Fixes#664

@ExplodingCabbage

Copy link
Copy Markdown
CollaboratorAuthor

Okay, I think this is good to go. Might as well get an AI review, I guess!

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a crash in diffWords when used with intlSegmenter and text containing combining marks after whitespace characters (issue #664). The root cause was that Intl.Segmenter treats sequences like " \u0300" (space + combining grave accent) as a single non-word segment, causing whitespace deduplication logic to fail.

Changes:

  • Added segment() function to merge consecutive whitespace segments from Intl.Segmenter
  • Updated leadingWs(), trailingWs(), and added leadingAndTrailingWs() to handle segmenter-based whitespace detection
  • Threaded segmenter parameter through whitespace deduplication logic in diffWords

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
test/util/string.jsAdded tests for leadingWs and trailingWs with segmenter mode
test/diff/word.jsAdded regression tests for combining marks on whitespace; refactored existing test
src/util/string.tsImplemented segment(), leadingAndTrailingWs(), and updated leadingWs()/trailingWs() to support segmenter
src/diff/word.tsUpdated dedupeWhitespaceInChangeObjects() to pass segmenter parameter through; uses new segment() function
release-notes.mdDocumented the fix for version 8.0.4

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadtest/diff/word.js Outdated
]);
];
expect(wordDiff.tokenize(string)).to.deep.equal(expectedResult);
expect(wordDiff.tokenize(string, new Intl.Segmenter({granularity: 'word'}))).to.deep.equal(expectedResult);

CopilotAIFeb 18, 2026

Copy link

Choose a reason for hiding this comment

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

The second argument to wordDiff.tokenize() should be an options object with an intlSegmenter property, not a segmenter directly. This test is passing a segmenter as the second argument directly, which would be assigned to the options parameter but doesn't match the expected structure. The call should be wordDiff.tokenize(string, {intlSegmenter: new Intl.Segmenter({granularity: 'word'})}).

Suggested change
expect(wordDiff.tokenize(string,newIntl.Segmenter({granularity: 'word'}))).to.deep.equal(expectedResult);
expect(
wordDiff.tokenize(string,{
intlSegmenter: newIntl.Segmenter({granularity: 'word'})
})
).to.deep.equal(expectedResult);

Copilot uses AI. Check for mistakes.
@ExplodingCabbage

Copy link
Copy Markdown
CollaboratorAuthor

Good catch, AI friend!

@ExplodingCabbage
ExplodingCabbage merged commit 4b5d180 into masterFeb 18, 2026
@ExplodingCabbage
ExplodingCabbage deleted the fix-another-segmenter-bug branch February 18, 2026 16:43
Sign up for freeto 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.

diffWords crashes with intlSegmenter when text contains orphaned combining marks

2 participants

@ExplodingCabbage