fix for infinite loop in split_graphemes#4006
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4006 +/- ##
=======================================
Coverage 97.87% 97.87%
=======================================
Files 96 96
Lines 8366 8370 +4
=======================================
+ Hits 8188 8192 +4
Misses 178 178
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@jmw182 Feel free to kick the tires. |
There was a problem hiding this comment.
Pull request overview
Fixes a hang in Rich’s terminal cell measurement logic (rich.cells.split_graphemes) when encountering zero-width characters such as ANSI escape codes, addressing #3958.
Changes:
- Updated
split_graphemesto ensure the loop advances for certain zero-width/special codepoints instead of stalling. - Added regression tests for zero-width inputs in
chop_cells/split_graphemes. - Documented the fix in
CHANGELOG.mdunder “Unreleased”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
rich/cells.py |
Adjusts grapheme splitting logic to avoid non-advancing index scenarios for zero-width/special characters. |
tests/test_cells.py |
Adds regression coverage for zero-width inputs (and updates expected spans/cell lengths). |
CHANGELOG.md |
Notes the infinite-loop fix in the Unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| total_width = 0 | ||
| spans: list[tuple[int, int, int]] = [] | ||
| SPECIAL = {"\u200d", "\ufe0f"} |
There was a problem hiding this comment.
The new if not spans handling for SPECIAL chars avoids hangs when the string starts with VS16/ZWJ, but split_graphemes can still hang on "\ufe0f" when spans is non-empty and last_measured_character is None (e.g. "\x1b\ufe0f" after the new zero-width span logic). In that case the VS16 branch doesn't advance index. Make sure VS16 always increments index (and extends/creates a span) even when there is no prior measured character.
| ( | ||
| "\u200d\u200d", | ||
| [(0, 2, 0)], | ||
| 0, |
There was a problem hiding this comment.
Remove the print(spans) debug output from this test; it will add noise to test runs/CI output and isn't needed for assertions.
There was a problem hiding this comment.
The print is removed by pytest if the test passes.
| 0, | ||
| ), # Variation selector 16, without anything to change should have zero width | ||
| ( | ||
| "\ufe0f\ufe0f", |
There was a problem hiding this comment.
Typo in comment: "within noting prior" → "with nothing prior".
|
@willmcgugan I've opened a new pull request, #4007, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Fix for #3958