Skip to content

Use StringComparison.Ordinal when locating token indices in PlaceMarkersUsfmUpdateBlockHandler - #496

Merged
Enkidu93 merged 1 commit into
masterfrom
fix-marker-placement-error-with-combining-character-token
Sep 8, 2026
Merged

Use StringComparison.Ordinal when locating token indices in PlaceMarkersUsfmUpdateBlockHandler#496
Enkidu93 merged 1 commit into
masterfrom
fix-marker-placement-error-with-combining-character-token

Conversation

@Enkidu93

@Enkidu93 Enkidu93 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #492.

I believe that using Ordinal is safe since the tokenizer doesn't do any normalization of combining characters I don't think.


This change is Reviewable

@pmachapman pmachapman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@pmachapman reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ddaspit).

@ddaspit ddaspit 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.

:lgtm:

@ddaspit reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Enkidu93).

@Enkidu93
Enkidu93 force-pushed the fix-marker-placement-error-with-combining-character-token branch from 6be3bab to b98b21d Compare September 8, 2026 21:16
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (ae09770) to head (b98b21d).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #496      +/-   ##
==========================================
+ Coverage   73.51%   73.52%   +0.01%     
==========================================
  Files         451      451              
  Lines       37692    37697       +5     
  Branches     5183     5183              
==========================================
+ Hits        27708    27718      +10     
+ Misses       8845     8841       -4     
+ Partials     1139     1138       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Enkidu93
Enkidu93 merged commit 418ff22 into master Sep 8, 2026
3 of 4 checks passed
@Enkidu93
Enkidu93 deleted the fix-marker-placement-error-with-combining-character-token branch September 8, 2026 21:29
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.

Exception using PlaceMarkersUsfmUpdateBlockHandler with Devanagari

4 participants