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.
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
Add support for custom box drawing and powerline glyphs #16729
Add support for custom box drawing and powerline glyphs #16729
Changes from 1 commit
ad2391c
3be7d11
710eb1b
ac1c8b6
7fc081a
5606691
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
We may want to call this
compatibility.integratedDrawingGlyphs
or something - IMO and of course open to discussion!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.
Alternatively we could call them "integrated pseudographics" or "builtin semigraphics". apparently those (pseudo/semigraphics) are names for these glyphs
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.
Feature label: Built-in Glyphs
Help text: "Use built-in glyphs for BoxDrawing, BlockElement and Powerline characters"
I think this fits well with the future plan where we would draw them even when the font itself may not come with any of those glyphs (Eg. unpatched fonts)
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.
I'm mostly worried about the JSON name - where there is no help text and "glyphs" is awfully broad (there are a looooottt of glyphs in a font!)
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.
I'm opposed to renaming this setting for 2 reasons:
custom_block_glyphs
terminal.integrated.customGlyphs
So while I agree that we should call it "integrated glyphs" I think consistency with other applications is more important to make it easier for users to find related settings in related applications.
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.
FWIW that's for "vscode's integrated terminal", not for "integrated custom glyphs"
That's fair, however.
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.
In case you're still considering other names, I think both Alacritty and Contour use something like
builtin_box_drawing
for this option (as a sub setting of thefont
configuration).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.
horrifying
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.
but i love it as long as it works
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.
It's not too bad actually if you search for
0x35FDC00
on the interwebz. You'll find tons of results of this exact pattern. :)