-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add Server Mono #1895
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
base: master
Are you sure you want to change the base?
Add Server Mono #1895
Conversation
|
@ryanoasis @Finii Are either of you able to review this please :) |
Finii
left a comment
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.
You requested a review ;-)
The fontfilenames file has been forgotten (but I also hate that the information is scattered so much), probably we should autogenerate that file instead.
(No need for you to do anything, just noticing.)
bin/scripts/lib/fonts.json
Outdated
| "unpatchedName": "Server Mono", | ||
| "licenseId": "OFL-1.1-no-RFN", | ||
| "RFN": false, | ||
| "version": "0.0.8", |
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.
They put 1.0.0 resp. 1.000 into the font file, so that is an upstream bug 😒
Edit:
Also if I remember correctly the version string should not start with "Version ". I see you (?) use Glyphs3, is that autocreated by it? Do not have my Font-MacBook on my right now, so can not check (on a Linux machine right now).
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.
Just checked the version number and you're right, I've updated it to be accurate. Also I checked in FontForge and I don't see the 'Version ' prefix, I'm re-adding the new font files though so it might fix things.
0da5a17 to
cf0fd19
Compare
173e4a8 to
6015814
Compare
|
Thanks for taking the time to review this and help guide me on the stuff I missed! I really appreciate the effort and knowledge! I agree that it'd be helpful for new folks like myself to have the documentation in one place (or at least pointing to the various places). I noticed a bug when generating the diff --git a/src/unpatched-fonts/README.md b/src/unpatched-fonts/README.md
index be50bdb4..4b597e19 100644
--- a/src/unpatched-fonts/README.md
+++ b/src/unpatched-fonts/README.md
@@ -13,8 +13,8 @@ all the fonts in `fontfilenames`. This can be used via
Just call
- jq -r '.fonts[$i].imagePreviewFontSource' ../../bin/scripts/lib/fonts.json \
+ jq -r '.fonts[].imagePreviewFontSource' ../../bin/scripts/lib/fonts.json \
| grep -v '\.sfd$' \
| sed 's/ /\\ /g' \
| LC_ALL=C sort --ignore-case \
- > fontfilenames
+ >| fontfilenames
I removed the problematic I can add this change to this PR, but if you'd prefer to keep it separate then I'll let you add it on your own accord. |
2d04ce4 to
ab3edda
Compare


Description
Added the font Server Mono, followed the
contributing.mdguide.Requirements / Checklist
Issue number where discussion took place: #xxx
What does this Pull Request (PR) do?
Add the Server Mono font
How should this be manually tested?
Any background context you can provide?
Origin: https://github.com/internet-development/www-server-mono
I'm the maintainer of this font, please let me know if there are any questions/concerns and I'd be happy to help.
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)