Introduce a font name parser #717
Conversation
Finii
force-pushed
the
feature/rewrite-setup_font_names
branch
from
77e89f0
to
71e48f9
Compare
[why] Code Climate configuration for Python is per default rather restrictive and would force us to split all stuff into too many files. [how] Have some faith in the cognitive capabilities of people. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
DO NOT MERGE [why] A lot of the fonts have incorrect naming after patching. A completely different approach can help to come up with a consistent naming scheme. [how] See bin/scripts/name-parser/README.md Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
force-pushed
the
feature/rewrite-setup_font_names
branch
from
71e48f9
to
ae0f6bf
Compare
[why] Some CJK fonts seem to have no Fullname. [how] But they have a Postscript name. Use that for parsing the names. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
changed the title
Here an example.This is how the fonts come out now: Everything looks fine, but ... If we would 'write want we want' we would end up with this: The names in The Family/SubFamily grouping is for sets of four font, that have combinations of Bold and Italic. Not for weights. Obviously what we want is not correct.This is not so easy to tackle with the current naming code; and the reason I started the complete thing from scratch. A compromise is to Really set Family to what we want (a variant of #690). With that it would look like this: Now the Family is the same for all fonts, which is too many in the Family but anyhow. Because we do not touch the SubFamily it at least does make sense a bit - if only one weight is installed by the user. Which can be the case; is often the case? I don't know. And there are other issues aplenty. One is this: What This MR adds a class
Once you have this information it is easy to assemble all name fields in a consistent matter ( I guess a lot of this is already in the README file, but I wanted to add the images here. Edit: Correct |
|
Just stumbled over #107 with this cite:
Well, I guess this is a solution. Apart maybe for the limited Family name length on Windows. But then, maybe that is fixed in Windows now. I can run a check if the (supposed) length restriction is ever violated, and if not we can for sure drop the 'for Windows' prepatched fonts. |
[why]
Under certain circumstances the WWS names (Family and Subfamily) are
used to identify a font. We do not touch these SFNT table entries, so
when the font is renamed these are wrong (have the original name).
Font-grouping will go wrong then.
[how]
The typographic ('Preferred') Family and Styles are set correctly
already and they follow the WWS pattern, so the WWS fields can be (and
should) be empty. They exist to allow font grouping in the case where
the typographic names do not follow that pattern.
Remove preexisting WWS entries (because they are not needed anymore,
otherwise we would need to write the corrected new names there).
We already set the WWS bit in fsSelection that is needed:
def fs_selection(self, fs):
"""Modify a given fsSelection value for current name, bits 0, 5, 6, 8, 9 touched"""
[...]
b |= WWS # We assert this by our naming process
return b
Unfortunately we have no way (jet) to set fsSelection.
This is only the case for Iosevka for all fonts in src/unpatched-fonts.
Reported-by: Rui Ming (Max) Xiong <xsrvmy>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
@xsrvmy: Now I understood the issue. We need to remove the WWS entries, so that the Typographic entries are used. They are set with the correct names already, and as they follow WWS no extra WWS entries are needed (and existing have to be removed). I missed the step in parens, because no font Thanks for pointing that out.
|
fontforge has an undocumented call to set the fsSelection bits. Never rely on documentation :-( Found this here: fontforge/fontforge#3174 And the readback values are actually not read from the source font, so we do not use them. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] A lot people expect the font-patcher to be a stand alone script. They even think that the source glyphs (symbols) to be added to be somehow magically there and one PR makes sure that they are fetched if missing. The same problem arises when we have a script distributed over multiple files. For maintenance reasons and code quality this is what one wants. But that might hinder easy use of the font-patcher. [how] Put all the code in the main script. That has an additional drawback: For the nameparser_test* scripts to work we need stand alone files for that classes. Now the code is duplicated and will get out of sync. I have no solution for that, and it all boils down what Nerd Font wants to do. One solution would be to have font-patcher properly set up / divided in many .py files, and to create one monolithic font-patcher from all the sources on demand (via github actions or manually when someone pushes changes to any of the constituends). That approach is taken by a lot of C++ 'header only libraries' that originally consist of a lot files but create one big 'all in one' file automatically from all the small files. For now I guess we can live with the duplication, but we need to think about a solution, as this will bite us sooner or later. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
changed the title
|
I have forgotten what I wanted to add to these comments before review. Probably everything has been said already, but unfortunately all over the place and not only here. Maybe it is sufficient anyhow. Please let me know if you need more information and / or examples. What can be done in CI is to run a |
[why] We want to patch Cascadia with `--parser` while all other fonts shall be patched as before. [how] Use the config.cfg file that each source font can have to specify one arbitrary option to the font-patcher calls. This is just set in Cascadia's config.cfg, but can be extended to other fonts gradually. In this way the stand alone `font-patcher` works as before, unless someone adds the `--parser` option. Which probably will become the recommended way to use it over time. The patch-em-all script on the other hand can be instructed to use or not to use --parser on a font by font basis via their cfg file. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The fontname for Windows can be quite unusable, for example `CaskaydiaCoveNerdFontCompleteM-` for several different fonts, as this is the maximum allowed length of 31 characters that is enforced. The style/weight is completely lost. [how] Split the name into base and style (at a dash `-`) and just shrink the base name. Result for example: `CaskaydiaCoveN-ExtraLightItalic` Use equal approach for the PostScriptName (although it is less likely that length limit is ever met). Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
Pulled last two commits developed while working on #723 that would really belong here. CodeClimate has some issue, but I can not see the job page... trying ... The short names we generate for Windows are probably not needed, or only for very old stuff. The exact origin of that issue is hard to get, talks about Microsort Word 2011 for Mac that had it? Maybe we should try to run without the short names. I for my part usually use the 'normal' fonts with no issues on Windows 10. See also |
|
Note to self: Check regression #526 |
|
I guess this is finally superseeded by #723; as that contains THIS and some more. |
|
One thing I also noticed in the current state of the code (not applying this patch), is that:
|
The code in this MR keeps the blank, I believe.
You are correct. It's fixed by this MR. This is the reason why the code must be 'smart' enough to differentiate between 'name parts' and 'style/weight' parts. |
The FamilyName should of course not contain For that fonts it can be selected... see Here a list of the changes that this MR does ( |
This 'camelcasing' (i.e. removing blank between name parts) is a Nerd Fonts quirk 😬 |
Right, in the above example the family name should be "Menlo Nerd Font", and the full name "Menlo Nerd Font Regular". Which seems to be the pattern followed by this, which is great: |
|
Just noticed that ... According to the README one has to call (from the And it finds the Meslo fonts, so why are they missing? Puzzling |
|
This has been pulled indirectly via #723 |
[why]
A lot of the fonts have incorrect naming after patching. A completely
different approach can help to come up with a consistent naming scheme.
[how]
See README
Requirements / Checklist
./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons./gotta-patch-em-all-font-patcher\!.sh HermitWhat does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
In general (MS or Apple, is the same):
https://docs.microsoft.com/en-us/typography/opentype/spec/name#name-ids
https://www.fonttutorials.com/how-to-name-font-family/
About the length limits:
https://adobe-type-tools.github.io/font-tech-notes/pdfs/5088.FontNames.pdf
https://forum.glyphsapp.com/t/overly-strict-font-name-max-length-recommendation-in-naming-tutorial/10164
fonttools/fontbakery#1488
https://typedrawers.com/discussion/617/family-name/p2
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)
Edit: Add all links in Background paragraph