Skip to content

Add DejaVu and Noto as fallback Linux fonts.#33319

Closed
andyli wants to merge 1 commit intomicrosoft:masterfrom
andyli:patch-1
Closed

Add DejaVu and Noto as fallback Linux fonts.#33319
andyli wants to merge 1 commit intomicrosoft:masterfrom
andyli:patch-1

Conversation

@andyli
Copy link
Contributor

@andyli andyli commented Aug 29, 2017

Close #5742.

This PR enable to use of fallback fonts before font bundling (suggested in #26893) is implemented.

Close microsoft#5742.

This PR enable to use of fallback fonts before font bundling (suggested in microsoft#26893) is implemented.
@mention-bot
Copy link

@andyli, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @rebornix to be potential reviewers.

@msftclas
Copy link

@andyli,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@jhasse
Copy link
Contributor

jhasse commented Sep 13, 2017

I'm against this since it removes the incentive for Ubuntu/Chromium to fix the underlying issue. See #5742 (comment)

Also note that you've added the fonts before Courier New and monospace, which will result in a change of behavior for some people.

@andyli
Copy link
Contributor Author

andyli commented Sep 24, 2017

removes the incentive for Ubuntu/Chromium to fix the underlying issue

I still think that a simple workaround is better than waiting for a complete solution for ages. Note that the original issue (#5742) is created for over an year. And I don't see any real progress in #5742 either.

added the fonts before Courier New and monospace

This is intentional as #5742 mentioned Courier New is very different from the first preference, Droid Sans Mono. It was even suggested to remove Courier New from the list.

@jhasse
Copy link
Contributor

jhasse commented Sep 25, 2017

Good points. Wouldn't removing Courier New be the better option then, since hard-coding a font is what caused #5742 in the first place?

@andyli
Copy link
Contributor Author

andyli commented Sep 25, 2017

I'm happy to remove Courier New since I'm no fan of it anyway. However, Courier New is also in the lists of default fonts for Windows and Mac, so removing it may also affect those in some sense. If any of you really care, it can be done in another PR.

@jhasse
Copy link
Contributor

jhasse commented Sep 25, 2017

Courier New makes sense on Windows, I don't think so about Mac. On Linux monospace makes the most sense IMHO. Would moving Courier New after monospace fix the display issue for you? I think that would be the safest change.

@andyli
Copy link
Contributor Author

andyli commented Sep 25, 2017

moving Courier New after monospace

For my computer, it looks exactly the same. Probably once Courier New is installed it became the default font for monospace...

@jhasse
Copy link
Contributor

jhasse commented Sep 25, 2017

Or maybe it's the bug mentioned here: #5742 (comment)
Can you try putting monospace in single quotes?

@andyli
Copy link
Contributor Author

andyli commented Sep 25, 2017

Ah, indeed, putting monospace in single quotes did make it use DejaVu Sans Mono which is the one I installed.

@jhasse
Copy link
Contributor

jhasse commented Sep 25, 2017

Nice :) I would then suggest

const DEFAULT_LINUX_FONT_FAMILY = '\'Droid Sans Mono\', \'monospace\', monospace, \'Droid Sans Fallback\'';

for the constant. What do you think?

@andyli
Copy link
Contributor Author

andyli commented Sep 25, 2017

Yes, it sounds better than adding extra fallbacks.
I've submitted #34947 to replace this PR.

@andyli andyli closed this Sep 25, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default font on Ubuntu looks terrible (should we ship a font with vscode?)

5 participants