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

Interpret "na" as narrow #44

Merged
merged 3 commits into from Mar 31, 2021
Merged

Interpret "na" as narrow #44

merged 3 commits into from Mar 31, 2021

Conversation

wedaly
Copy link
Contributor

@wedaly wedaly commented Aug 14, 2020

According to tr11, the "na" designation stands for "East Asian Narrow". This includes both ASCII characters as well as some non-ASCII characters like double angle brackets. Previous versions of go-runewidth (v0.0.4) assigned these a width of one, but more recent versions assign it a width of zero. I noticed this because tcell versions after this commit refused to display the double angle bracket characters.

The fix proposed in this PR is to assign characters with the "na" designation a width of one.

According to tr11, the "Na" category stands for
"East Asian Narrow", but the generation script named the
table "notassigned".  Rename the table to "narrow"
for consistency with the unicode annex.
The narrow category applies to all ASCII characters, as well as
characters such as double angle brackets.  The tr11 annex says
that these characters are "always narrow and have explicit fullwidth
or wide counterparts", which I believe means they should be
assigned width one.

Previously, these characters were being assigned width 0.
I think before v0.0.5 they were being assigned width one,
but somewhere between v0.0.4 and v0.0.7 it changed to width 0
(maybe when introducing the generation script?).
Since we're now assigning "na" (narrow) characters width 1,
we can include the ASCII character ranges in the table.
@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2020

Codecov Report

Merging #44 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #44   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          160       162    +2     
=========================================
+ Hits           160       162    +2     
Impacted Files Coverage Δ
runewidth.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14e809f...e7278bc. Read the comment docs.

@wedaly
Copy link
Contributor Author

wedaly commented Feb 28, 2021

Hi @mattn , I was wondering if you would have time to take a look at this PR? I see at least one other user reported this issue (#45), and I'd love to get this merged upstream so other folks using tcell can get the fix.

@mattn
Copy link
Owner

mattn commented Mar 31, 2021

Sorry my long delay.

@mattn mattn merged commit 69fcf62 into mattn:master Mar 31, 2021
@mattn
Copy link
Owner

mattn commented Mar 31, 2021

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants