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

Display errors with emoji of various widths #758

Closed
lxcode opened this issue Oct 28, 2020 · 10 comments
Closed

Display errors with emoji of various widths #758

lxcode opened this issue Oct 28, 2020 · 10 comments
Labels

Comments

@lxcode
Copy link

lxcode commented Oct 28, 2020

Small description

I apologize for even wading into the waters of unicode and terminal character width, but I haven't found a workaround that will sanitize text appropriately. When looking at data that contains smatterings of emoji and other unicode oddities, the glitches in character width cause the column separators to jump around and get out of alignment. This is particularly apparent with various flag emoji (particularly severe when many of them are used in sequence). Is there anything that visidata could do to mitigate this, perhaps using jquast/wcwidth?

Expected result

Everything displays cleanly and columns line up.

Actual result with screenshot

Screen Shot 2020-10-28 at 11 05 32

Additional context
v 2.0.1
Tested with both kitty and iTerm2 on macOS, local machine (no tmux or anything).

@lxcode lxcode added the bug label Oct 28, 2020
@saulpw
Copy link
Owner

saulpw commented Oct 28, 2020

Try setting disp_ambig_width to 2 (it defaults to 1) and let me know if that helps.

@lxcode
Copy link
Author

lxcode commented Oct 28, 2020

Unfortunately, that doesn't appear to make a difference. :-/ I've attached a tsv (had to rename it to txt to upload) that should work for repro.

chartest.txt

@saulpw
Copy link
Owner

saulpw commented Oct 29, 2020

Thanks for the test data. I use urxvt on Linux, and I also tried it with lxterminal:

urxvt
lxterminal

My font doesn't include country flags apparently, but you can see that a) the non-emoji full-width characters display properly, and b) the alignment of the columns is correct. I'd love to get mac terminals to do the right thing, especially (b), but I'm not sure it's possible. I'm guessing this has to do with combining characters (a flag is actually two characters which result in the country flag icon when combined), which VisiData is allocating 2 full-width spaces for, but only takes up one space when drawn.

@ajkerrigan
Copy link
Collaborator

This is an interesting issue - I've had similar trouble but oddly not on my Mac. When I use VisiData locally (generally inside Alacritty+tmux) everything is fine. If I record and play back an asciicast everything still looks fine. But when I upload a cast to asciinema.org, the column separators go out of alignment for rows with unicode values (sample).

I suspected I'd be able to work around this with some combination of font/encoding/terminal tweaks, but haven't found the right magic mix yet. I also tried messing with the disp_ambig_width option thanks to this thread. I'm not sure if this is the same core issue or a subtly different one, but it seems worth noting in case it's a useful data point.

@lxcode
Copy link
Author

lxcode commented Oct 29, 2020

I just tested on Linux in kitty, and I do get misalignments when I expand rows to full width (row 3). gnome-terminal keeps the alignment right, but mangles the emoji pretty badly.

My guess is this is primarily going to be an issue with flags and things like 👪 that are composite emoji of skin tone/gender.

@lxcode
Copy link
Author

lxcode commented Nov 24, 2020

Digging a bit further into this: it looks like dispwidth() can be replaced with calls to wcwidth/wcswidth (in cliptext.py and column.py), and this is probably a more future-proof and comprehensive way to deal with unicode string lengths. General calls to len() for strings like trunch and sepchars may as well use wcwidth as well.

Unfortunately, replacing those doesn't seem to fix the issue in kitty, iTerm or Terminal. I'm assuming the answer is somewhere in drawRow, but I'm not seeing it.

@saulpw
Copy link
Owner

saulpw commented Dec 2, 2020

Maybe there's a way to work around it by directly placing the column separators instead of appending them to the value string. I thought it was already doing this, but apparently not. The wcwidth library looks great (and I have been frustrated myself by the same problem, as evidenced by dispwidth), but for the time being I'd rather not take on another dependency if it doesn't fix a problem.

@saulpw
Copy link
Owner

saulpw commented Jun 26, 2021

The primary issue here is with "❤️" which is actually U+2764 ("❤") followed by U+FE0F (VARIATION SELECTOR-16). The base heart is width 1, and U+FE0F is width 0 (it's categorized as a "nonspacing mark"). But the combined emoji is width 2--and there's no way to detect this from the characters themselves. The wcwidth library doesn't handle this either.

@polm
Copy link
Contributor

polm commented Jun 26, 2021

It seems that wcwidth doesn't handle this yet but they're working on it in jquast/wcwidth#39.

@saulpw
Copy link
Owner

saulpw commented Jun 26, 2021

Thanks @polm, good find. I noticed in my above comment that Github rendered both hearts identically (with images! Unicode be damned), which got me to thinking that we can filter out these tricky combining/variant characters and replace them with indicators like for display purposes. So I implemented that and it improves things:

issue758-vis0
issue758-vis1

By default the behavior will stay the same, but with options.visibility set to 1 you can see the deconstructed form which has a deterministic layout. (This option can't be changed within vd at the moment for some reason; may be a caching issue.)

In any event, I think this is the best we can do, especially since different terminals render combinations differently. Hope this helps a bit with complicated unicode data. Thanks again for filing, @lxcode.

Oh and we also have a new sample_data/test-unicode-display.tsv which has minimum tests for some of these tricky cases (this is what's shown in the images above). Please submit other test cases that don't work (it's good to collect them all, even if we can't fix them all).

@saulpw saulpw closed this as completed Jun 26, 2021
saulpw added a commit that referenced this issue Jun 26, 2021
- add sample_data/test-unicode-display.tsv
- filter/replace combining and variant chars if options.visibility non-zero
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants