-
Notifications
You must be signed in to change notification settings - Fork 58
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
Variation Selector 15 (VS-15, U+FE0E) support. #120
base: master
Are you sure you want to change the base?
Conversation
I did a few spot checks of VS-15 when implementing VS-16, and erroneously believed that all emojis in VS-15 sequences were already listed as an EAW width of 1. But that's not true. There are several emojis that are "wide" that are changed to "narrow" with VS-15.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #120 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 6 +1
Lines 105 115 +10
Branches 25 28 +3
=========================================
+ Hits 105 115 +10 ☔ View full report in Codecov by Sentry. |
Add any additional U+FE0F/U+FE0E check in sequence of wcswidth() to ensure 100% code coverage
basepython = python3.11 | ||
commands = {envbindir}/pylint --rcfile={toxinidir}/.pylintrc \ | ||
--ignore=tests,docs,setup.py,conf.py,build,distutils,.pyenv,.git,.tox \ | ||
--ignore=tests,docs,setup.py,conf.py,build,distutils,.pyenv,.git,.tox,table_wide.py,table_vs15.py \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, this is for .pylintrc
/ pyproject.toml
section for pylint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[tool:pylint]
may also be included in tox.ini, but I decided it was less complex to just include it here with the others
@@ -201,6 +202,17 @@ def wcswidth(pwcs, n=None, unicode_version='auto'): | |||
last_measured_char = None | |||
idx += 1 | |||
continue | |||
if char == u'\uFE0E' and last_measured_char: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u'\uFE0E'
- maybe time kill python 2.7?
I guess VS15 was the main one. I noticed other minor things like U+FF9E or U+FF9F which the grapheme cluster spec lists as "extending characters", thus width of 0 IMO but would get a width of 1 according to your algorithm, thus, e.g. ギ would result in a width of 2. I'm also not sure what exactly this means:
How do you determine when such a sequence ends? E.g. how do you determine the width of a string such as this one:
I personally also think that characters such as U+2E3B should be much wider (width of 1 in your library). I've seen it span at least 4 cells in various macOS applications. There may be more, or maybe not, I don't know. Validating your algorithm would be a lot of effort. I put the word Specification in quotes because while you do note that it's a description of your implementation, I find that writing "I authored a formal Specification detailing how characters should be measured" makes it sound more authoritative than it is, especially in the context of assigning grades to other applications based on how well they "perform". In my comment, I was trying to make the point that there is no official specification and so far, every attempt I have seen has had flaws. Even my own implementation is not perfect. Consider U+FDFD which both of our libraries will assign a width of 1:
In iTerm2, it spans 5 cells. In Chrome on macOS with a monospace font, it's even 11 cells wide. It would be interesting to render out these characters on different platforms with the most common fonts and check how their widths compare to our calculated widths. This might reveal some general flaws or at the very least it would identify outliers such as U+FDFD. I haven't had time for such a project yet, though. |
Shouldn't the width of ギ be 2? This string is aligned with East Asian Wide characters in my web browser.
|
This is U+FF77 and U+FF9E. U+FF77 is a half-width character, thus width=1. Is U+FF9E a separate character? The Grapheme Cluster Spec says it's an "extending character" and those typically don't take up extra space. They typically fall into the "Mn" category. I'm aware that some fonts will still create extra space for them, which is likely why it looks ok in your browser. I guess it's nothing big to worry about. (That's why I wrote "minor" above.) |
Thank you for your feedback @rivo I honestly appreciate it,
The characters that follow U+200d are not counted. So the first character, U+1f469 is of width 2, but the characters following the three U+200d's (U+1f469, U+1f466, U+1f466) are not counted, while 'abc' is counted normally. So the final width is 2 + 3 = 5:
The algorithm in wcswidth is fairly basic, Lines 189 to 192 in 056ee4b
Maybe you missed the details of the ucs-detect tool that I have written, but it does validate the ZWJ algorithm is 100% compliant with Konsole, foot, iTerm2, and WezTerm. (A+ score for "ZWJ" at https://ucs-detect.readthedocs.io/results.html)
My apologies for that. I will modify all references to be very specific that it is the "Specification of how python wcwidth package measures..." etc, I can only say that I try to be as terse as possible. Because wcwidth is of interest to non-english speakers that may be reading it with difficulty or through translation, I try not to mince too many words, so it may come across as more authoritative than I intended.
As for these kinds of scripts (Arabic in this case), fixed-width fonts and monospace constraints of a terminal is not appropriate. We can only do so much to interpret unicode.org specifications for the terminal environment, but measuring this kind of script isn't possible with the data files that they publish. Supporting this kind of thing would require digging into the font and its rendering engine, which wouldn't be very reasonable to implement for a general purpose command-line application library. Even terminal emulators don't often dig into the font engine. I don't believe that folks who use these languages will be very successful in designing interactive curses applications. Aside, I do wish for there to be a terminal sequence to display variable-width fonts and be released from the constraints of monospacing for such languages. Such a sequence could be used or detected at the application level to assume that the position is indeterminate, and rely on "cursor position report" queries for only an approximation of the nearest current cell. Because popular multi-language terminals (mlterm, foot, iTerm2, Konsole) measure it as width of 1 then that is what I wish for my library to report. I don't wish to invent any new specification or standards, I apologize if it is ever interpreted that way, I will include more phrasing in the README.rst to make that clear. For example, if I found a statement in a unicode.org document that disagreed with all popular terminals, I would rather our library and specification match the most popular terminals. |
I appreciate your thoughtful response.
Our goals may differ but in my Golang implementation, I don't think I mention terminals at all. Of course, it's a common area where these libraries are being used. But, for example, lots of people use VS Code and other IDEs which also use monospace fonts (except for the few who swear by variable fonts for programming but let's ignore them for now). Increasingly, such editors are used for more than just programming. Markdown, for example, appears to be integrated in more and more contexts (e.g. blog publishing, note taking, e-book authoring) and since IDEs have good support for writing Markdown, people will naturally gravitate towards using them. So I expect that these kind of algorithms are / will be relevant outside of terminals and for many different languages. You are completely right in that there is value in offering an algorithm that matches the most commonly used terminals, even when they're "wrong". There is no point in deciding a character is 2 cells wide when all terminals render it in 1 cell. In |
(0x02b55, 0x02b55,), # Heavy Large Circle | ||
(0x03030, 0x03030,), # Wavy Dash | ||
(0x0303d, 0x0303d,), # Part Alternation Mark | ||
(0x03297, 0x03297,), # Circled Ideograph Congratulation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CJK ideographs should never be narrow, emoji presentation or not.
I did a few spot checks of VS-15 when implementing VS-16, and erroneously believed that all emojis in VS-15 sequences were already listed as by EastAsianWidth.txt as width of 1.
But that's not true. There are several emojis that are "wide" that are changed to "narrow" with VS-15.
Reported by @rivo in muesli/reflow#73 (comment)
@rivo: you declare that our "Specification" is "missing some things", I would appreciate any further things that you find wrong.