Removed Text.from_ansi() monkeypatch (3.x)#1631
Conversation
|
🤖 Hi @kmvanbrunt, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.x #1631 +/- ##
======================================
Coverage ? 99.21%
======================================
Files ? 21
Lines ? 4862
Branches ? 0
======================================
Hits ? 4824
Misses ? 38
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This Pull Request successfully removes the Text.from_ansi() monkey patch from cmd2/rich_utils.py, following the fix in the upstream rich library. By increasing the minimum rich version to 15.0.0, the workaround is no longer necessary, leading to a cleaner codebase.
🔍 General Feedback
- Regression Testing: While the monkey patch itself is obsolete, it is recommended to retain the functional part of the
Text.from_ansitest to ensure the library's behavior remains consistent withcmd2's requirements across future updates. - Changelog: Consider adding an entry to
CHANGELOG.mdto document therichversion bump and the removal of the monkey patch.
| assert ru.APP_THEME.styles[rich_style_key] == theme[rich_style_key] | ||
|
|
||
|
|
||
| def test_from_ansi_wrapper() -> None: |
There was a problem hiding this comment.
🟢 While the monkey patch is being removed, it is beneficial to keep a simplified version of this test as a regression check. This ensures that the required version of the rich library (>= 15.0.0) continues to provide the behavior cmd2 expects regarding trailing newlines.
| def test_from_ansi_wrapper() -> None: | |
| def test_text_from_ansi() -> None: | |
| # Line breaks recognized by str.splitlines(). | |
| # Source: https://docs.python.org/3/library/stdtypes.html#str.splitlines | |
| line_breaks = { | |
| "\n", # Line Feed | |
| "\r", # Carriage Return | |
| "\r\n", # Carriage Return + Line Feed | |
| "\v", # Vertical Tab | |
| "\f", # Form Feed | |
| "\x1c", # File Separator | |
| "\x1d", # Group Separator | |
| "\x1e", # Record Separator | |
| "\x85", # Next Line (NEL) | |
| "\u2028", # Line Separator | |
| "\u2029", # Paragraph Separator | |
| } | |
| # Test all line breaks | |
| for lb in line_breaks: | |
| input_string = f"Text{lb}" | |
| expected_output = input_string.replace(lb, "\n") | |
| assert Text.from_ansi(input_string).plain == expected_output | |
| # Test string without trailing line break | |
| input_string = "No trailing\nline break" | |
| assert Text.from_ansi(input_string).plain == input_string | |
| # Test empty string | |
| input_string = "" | |
| assert Text.from_ansi(input_string).plain == input_string |
aeac8a9 to
3648426
Compare
No description provided.