Skip to content

gh-156207: Fix curses Textbox.gather() for double-width characters - #156208

Open
fedonman wants to merge 1 commit into
python:mainfrom
fedonman:fix-textpad-gather-wide
Open

gh-156207: Fix curses Textbox.gather() for double-width characters#156208
fedonman wants to merge 1 commit into
python:mainfrom
fedonman:fix-textpad-gather-wide

Conversation

@fedonman

Copy link
Copy Markdown
Contributor

gather() now reads each line with in_wchstr(), whose count is a cell count and which skips a double-width character's continuation cell, instead of reading one cell at a time and reporting that second cell as another copy of the character.

This fixes only the gather() symptom of gh-156207. The insert-mode corruption and the curses.error at the last column are untouched: both need _insert_printable_char() to know a character's display width, which is a larger change.

$ ./python -m test test_curses -u curses
Using random seed: 2577236069
0:00:00 load avg: 3.88 mem: 22.8 MiB Run 1 test sequentially in a single process
0:00:00 load avg: 3.88 mem: 22.8 MiB [1/1] test_curses
== Tests result: SUCCESS ==
Total tests: run=181
Result: SUCCESS

The duplication only exists on the main branch, where gather() started reading cells with in_wch(), so there is no NEWS entry.

gather() read the window one cell at a time, so the second cell of a
double-width character was reported as another copy of it.  Read each
line with in_wchstr() instead: its count is a cell count, and it skips
the continuation cell.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This all is right, but using the new API allows us to go further. We no longer need move() + _end_of_line(y) to get the count here. We can simply read to the end of line and then strip redundant spaces (except one). Probably we can even use in_wstr().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants