-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
font-patcher: Fix: Fix more 'Nerd Font Mono' too wide #1062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[why] With commit 99c2608 font-patcher: Fix more 'Nerd Font Mono' too wide the glyphs 'ij' and 'IJ' are exempted from the advance width calculation, because some fonts (i.e. Overpass Mono) defines them as two cell wide glyphs (Hello? 'Mono'?) For some obscure reason it was 'IJ' and 'J circumflex' that were exempt, not 'ij'. [how] Exempt correct code. Fixes: #703 Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] That feature mainlined since ... 2.2.0 or so? Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] If a font is problematic to patch as monospaced font, that is detected but the reporting is maybe not strong enough and gets overlooked. [how] Pull font property reporting into dedicated functions. Use that function additionally in other warning. [note] The monospace check uses all glyphs to determine the advance width, but the actual advance width later ignores some glyphs (that are problematic in some fonts and are thus ignored, although that glyphs will 'break' after patching). This might or might not be useful, I just leave it as it was before. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Checked all fonts in
Filtering out all standard cases we have these problematic ones:
Where
|
[why] If a `Nerd Font Mono` font is to be created we need to make sure the original font is indeed monospaced. If it is not and we enforce the same adavnce width on all glyphs they will look very ugly. Fonts need to be designed to be monospaced. We spot check only some characteristic glyphs for that. Hermit Bold has a problem. Although it looks more or less monospaced it has some glyphs wider than all the others, for example the small letter `m`. Creating a `Nerd Font Mono` (a font where all glyphs have the same width) will either: Add too much space to the right of all the other (smaller) glyphs, or will have the wider glyphs cut off on the right. [how] Add small letter 'm' to the spot check list. Now the patcher will by default refuse to --mono patch that font. Also add output of first char that fails the monospace check. This makes debugging easier. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Although Monofur is monospaced it has one glyph (hyphen) that is slightly wider than all others. This results in a Monospaced font that is slightly too wide. [how] Ignore the hyphen width. [note] Additionally improve (commented out) debug code (shows now hex codepoint). Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
force-pushed
the
bugfix/Overpass-Mono-NFM
branch
from
January 22, 2023 13:57
1f496b5
to
dd3ed4d
Compare
This was referenced Jan 28, 2023
LNKLEO
pushed a commit
to LNKLEO/Nerd
that referenced
this pull request
Nov 24, 2023
…-NFM font-patcher: Fix: Fix more 'Nerd Font Mono' too wide
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
[why]
With commit
99c2608 font-patcher: Fix more 'Nerd Font Mono' too wide
the glyphs 'ij' and 'IJ' are exempted from the advance width calculation, because some fonts (i.e. Overpass Mono) defines them as two cell wide glyphs (Hello? 'Mono'?)
For some obscure reason it was 'IJ' and 'J circumflex' that were exempt, not 'ij'.
[how]
Exempt correct code.
Fixes: #703
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)