Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork252
Add width awareness to fix wrapping of strings with full width chars#839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
sebastinas commentedAug 14, 2020
Could you please rebase this PR? |
rybarczykj commentedAug 17, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Just noticed a bug with this; maybe don't accept yet.
|
rybarczykj commentedAug 18, 2020
That bug I mentioned ^^ is fixed now, as of that last push. Now, if wcswidth throws an error, we catch it and use the old display_linize. I also added some relevant tests. |
- update display_linize to use curtsies 3.0 width functionality- correct cursor positioning on wrapped fullwidth chars by adding new method- add test coverage
This is a redux of#731 which was put on hold until a need for width awareness was found. I'd say this is a decent justification for bringing it back
Wrapping strings with fullwidth chars

Here's the same string being typed before and after width awareness:
New method to fix cursor position:
When we put a 2-column character at the end of the line where there's only 1 column of room, curtsies adds a padding character (space) so we can kick it down to the line below. Therefore, in order to get the right cursor position, I had to add a method that determines how many padding chars there are (
number_of_padding_chars_on_current_cursor_line).Note on this method: Right now, to determine the amount of padding, it calls
display_linize, doubling the amount of times we call that function. It's definitely possible to keep track of this as a variable in the repl class instead, or to do some changes other to ensure less redundant work is done. Feedback appreciated.Runtime
One of the barriers stopping us from implementing this before was runtime . I did not find any noticeable difference when pasting, printing, or moving my cursor around large strings.