Fix text-presentation checkmark width for Docker progress output#5
Open
jchristn wants to merge 1 commit into
Open
Fix text-presentation checkmark width for Docker progress output#5jchristn wants to merge 1 commit into
jchristn wants to merge 1 commit into
Conversation
Author
|
Note: I merged |
Author
|
Also, thanks for producing this fantastic library! |
This file contains hidden or 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
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.
Summary
This PR fixes Docker Compose progress-row alignment by correcting how XTerm.NET calculates the cell width of text-presentation emoji-capable characters.
The issue was identified while integrating XTerm.NET through Termrig's Avalonia terminal control:
Root Cause
InputHandler.GetStringCellWidthpreviously treated every code point classified byNeoSmart.Unicode.Emoji.IsEmoji(...)as width 2:That is too broad for characters that can be rendered in either text or emoji presentation. Docker Compose emits U+2714 HEAVY CHECK MARK (
\u2714) as a plain text-presentation character, without U+FE0F. Windowscmd.exerenders that glyph as a single terminal cell. XTerm.NET counted it as two cells, which shifted the rest of the row by one column.The visible symptom in Termrig was a missing alignment space before Docker's status text:
What Changed
This PR changes the base width calculation to use
Wcwidth.UnicodeCalculator.GetWidth(rune)and preserves the existing variation-selector adjustments:\u2714is width 1.\u2714\uFE0Fremains width 2 because U+FE0F explicitly requests emoji presentation.UnicodeCalculator.Regression Coverage
The PR adds tests for:
\u2714Xadvances two cells total and stores the checkmark as width 1.\u2714\uFE0FXadvances three cells total and stores the checkmark as a width-2 glyph with a spacer.Createdstatus text.CSI Ps Xerase-character preserving cursor position.CSI Ps Ccursor-forward clamping at the right margin without wrapping.The
CSI Ps XandCSI Ps Cbehavior already appears correct in the current code; those tests document the VT semantics that mattered during the Docker progress investigation and protect against future regressions.Reproduction
Run Docker Compose in a terminal embedding XTerm.NET:
cd C:\Code\Pneuma\docker docker compose up -d docker compose downDocker emits progress rows containing text-presentation checkmarks and column-sensitive status output. When
\u2714is counted as two cells, the status column is shifted.A minimal library-level reproduction is included in the tests:
Report
I also included
FIXES.mdin this branch with the investigation notes, reproduction examples, Termrig links, and validation details so the reasoning is preserved in the repository while reviewing this PR.Validation
Run from the repository root:
dotnet test src/XTerm.NET.slnxResult locally: