Skip to content

Add ICU sentence segmentation to OffsetsTextInfo - #20603

Open
LeonarddeR wants to merge 4 commits into
nvaccess:masterfrom
LeonarddeR:sentenceSegIcu
Open

Add ICU sentence segmentation to OffsetsTextInfo#20603
LeonarddeR wants to merge 4 commits into
nvaccess:masterfrom
LeonarddeR:sentenceSegIcu

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

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 _getSentenceOffsets raised NotImplementedError. 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.icu gains UBRK.SENTENCE. textUtils.icu gains calculateSentenceOffsets. OffsetsTextInfo._getSentenceOffsets is implemented, so UNIT_SENTENCE becomes 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/expand tiling invariant requires. When ICU is unavailable (Windows before version 1703), the method keeps raising NotImplementedError.

Testing strategy:

New unit tests in test_sentenceSegIcu.py cover the calculateSentenceOffsets primitive (English, Japanese, abbreviation splits, surrogate pairs, containment) and the OffsetsTextInfo integration, 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.sentenceHelper still uses its own logic to detect sentence boundaries.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

Copilot AI 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 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 new textUtils.icu.calculateSentenceOffsets primitive.
  • Implement OffsetsTextInfo._getSentenceOffsets using 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.

Comment on lines +13 to +14
skipIfNoICU = unittest.skipUnless(ICU_AVAILABLE, "ICU library not available on this system")
"""Decorator skipping tests that need the Windows built-in ICU library."""
Copilot AI review requested due to automatic review settings August 3, 2026 06:29

Copilot AI 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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 3, 2026 07:31
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Closing until #20602 is merged first

@LeonarddeR LeonarddeR closed this Aug 3, 2026

Copilot AI 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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

LeonarddeR and others added 2 commits August 8, 2026 09:39
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 LeonarddeR reopened this Aug 8, 2026
@LeonarddeR
LeonarddeR marked this pull request as ready for review August 10, 2026 07:06
@LeonarddeR
LeonarddeR requested a review from a team as a code owner August 10, 2026 07:06
@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants