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

fix: emoji "Regional Indicator Symbols" handling #19259

Closed
wants to merge 1 commit into from
Closed

fix: emoji "Regional Indicator Symbols" handling #19259

wants to merge 1 commit into from

Conversation

loops
Copy link

@loops loops commented Jul 6, 2022

The regional indicator symbol characters 0x1F1E6 to 0x1F1FF (as
given in www.unicode.org/charts/PDF/U1F100.pdf) are by themselves
double width characters. And as such, currently are allocated
two virtual columns when displayed.

However, in actuality these characters are almost always used in
pairs that represent a country flag. Within a terminal window,
the flag is displayed as a double width character. But we
incorrectly allocate 4 virtual columns for the displayed flag,
(two combining-characters multiplied by two virtual columns each).

Unfortunately, there is no good way within the code to indicate
that these characters should be handled differently when combined
in sequence.

On balance, editing is much easier if we properly handle the usual
case where they appear as combined characters, and thus accept the
minor visual glitch when they appear as lone characters.

While not ideal, special case these unicode regional indicator
emoji symbols as single width.

Neovim issues #19258 #16447

@loops loops changed the title Special handling of Unicode "Regional Indicator Symbols" fix: Emoji "Regional Indicator Symbols" handling Jul 6, 2022
@loops loops changed the title fix: Emoji "Regional Indicator Symbols" handling fix: emoji "Regional Indicator Symbols" handling Jul 6, 2022
@loops loops closed this Jul 6, 2022
@loops loops reopened this Jul 6, 2022
@justinmk justinmk added the unicode 💩 (multibyte) unicode characters label Jul 7, 2022
@loops
Copy link
Author

loops commented Jul 18, 2022

I notice above it says:

"Still in progress? Convert to draft"

Which I don't understand, and am not sure if there's something I should be doing to move things forward?

@WhyNotHugo
Copy link

Which I don't understand, and am not sure if there's something I should be doing to move things forward?

See above the comment box:

image

@loops
Copy link
Author

loops commented Jul 30, 2022

Which I don't understand, and am not sure if there's something I should be doing to move things forward?

See above the comment box:

Thanks. But I don't understand what a draft pull request is. How did it get marked "draft", how do you mark it as no-longer-in-draft?

@WhyNotHugo
Copy link

how do you mark it as no-longer-in-draft?

Click the "ready for review" button.

"Draft" is generally used to indicate "this isn't ready for a final review or merge, just some WIP". Not sure how it ended up in draft; the details on the issue don't indicate that.

@WhyNotHugo
Copy link

CI is failing mostly because commit messages must conform to a strict standard, see: https://github.com/neovim/neovim/blob/master/CONTRIBUTING.md#commit-messages

I think commit messages should be prefixed with fix: in this case.

The regional indicator symbol characters 0x1F1E6 to 0x1F1FF (as
given in www.unicode.org/charts/PDF/U1F100.pdf) are by themselves
double width characters.  And as such, currently are allocated
two virtual columns when displayed.

However, in actuality these characters are almost always used in
pairs that represent a country flag.  Within a terminal window,
the flag is displayed as a double width character.  But we
incorrectly allocate 4 virtual columns for the displayed flag,
(two combining-characters multiplied by two virtual columns each).

Unfortunately, there is no good way within the code to indicate
that these characters should be handled differently when combined
in sequence.

On balance, editing is much easier if we properly handle the usual
case where they appear as combined characters, and thus accept the
minor visual glitch when they appear as lone characters.

While not ideal, special case these unicode regional indicator
emoji symbols as single width.

Neovim issues #19258 #16447
@loops
Copy link
Author

loops commented Jul 31, 2022

I think commit messages should be prefixed with fix: in this case.

Thanks very much for your help.

That was a commit I didn't even create, it was created by the review process -- i just approved it. This is the second problem that one step created, making this a bit of a frustrating experience.

Ah well, hope it's all good now.

@zeertzjq
Copy link
Member

zeertzjq commented Aug 9, 2022

After #19674 :call setcellwidths([[0x1f1e6, 0x1f1ff, 1]]) does the same thing as this, and after #19686 this no longer works.

@zeertzjq zeertzjq closed this Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unicode 💩 (multibyte) unicode characters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants