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
Draft: Introduce a file name parser #706
Conversation
DO NOT MERGE Invoke: $ fontforge bin/scripts/name_parser_test1 src/unpatched-fonts/**/*.[ot]tf 2>/dev/null This looks into name_parser_test1.known_issues for explained issues and generates a new known_issues file from the findings. If there are new issues they will turn up as 'AUTOGENERATED', and obsolete issues (fixed ones) will be listed at the end of the new file. In this way one can tweak the code and compare very easily what a change means for all the fonts, which will break or be repaired. This can also be used if new fonts are added, one can check the parser output and potentially add the needed new known_issue rules by just moving known_issued.new to knows_issues. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
I just wanted to get this going/official. Further explanations, tables, screenshots, reference websites, etc pp will be added tomorrow (or so). Please bear with me. |
[why] We want to reuse the code in different tests. [how] Modularize. [note] Also remove wrong 'close' call. Also correct remark on Lilex. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The Light fonts are not grouped to the Regular and Bold fonts in typographic names aware applications. [how] This is a longer standing problem with font-patcher, that does not fill the typographic family and style with meaningful values. We could do better with the `do_rename` script, but we did not. Instead of repairing the `do_rename` script the `name_parser.py` module is included, that I would like to get into `font-parser` to repair its behaviour for all fonts. As noone knows when or if at all it will end up in the patcher script, we add the same code here. The module will set the 4 names (family, subfamily, typogr. family and typogr. style) in a way that we get correct grouping in both application types: typographic aware one and 'ordinary' ones. [note] The `name_parser.py` originates here: ryanoasis/nerd-fonts#706 Fixes: #72 Reported-by: Rashil Gandhi <mail@rashil2000.me> Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The fontforge Python interface is not able to easily remove existing SFNT entries. [how] Because it it not so easy to apply the findings of the FontnameParser to a font a helper function is added that does all the necessary bits and pieces. [note] Also fix instances where name_parser uses Python features that are too 'new' on some machines (i.e. Gitlab Ubu 18.04 runner). Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Added a convenience function to the parser, that applies all the needed stuff to a font. Example: font = fontforge.open(fname)
n = FontnameParser(fname)
n.enable_short_style_when('Noto')
n.add_name_substitution_table(SIL_TABLE)
n.inject_suffix("Nerd Font Complete Mono", "Nerd Font Complete M", "Nerd Font Mono")
n.rename_font(font) # <=- does all the magic Use this thing now at adam7/delugia-code@203324a |
Codeclimate ... *cough* |
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
The very low number of lines per file forces us to split up the stuff into two files... To still keep this a bit organized (and with the background that the code propably will end up in the font-patcher file anyhow), all this stuff is put into a dedicated subdir. The call changes thus: $ fontforge bin/scripts/name_parser/name_parser_test1 src/unpatched-fonts/**/*.[ot]tf 2>/dev/null Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
d85f8af
to
694e999
Compare
Fixed the code climate stuff. There are some changes for the better but also some that I would rather have put to 'wontfix'. Especially the limit to 250 LOC is Whatever. Lets turn to the actual issues this shall fix, in the next comment. |
[why] Fontforge makes a lot of assumptions when we want to manipulate the SFNT table. We do not want that; we want to get what we asked for. [how] Manipulate the SFNT tuples directly. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Suffixes should not start with a blank or end with one, the resulting names would look wrong. But `font-patcher` calculates them with a beginning blank (for easy addition in the old code). It is more convenient and also more robust if the parser makes sure that no superfluous blanks are added. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] These scripts can help to detect issues with fonts. Of course there are also a lot of other tools that do the same. These tools document how the comments in the PR were generated. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] To correct a lot of problems we want to use a new way to set the various name parts in the patched font. [how] Keep the old behavior, add new command line flag that enables the new method (i.e. --parser). The result will be unchanged if the flag is not given. The FontnameParser is only used to set the names if the flag is given. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Here one motivation example, why we want a better renaming process. The helper scripts are also in this branch. I removed some 'noise' from the output.
If the renaming should result in |
[why] The `font-patcher` supresses name parts that are 'for powerline' or 'powerline'. The reason is, that all Nerd Fonts font contains these anyhow. [note] Change `font-patcher` to use the new function. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Somehow the line breaks of now-dropped known issues are missing [how] Add a newline to output files. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] When we apply one substitution table and then switch to another the old table seems to keep being active. [how] If there is no new replacement the old replacement is not undone. Restore the basename now always to the default/startvalue. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The `font-patcher` (historically?) created the Family names of 'Caskaydia Cove' without the blank as 'CaskaydiaCove' (same for Mono). This SIL table adapted that behavior. But then, it seems almost wrong. [how] Change it back to include the blank (if existing). Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
I changed the But Nice that we can test if code changes resulted in any unexpected changes... but no:
So all the 'issues' are already known and explained for / accepted. ;-) CodeClimate is so bad ... Refactor one function away ... yes, that improves code quality for sure *raise eyebrows* |
[why] I have no clue. [how] Just drop a debugging functions. Who needs debugging. And we can get the data out of the object anyhow. Encapsulation in Python is a farce ;-) Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] It can be good to query more than one SFTN value at once. [how] The key can now be given as comma-separated-list. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Here is another Issue of the current
Both the |
Compare with a
|
[why] The font-patcher historically grouped all name parts of the original font together (if there are any) to form a camel case name: Cascadia Code => CaskaydiaCove But only in the Family and Preferred Family names. [how] We already have an option to create short(er) Families by usage of abbreviated style names (stemming from Noto). Add an additional option that CamelCases the basic fontname in the Family names. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Now we mimic the historic
I am not sure if the behavior makes any sense or is accidentially. If we really strive for short Family names, we should maybe activate 'short styles' also, for all fonts. But for the moment lets stick as close to the old naming as reasonably possible. |
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
The script patches a given font two times with font-patcher * 'normal', just with --powerline and noithing more * one times with --parser specified (will use the FontnameParser The font files that resulted will be opened and the embedded names compared. It is important to really patch a file and see the outcome because fontforge is a bit unpredictable when it comes to what actuall is set in a font file ;) The names are compared with a but of lenience, like with name_parser_test1. What is missing is a known-issues file. Will add that later. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
4370add
to
e77f382
Compare
(Force push because commit title was wrong 🙄 (CWD vs PWD)) |
I guess this MR has too much noise. Will close this and reopen a new one, with hopefully better reasoning. (All the explanatory scripts are ready now, I guess.) |
hey @Finii I was somewhat following along in the shadows. Sorry I haven't commented to know. All this work is really impressive and to be honest I feel a lot of it is above my head (at least at this moment) so I wasn't sure what to comment specifically. It's impressive work, I'll be reviewing some other PRs (including yours in the meantime).
Sure if that makes more sense to you. Also I did see that the font patcher test workflows finally showed up and that was of particular interest to me at that point. 👍🏻 |
Yes, sorry; to prepare all the additional material (reasoning, tests, explanations) takes much more time than the actual coding itself. One test-run with all fonts takes like 3 hours on my machine. But I guess I'm getting close to present this in a easier to follow way. This here is way too convoluted in train of thought. The problem is to present the results (741 fonts with all possibly different names) in a way that is easy to follow and is not more than some lines long, but still explains everything :-D |
If you like some preliminary reading, this is what I have written so far for the upcoming PR. ;-) Creating Consistently Grouped Patched FontsThis is a small sub-project to font-patcher that uses a little bit more knowledge Consider a font named 'Times' that has two variants: normal and bold. For this font the With this applications are able to group all 'Times' together and additionally choose the MotivationQuite a number of patched fonts have inconsistent or simply wrong font grouping. The naming in And we would like to have the information within the names sorted in a consistent way.
This is important because we want to add subvariant information, namely the Example:
The PlanTo solve these issues the font name parts have to be analyzed more thoroughly and then categorized. Typographically aware applications, on the other hand, get all styles grouped under one Family name. First experiments showed that the full information can usually be restored already from the file This new naming is complete optional (but recommended). Give the option The TestsIn this directory there are two tests.
All tests base on these assumptions
Test 1
This test takes the filename of a font, parses it and generates names from it. Then the actual The output shows all the names, always two lines: first the generated names, then the readout The differences have reasons, and there is a file with textual explanations for them. So far Test 2
This test compares actually patched fonts. Every font in Also again a file with known differences (with explanations) is read, and any new or vanished Further stepsOne can examine all the (current) naming differences in the The Explanation sorts most differences into common groups. This helps to weed out Helper scriptsThere are some helper scripts that help examining the font files. Of course there are other, |
Just so my thought isn't lost, on the code climate. I just threw that integration in a few years maybe ago? I'm not tied to it and up for any better suggestions. That said there isn't much fine tuning available via the UI however there is some advanced configuration to make note of that is interesting: https://docs.codeclimate.com/docs/advanced-configuration#default-checks |
I reckon it's better than nothing. It did trigger some improvements in my code here. Something should be there. Personally I have no experience with code quality for Python, I do C++ the whole day ;)
I added some tweaks to it in my new (to be completed) MR, namely:
I believe these values are still low enough to encourage more thoughts being put into code structure, but the code can leave the 'casual script' level. |
This is obolete -=> See #717
Description
The
font-patcher
shall patch (almost) any input font file and generate a useful new font file. This can be complicated because the font name and font name parts need to be changed in the process. At least the "Nerd Font" label shall be added to the name, so that the fonts are differentiated from the original fonts if both are installed.This is furthermore complicated, because the "Nerd Fonts" has to be placed at the right position in the font name.
Then there are some fonts with issues, where the grouping of related fonts is not correct in some applications, or some become invisible.
The current solutiuon (in
font-patcher:setup_font_names
) is not very systematic. This MR shall develop a new way to setup the names (and naming parts) of the patched fonts and more.The goal is to have a very robust font (re)naming implemented so that any sets of fonts can be patched without loss (and possibly increased) usability apart from the added glyphs. That would make adding new source fonts a breeze (like all the Cascadia styles) and additionally helps people that patch other fonts for their own use.
What does this Pull Request (PR) do?
At the moment the proposed algorithm is added as test script. It uses just/exclusively the font filenames to generate the font naming parts.
To show that this approach is viable:
src/unpatched_fonts
and compares these to the embedded (original) information in all these filesMore will be explained later.
The proposed algorithm is a Python class like this:
The use rater goes along this lines:
How should this be manually tested?
$ fontforge bin/scripts/name_parser_test1 src/unpatched-fonts/**/*.[ot]tf 2>/dev/null
Any background context you can provide?
What are the relevant tickets (if any)?
#663 #690 #695
Screenshots (if appropriate or helpful)
Requirements / Checklist
Put this last, as this is not so important at the moment.
./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons
./gotta-patch-em-all-font-patcher\!.sh Hermit