TermControl: force all ambiguous glyphs to be narrow #2928
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From Egmont Koblinger: > In terminal emulation, apps have to be able to print something and keep track of the cursor, whereas they by design have no idea of the font being used. In many terminals the font can also be changed runtime and it's absolutely not feasible to then rearrange the cells. In some other cases there is no font at all (e.g. the libvterm headless terminal emulation library, or a detached screen/tmux), or there are multiple fonts at once (a screen/tmux attached from multiple graphical emulators). > The only way to do that is via some external agreement on the number of cells, which is typically the Unicode EastAsianWidth, often accessed via wcwidth(). It's not perfect (changes through Unicode versions, has ambiguous characters, etc.) but is still the best we have. > glibc's wcwidth() reports 1 for ambiguous width characters, so the de facto standard is that in terminals they are narrow. > If the glyph is wider then the terminal has to figure out what to do. It could crop it (newer versions of Konsole, as far as I know), overflow to the right (VTE), shrink it (Kitty I believe does this), etc. See Also: https://bugzilla.gnome.org/show_bug.cgi?id=767529 https://gitlab.freedesktop.org/terminal-wg/specifications/issues/9 https://www.unicode.org/reports/tr11/tr11-34.html Salient point from proposed update to Unicode Standard Annex #11: > Note: The East_Asian_Width property is not intended for use by modern terminal emulators without appropriate tailoring on a case-by-case basis. Fixes #2066 Fixes #2375
zadjii-msft
approved these changes
Sep 27, 2019
There was a problem hiding this comment.
I'm okay with this, but I also really want #900 to get better in 0.6 if we're also making it aggressively worse.
zadjii-msft
added
Area-TerminalControl
|
@zadjii-msft i can produce a table of all the emoji that exist, and which ones are miscolumned. Would that be sufficient evidence to move #900 forward, do you reckon? |
|
@DHowett-MSFT Probably. I'm probably not going to be the one solving #900 though, that seems more like @miniksa's wheelhouse |
I'm fine with building such a table, @DHowett-MSFT and resolving the columns. It looks like in #900 that Fcharlie already did something like that |
miniksa
reviewed
Oct 14, 2019
| static bool _EnsureStaticInitialization() | ||
| { | ||
| // use C++11 magic statics to make sure we only do this once. | ||
| static bool initialized = []() { |
miniksa
approved these changes
Oct 14, 2019
DHowett-MSFT
deleted the
dev/duhowett/lose_a_little_bit_of_weight_but_not_too_much
branch
|
🎉 Handy links: |
ghost
pushed a commit
that referenced
this pull request
Jul 20, 2020
* send alt/F10 through the control
We were not listening for WM_SYSKEY{UP,DOWN}
* extract the actual scancode during WM_CHAR, not the bitfield
We were accidentally sending some of the additional keypress data in with
the character event in Win32 Input Mode
* set default fg/bg to campbell
The WPF control starts up in PowerShell blue even though it's not typically used
in PowerShell blue.
* don't rely on the font to determine wideness
This is a cross-port of #2928 to the WPF control
* deterministic shutdown
In testing, I saw a handful of crashes on teardown because we were not shutting
down the render thread properly.
* don't pass 10 for the font weight ...
When Cascadia Code is set, it just looks silly.
* trigger render when selection is cleared, do it under lock
Fixes #6966.
DHowett
added a commit
that referenced
this pull request
Jul 20, 2020
* send alt/F10 through the control
We were not listening for WM_SYSKEY{UP,DOWN}
* extract the actual scancode during WM_CHAR, not the bitfield
We were accidentally sending some of the additional keypress data in with
the character event in Win32 Input Mode
* set default fg/bg to campbell
The WPF control starts up in PowerShell blue even though it's not typically used
in PowerShell blue.
* don't rely on the font to determine wideness
This is a cross-port of #2928 to the WPF control
* deterministic shutdown
In testing, I saw a handful of crashes on teardown because we were not shutting
down the render thread properly.
* don't pass 10 for the font weight ...
When Cascadia Code is set, it just looks silly.
* trigger render when selection is cleared, do it under lock
Fixes #6966.
(cherry picked from commit 76de2ae)
DHowett
added a commit
that referenced
this pull request
Jul 20, 2020
* send alt/F10 through the control
We were not listening for WM_SYSKEY{UP,DOWN}
* extract the actual scancode during WM_CHAR, not the bitfield
We were accidentally sending some of the additional keypress data in with
the character event in Win32 Input Mode
* set default fg/bg to campbell
The WPF control starts up in PowerShell blue even though it's not typically used
in PowerShell blue.
* don't rely on the font to determine wideness
This is a cross-port of #2928 to the WPF control
* deterministic shutdown
In testing, I saw a handful of crashes on teardown because we were not shutting
down the render thread properly.
* don't pass 10 for the font weight ...
When Cascadia Code is set, it just looks silly.
* trigger render when selection is cleared, do it under lock
Fixes #6966.
(cherry picked from commit 76de2ae)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From Egmont Koblinger:
See Also:
https://bugzilla.gnome.org/show_bug.cgi?id=767529
https://gitlab.freedesktop.org/terminal-wg/specifications/issues/9
https://www.unicode.org/reports/tr11/tr11-34.html
Salient point from proposed update to Unicode Standard Annex #11:
Fixes #2066
Fixes #2375
PR Checklist
Requires documentation to be updatedValidation Steps Performed
This one has some followup issues. I went wild on it, but it does still have some issues. It makes #900 worse, and introduces a debug break you hit when the renderer and the column counter disagree. It might make the renderer discard the rest of the line it was working on.