Add ICU sentence segmentation to OffsetsTextInfo - #20603
Open
LeonarddeR wants to merge 8 commits into
Open
Conversation
Closed
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds ICU (Windows built-in) sentence segmentation support for OffsetsTextInfo, enabling UNIT_SENTENCE navigation for all TextInfos derived from offsets-based implementations (e.g. virtual buffers), while preserving the existing NotImplementedError behavior when ICU is unavailable.
Changes:
- Add ICU sentence break support (
UBRK.SENTENCE) and a newtextUtils.icu.calculateSentenceOffsetsprimitive. - Implement
OffsetsTextInfo._getSentenceOffsetsusing ICU sentence segmentation over paragraph text and rebase to document offsets. - Factor out shared ICU offset/encoding conversion via
textUtils.icu.calculateOffsetsForEncoding, and update the ICU word segmentation strategy to use it.
Reviewed changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_textUtils/test_wordSegIcu.py | Adds focused unit tests for IcuWordSegmentationStrategy selection and primitive invocation. |
| tests/unit/test_textUtils/test_wordSeg.py | Adds broader unit tests for word segmentation initialization and related utilities. |
| tests/unit/test_textUtils/test_sentenceSegIcu.py | Adds unit tests for ICU sentence segmentation primitive and OffsetsTextInfo tiling/iteration behavior. |
| tests/unit/test_textUtils/test_offsetConverters.py | Adds/extends unit tests validating offset converters (incl. surrogate-pair behavior). |
| tests/unit/test_textUtils/test_hyphenation.py | Adds unit tests for textUtils.hyphenation behavior for known/unknown languages. |
| tests/unit/test_textUtils/test_backendComparison.py | Switches ICU-availability skipping to shared skipIfNoICU. |
| tests/unit/test_textUtils/init.py | Introduces skipIfNoICU helper for ICU-dependent tests. |
| source/winBindings/icu.py | Adds UBRK.SENTENCE enum member for ICU sentence break iteration. |
| source/textUtils/icu.py | Adds sentence segmentation primitive and shared offset conversion helper; refactors word segmentation to reuse _containingSegment. |
| source/textUtils/_wordSeg/wordSegStrategy.py | Updates ICU word segmentation strategy to use the shared encoding conversion helper. |
| source/textInfos/offsets.py | Implements _getSentenceOffsets using ICU sentence segmentation over paragraph scope. |
LeonarddeR
force-pushed
the
sentenceSegIcu
branch
from
August 3, 2026 06:29
9c9e266 to
f7d15cc
Compare
Collaborator
Author
|
Closing until #20602 is merged first |
textUtils.icu gains calculateSentenceOffsets using the SENTENCE break iterator. OffsetsTextInfo._getSentenceOffsets segments the containing paragraph with it instead of raising NotImplementedError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fsets OffsetsTextInfo._getSentenceOffsets now applies the same encoding guard as _getWordOffsets and _getCharacterOffsets, accepting only UTF-16 and the encodings whose offsets are str indices. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LeonarddeR
force-pushed
the
sentenceSegIcu
branch
from
August 8, 2026 07:43
4173689 to
67fef60
Compare
LeonarddeR
marked this pull request as ready for review
August 10, 2026 07:06
SaschaCowley
added
the
conceptApproved
SaschaCowley
requested changes
Aug 21, 2026
SaschaCowley
left a comment
SaschaCowley
left a comment
Member
There was a problem hiding this comment.
Mostly looks good, just a few minor suggestions.
SaschaCowley
marked this pull request as draft
August 21, 2026 04:11
# Conflicts: # user_docs/en/changes.md
…st, rewrite the encoding check as a membership test, reword docstrings and the surrogate pair test comment
LeonarddeR
marked this pull request as ready for review
August 21, 2026 06:52
SaschaCowley
enabled auto-merge (squash)
August 21, 2026 07:45
Member
|
The system tests need to updated to adapt to the new behaviour |
LeonarddeR
marked this pull request as draft
August 25, 2026 06:43
auto-merge was automatically disabled
August 25, 2026 06:43
Pull request was converted to draft
Collaborator
Author
|
O wow, I couldn't have imagined this. Of course now sentense offsets work in browse mode, say all behaves somewhat differently now. |
…d document sentence support in browse mode Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
@seanbudd the ARIA description Say All test failing earlier now passes. The only failure is one test on win 2025 that succeeds on 2022. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Part of #18901.
Follow up for #20602.
Summary of the issue:
Sentence navigation only works in Microsoft Word and Outlook, which have native sentence support. Browse mode in virtual buffers uses TextInfos based on
OffsetsTextInfo, and its_getSentenceOffsetsraisedNotImplementedError. Working sentence navigation in those documents therefore needs a sentence backend first.Description of user facing changes:
No direct changes. This PR is the necessary building block: programmatic sentence navigation is now possible in cursor managers and in TextInfos based on offsets. The next PR in the stack routes the browse mode sentence gestures to it.
Description of developer facing changes:
winBindings.icugainsUBRK.SENTENCE.textUtils.icugainscalculateSentenceOffsets.OffsetsTextInfo._getSentenceOffsetsis implemented, soUNIT_SENTENCEbecomes available on all TextInfos derived from it.Description of development approach:
Sentence boundaries come from the Windows built-in ICU BreakIterator using UAX#29 default rules. Segmentation runs over the containing paragraph, so sentences can span lines. Results are rebased by the paragraph start offset. The root locale is used: UAX#29 sentence boundaries are driven by the language-neutral Sentence_Break property, and locale only adds abbreviation tailoring. A fixed locale also keeps boundaries a deterministic function of position, which the
move/expandtiling invariant requires. When ICU is unavailable (Windows before version 1703), the method keeps raisingNotImplementedError.Testing strategy:
New unit tests in
test_sentenceSegIcu.pycover thecalculateSentenceOffsetsprimitive (English, Japanese, abbreviation splits, surrogate pairs, containment) and theOffsetsTextInfointegration, including forward and backward tiling across a block-paragraph fixture.Known issues with pull request:
Abbreviations such as "Dr." end a sentence. Locale-specific tailoring could address this later.
documentNavigation.sentenceHelperstill uses its own logic to detect sentence boundaries.Code Review Checklist: