-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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 Serious Shanns font #1650
Add Serious Shanns font #1650
Conversation
@all-contributors please add @kaBeech for code, design |
I've put up a pull request to add @kaBeech! 🎉 |
Thanks @kaBeech for suggesting this interesting font! Some thoughts on the font (superficial review) following.
ReferencesSelf intersections everywhere as usual when using for example On the other hand it validates fine in FontBook and Glyphs3. Reference 1 Reference 2 Reference 3 Regarding the inclusion of Serious Sans to the prepatched Nerd Font setAlthough it would "solve" #1393 I am not sure the added weights and styles (Light. Bold, Italic) are hand optimised or just script runs. Serious Sans forked Comic Mono, a stale Comic Shanns fork? I believe this could be a good addition to Comic Shanns Mono instead of a stand alone half-top-of-the-lineage fork. If we add this font to NF users will get some more weights compared to Comic Shanns, but loose all the other good stuff. I literally see Issues pouring in here ;-) So, while I believe the Italics (to get a full RIBBI set) and more weights (not so much) would be a good addition, this is not really cutting it. I will add it to the suggested font list in #1095 and close this for now here. Thank you again! |
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.
Some technical comments for the PR, that might need to be addressed before pulling.
All other aspects seem to be filled out extremely careful (i.e. correct)! 👍
Hey @Finii, thanks so much for the review!
To be perfectly honest, this came down to 3 main things:
Very true. See above for why I chose this fork which lacks the extra characters. I remember vaguely wondering about this, but I didn't notice any issues when coding in Spanish or Arabic, so I didn't worry too much about it. Looking at my code now I can see that the missing glyphs were substituted in from other fonts 😅 I would like to add more characters in the future but do not have the time to do so currently.
This is probably correct (obviously correct about the self-intersections). I did autogenerate the Bold/Light/Italic styles, and I remember tweaking the Light and Italic styles, but only for the way they looked visually. It's entirely possible that I didn't tweak Bold at all (you might notice that I didn't mention the Bold style at all in my original PR comment). Honestly I'm still a font n00b and don't know much about which issues need to be fixed vs what is less necessary to address, especially when similar fonts (such as Comic Shanns Mono) display similar errors on validation. My grug brain didn't understand why a line that overlaps with itself but still makes a valid glyph that looks good would be a problem and didn't know how to fix it, so I let it be. I did just spend 5 minutes learning about this and fixed self-intersections for the regular Bold version locally, so I plan to fix them all soon. [Edit: Fixed all the self-intersections, points missing at extrema, etc. Also opened a PR to fix similar errors in Comic Shanns Mono]
I was actually confused about this when originally working on this font. I saw the link you mentioned but I didn't find any actual Point taken though - I renamed to
Oop, yep, thanks for catching that - fixed!
All told, I'll look into opening a PR for Comic Shanns Mono. My glyph modifications are the main advantage to me for using Serious Shanns compared with alternatives. I like having the Italic/Light/Bold versions available, but I remember getting them to look good taking way more work than I have time for at the moment and I feel like submitting a PR that adds more styles for the limited charset without touching the other glyphs might be tacky. As far as I can tell, the only improvements that Comic Shanns Mono has that Serious Shanns does not is the expanded charset (which I plan to incorporate eventually - once that's done it'll likely be simple enough to add the extra styles to Comic Shanns Mono as well), but if there's other good stuff Serious Shanns is missing out on then I'm open to adding it!
Hey friend, I appreciate all the work you're putting into this project! Nerd Fonts has been extremely helpful for me, both the prepatched font set and the Font Patcher. It's helped me feel empowered to adapt the tools I use for my specific needs. And I really appreciate the quick and thorough PR response! Thank you! |
Description
This adds Serious Shanns, a fork of Comic Shanns/Comic Mono I made that improves legibility, mainly by disambiguating the 'a'/'o', '1'/'l', and 'y'/'Y' glyphs, and also adds Light and Italic styles
Requirements / Checklist
Issue number where discussion took place: #xxx
What does this Pull Request (PR) do?
Adds the Serious Shanns font
How should this be manually tested?
Ideally, install it for your Terminal/IDE and check it out!
Any background context you can provide?
Font source is Serious Shanns on GitHub
What are the relevant tickets (if any)?
N/A
Screenshots (if appropriate or helpful)