-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[color] Use graphemes to measure strings. #11202
Conversation
Changelog[uncommitted] (2022-11-09)Bug Fixes
|
Stacks on top of #11201 |
652e057
to
221795e
Compare
dfacf7e
to
955e04c
Compare
The number of Unicode code points in a string is not the same as the number of user-visible characters (graphemes). When measuring colorized strings, we want the latter rather than the former. Notably, these changes fix some issues where the interactive display cut off before the right edge of the terminal.
955e04c
to
3aecebe
Compare
<{%reset%}> <{%reset%}> │ ├─ awsx:x:ec2:NatGateway vpc-0 <{%reset%}><{%reset%}> | ||
<{%reset%}> <{%reset%}> │ │ └─ aws:ec2:Eip vpc-0 <{%reset%}><{%reset%}> | ||
<{%reset%}> <{%reset%}> │ ├─ awsx:x:ec2:Subnet vpc-public-0 <{%reset%}><{%reset%}> | ||
<{%reset%}> <{%reset%}> │ │ └─ aws:ec2:RouteTable vpc-public-0 <{%reset%}><{%reset%}> |
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.
Nit: It looks like we can optimize out a colorized column of length 0 and elide it.
To elaborate on these issues: In the interactive display, resources are rendered in a tree view with visible edges from children to parents. Each of these edges is formed using Unicode Box Drawing characters. These characters are outside the ASCII range, and each of their UTF-8 code units consists of three bytes. The code in the colorizer that clamps the output of colorization to a particular width was using byte counts instead of code points or graphemes. With these changes, these measurements are now done using graphemes (i.e. user-visible characters). |
<{%reset%}> <{%reset%}> ├─ aws:ec2:SecurityGroupRule cluster-eksNodeClusterIngressRule <{%reset%}><{%reset%}> | ||
<{%reset%}> <{%reset%}> ├─ kubernetes:core/v1:ConfigMap cluster-nodeAccess <{%reset%}><{%reset%}> | ||
<{%reset%}> <{%reset%}> ├─ aws:ec2:LaunchConfiguration cluster-nodeLaunchConfiguration <{%reset%}><{%reset%}> | ||
<{%bold%}><{%fg 3%}>~ <{%reset%}> └─ aws:cloudformation:Stack cluster-nodes <{%bold%}><{%fg 3%}>updating<{%reset%}><{%bold%}><{%fg 3%}><{%reset%}> |
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.
Kudos: Having snapshot based tests for display logic make reviewing these PRs much easier. 👍
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.
Oops. Meant to approve. Nit is non-blocking, per name.
bors merge |
11202: [color] Use graphemes to measure strings. r=pgavlin a=pgavlin The number of Unicode code points in a string is not the same as the number of user-visible characters (graphemes). When measuring colorized strings, we want the latter rather than the former. Notably, these changes fix some issues where the interactive display cut off before the right edge of the terminal. Co-authored-by: Pat Gavlin <pat@pulumi.com>
Timed out. |
bors retry |
Build succeeded: |
The number of Unicode code points in a string is not the same as the
number of user-visible characters (graphemes). When measuring colorized
strings, we want the latter rather than the former. Notably, these
changes fix some issues where the interactive display cut off before the
right edge of the terminal.