Fix double-width characters disappearing when wrapping #3180
Conversation
This file contains 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
…'s referring to the number of *single-width* characters. Also a small addition to the gitignore file.
…hen the "fold" location sat *within* a double-width character. Ensure we retain browser logic of: if there is no space on the current line, move to a new line, and if theres not enough space on the entire new line, fold the text over multiple lines at appropriate locations.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3180 +/- ##
==========================================
- Coverage 98.30% 98.28% -0.03%
==========================================
Files 74 74
Lines 8038 8049 +11
==========================================
+ Hits 7902 7911 +9
- Misses 136 138 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
rodrigogiraoserrao
approved these changes
Nov 2, 2023
| divides: List[int] = [] | ||
| append = divides.append | ||
| line_position = 0 | ||
| def divide_line(text: str, width: int, fold: bool = True) -> list[int]: |
There was a problem hiding this comment.
I'm assuming cell_offset is keeping track of how much space is already occupied in the line we're currently filling, right?
I personally don't love the name but maybe that could be mitigated with a short comment next to it?
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
…ch into double-width-wrapping-fix
willmcgugan
approved these changes
Nov 14, 2023
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.
Type of changes
Checklist
Description
Update wrapping logic to fix issues with CJK charcters disappearing when the "fold" location sat within a double-width character. Ensure we retain browser logic of:
Adds some additional tests and docstrings, documentation etc.
Fixes #3176
The wrapping process is overall still quite simple and doesn't match the browser in many cases. For example, wrapping does not consider punctuation (lines can begin with punctuation), and whitespace is handled differently (but practically speaking it seems sensible).