Skip to content

Use new code font generally to fix Android monospace #1890

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

Merged
merged 2 commits into from
Apr 29, 2025

Conversation

Kissaki
Copy link
Contributor

@Kissaki Kissaki commented Apr 27, 2025

Using a specific font, and hosting it ourselves, allows us to ensure consistent behavior and a code font that covers the Nushell table border drawings characters.

Adds FiraCode font, which is under OFL (SIL Open Font license).

https://github.com/tonsky/FiraCode

Introduces a 100 kb font file download, which should happen only once through adequate cashing.


I can't test locally; if someone could do that, or we test it live.

Should resolve #83.

@cptpiepmatz
Copy link
Member

I'd like it better if we could just use a google font instead of rehosting a widely used font

@Kissaki
Copy link
Contributor Author

Kissaki commented Apr 28, 2025

Using Google fonts has some implications regarding data sharing.

While the Google Fonts Privacy FAQ declares no data is used for profiling or targeted advertising. Despite that, there's a German court ruling that sharing an IP address with Google and consequently into the USA constitutes sharing of personal data into a different jurisdiction / loss of control/consent.

The original benefits of CDNs often do not apply anyway. Different versions of fonts mean different websites will do their own requests. CDN delivery is often not or not significantly faster than delivering it directly. The font used here is not that popular on regular websites. And browsers have implemented resource isolation where fonts loaded from cross-origin are isolated to that context.

What do you dislike about hosting it?

@cptpiepmatz
Copy link
Member

Yeah sure, but please include the license of it:

  1. Original or Modified Versions of the Font Software may be bundled,
    redistributed and/or sold with any software, provided that each copy
    contains the above copyright notice and this license.

@fdncred
Copy link
Contributor

fdncred commented Apr 28, 2025

Sounds good @Kissaki. Thanks for the info. I agree with @cptpiepmatz that if we can get that license in the code, I will land it. Appreciate the support.

Kissaki added 2 commits April 29, 2025 20:07
Using a specific font, and hosting it ourselves, allows us to ensure consistent behavior and a code font that covers the Nushell table border drawings characters.

Adds FiraCode font, which is under OFL (SIL Open Font license).

https://github.com/tonsky/FiraCode

Introduces a 100 kb font file download, which should happen only once through adequate cashing.
@Kissaki Kissaki force-pushed the fix/codefont-android branch from e8029e9 to 447eeb8 Compare April 29, 2025 18:09
@Kissaki
Copy link
Contributor Author

Kissaki commented Apr 29, 2025

Sorry for missing the LICENSE file. I have added it.

@fdncred fdncred merged commit 2ec6143 into nushell:main Apr 29, 2025
2 checks passed
@fdncred
Copy link
Contributor

fdncred commented Apr 29, 2025

Thanks

@Kissaki Kissaki deleted the fix/codefont-android branch April 30, 2025 21:40
Kissaki added a commit to Kissaki/nushell.github.io that referenced this pull request May 1, 2025
Code blocks use a different variable, which this commit adds.

On the current Nushell website, for example [Quick Tour][quicktour] page,
we can see that the font-family being applied is in the generated style-*.css,
with declaration `code { font-family(--code-font-family); }`,
which does not match our supposed override which defines a var
under a different name `--font-family-code`.

This var name has been used since the introduction of the override in 83ced1d.

Maybe vuepress changed the variable at some point?

This is a follow-up to 2ec6143 (PR nushell#1890, PR nushell#1906),
which attempts to solve Android monospace due to missing font coverage (nushell#83).

Instead of replacing `--font-family-code`, `--code-font-family` is added as an additional value-identical declaration just in case the other variable is being used somewhere still.

[quicktour]: https://www.nushell.sh/book/quick_tour.html#nushell-commands-output-data
fdncred pushed a commit that referenced this pull request May 1, 2025
Code blocks use a different variable, which this commit adds.

On the current Nushell website, for example [Quick Tour][quicktour] page,
we can see that the font-family being applied is in the generated style-*.css,
with declaration `code { font-family(--code-font-family); }`,
which does not match our supposed override which defines a var
under a different name `--font-family-code`.

This var name has been used since the introduction of the override in 83ced1d.

Maybe vuepress changed the variable at some point?

This is a follow-up to 2ec6143 (PR #1890, PR #1906),
which attempts to solve Android monospace due to missing font coverage (#83).

Instead of replacing `--font-family-code`, `--code-font-family` is added as an additional value-identical declaration just in case the other variable is being used somewhere still.

[quicktour]: https://www.nushell.sh/book/quick_tour.html#nushell-commands-output-data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seeing issues with tables not aligning properly in some browsers
3 participants