Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[input-] fix editline() for characters having screen width > 1 #2662

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

midichef
Copy link
Contributor

@midichef midichef commented Jan 3, 2025

The input editor assumes characters have a screen width of 1. For double-width characters like , many alignment artifacts show up on screen. For example, edited values can extend 1 character past the edge of a visidata cell. Or after pressing =, pasting several double-width characters into the input field, will cause ellipses to show up on the right side of the input. And scrolling horizontally within a cell, some characters are not visible when the cursor is on them.

To demonstrate some of the issues, open a sample sheet with:
echo "12345678901234567890\nABCDEFGHIJ01-2--3---4--5--6--7--8-9" |vd -f tsv -
The header is single-width ASCII. The row is double-width letters and numbers, with single width dashes.
Shrink the cell with z_ 10. Then e and then press Right many times to see the problems. For example, after scrolling all the way right with End, the last character, , cannot be seen.

This PR fixes all known alignment issues in InputWidget.editline() for such characters. To handle the case where the screen does not have width to show a character with width > 1 at the start or end of the cell, extra characters will appear. For example, if a cell is 1 column too small to show …ABCD…, the ellipses on the right side can be repeated in the column where would otherwise start (because takes up 2 columns and can't fit into the one on screen): …ABC…… Or the ellipses on the left side can be repeated in the column where would otherwise finish: ……BCD…

(I am happy to finally be doing the len() vs dispwidth() audit that I originally scheduled for summer 2023.)

@midichef midichef force-pushed the editline_wide_chars branch from b92bacf to f70cb7c Compare January 6, 2025 19:35
@saulpw
Copy link
Owner

saulpw commented Jan 12, 2025

I'm glad you're doing the dispwidth audit, @midichef. It definitely has been a long time coming.

This particular code though is already pretty complicated, and I'm worried these improvements exceed the code budget here. Is this the essential complexity of accounting for wide characters when truncating a displayed string? I'm wondering if there's a single function like partition that can cover this ground with less cognitive overhead. Do you have a list of the known cases that were problematic? I'd like to put those into a unit test for whatever function(s) we eventually come up with.

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.

3 participants