name-parser: Remove filename parsing code and more #1259
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]
Patching CartographCF-Bold.ttf creates this naming:
Family (ID 1) : CartographF Nerd Font Condensed
SubFamily (ID 2) : Bold
Fullname (ID 4) : CartographF Nerd Font Condensed Bold
PSN (ID 6) : CartographFNF-CondensedBold
PrefFamily (ID 16) : CartographF Nerd Font
PrefStyles (ID 17) : Condensed Bold
CartographF Nerd Font Condensed Bold
\===> 'CartographFNerdFont-CondensedBold.ttf'
[how]
The font-patcher historically used the file name of the to-be-patched
font to come up with the new name. When the FontnameParser has been
developed that mechanics has been copied at least for fallback. The
earliest tests compared old and new naming with all the filenames.
Later, when the FontnameParser has been used to really apply name
changes it has always based the parsing on the Fullname or the PSname,
because they really hold the information (or at least should hold);
while the filename might be completely random.
Still code the dealt with specific problems in FILEnames prevailed. The
Ubuntu font for example has a file name like 'Ubuntu-C.ttf', and we
needed to convert the C to Condensed.
As that requirement vanished we can drop all the code that has been
added specifically only for parsing the Ubuntu font filenames.
Side note: USUALLY font filenames should be roughly equal to the PSname.
Fixes: #1258
Signed-off-by: Fini Jastrow <[email protected]>
[why] The code is obviously wrong. No effect has been seen, though. First we check if a certain string is a key in the dict. If it is, we retrieve the value with the string lower-cased as key. This does not make sense. [how] All the keys are lower case anyhow, so the code seems unneeded. Maybe it is a leftover. The styles that go into it _and are in the dict_ all come from a regex-enabled search and thus are lower-cased. Whatever, to have the correct code we use the lower-cased string for both, checking for existance and retrieving the value - this is the only sane approach. Also change to dict.get() method instead of a self made if code. Signed-off-by: Fini Jastrow <[email protected]>
Finii
changed the title
|
Used this to check the names before/after: From 48bbdc68f9d2b75550957d7dd5d9219a33547e8b Mon Sep 17 00:00:00 2001
From: Fini Jastrow <[email protected]>
Date: Fri, 26 May 2023 10:58:10 +0200
Subject: [PATCH] DEBUG
---
.../gotta-patch-em-all-font-patcher!.sh | 30 +++++++++----------
bin/scripts/name_parser/FontnameParser.py | 2 ++
font-patcher | 3 +-
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/bin/scripts/gotta-patch-em-all-font-patcher!.sh b/bin/scripts/gotta-patch-em-all-font-patcher!.sh
index a5020690e..128b74ddd 100755
--- a/bin/scripts/gotta-patch-em-all-font-patcher!.sh
+++ b/bin/scripts/gotta-patch-em-all-font-patcher!.sh
@@ -259,21 +259,21 @@ function patch_font {
{ OUT=$(fontforge -quiet -script ${PWD}/font-patcher "$f" -q ${font_config} $powerline $post_process -c --no-progressbars \
--outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS} 2>&1 1>&3 3>&- ); } 3>&1
if [ $? -ne 0 ]; then printf "$OUT\nPatcher run aborted!\n\n"; fi
- # Create "Nerd Font Mono"
- if [ -n "${verbose}" ]
- then
- echo "fontforge -quiet -script ${PWD}/font-patcher "$f" -q -s ${font_config} $powerline $post_process -c --no-progressbars --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS}"
- fi
- { OUT=$(fontforge -quiet -script ${PWD}/font-patcher "$f" -q -s ${font_config} $powerline $post_process -c --no-progressbars \
- --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS} 2>&1 1>&3 3>&- ); } 3>&1
- if [ $? -ne 0 ]; then printf "$OUT\nPatcher run aborted!\n\n"; fi
- # Create "Nerd Font Propo"
- if [ -n "${verbose}" ]
- then
- echo "fontforge -quiet -script ${PWD}/font-patcher "$f" -q --variable ${font_config} $powerline $post_process -c --no-progressbars --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS}"
- fi
- { OUT=$(fontforge -quiet -script ${PWD}/font-patcher "$f" -q --variable ${font_config} $powerline $post_process -c --no-progressbars \
- --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS} 2>&1 1>&3 3>&- ); } 3>&1
+ # # Create "Nerd Font Mono"
+ # if [ -n "${verbose}" ]
+ # then
+ # echo "fontforge -quiet -script ${PWD}/font-patcher "$f" -q -s ${font_config} $powerline $post_process -c --no-progressbars --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS}"
+ # fi
+ # { OUT=$(fontforge -quiet -script ${PWD}/font-patcher "$f" -q -s ${font_config} $powerline $post_process -c --no-progressbars \
+ # --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS} 2>&1 1>&3 3>&- ); } 3>&1
+ # if [ $? -ne 0 ]; then printf "$OUT\nPatcher run aborted!\n\n"; fi
+ # # Create "Nerd Font Propo"
+ # if [ -n "${verbose}" ]
+ # then
+ # echo "fontforge -quiet -script ${PWD}/font-patcher "$f" -q --variable ${font_config} $powerline $post_process -c --no-progressbars --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS}"
+ # fi
+ # { OUT=$(fontforge -quiet -script ${PWD}/font-patcher "$f" -q --variable ${font_config} $powerline $post_process -c --no-progressbars \
+ # --outputdir "${patched_font_dir}" $config_patch_flags ${NERDFONTS} 2>&1 1>&3 3>&- ); } 3>&1
if [ $? -ne 0 ]; then printf "$OUT\nPatcher run aborted!\n\n"; fi
# wait for this group of background processes to finish to avoid forking too many processes
diff --git a/bin/scripts/name_parser/FontnameParser.py b/bin/scripts/name_parser/FontnameParser.py
index 248f0d154..0a1098a38 100644
--- a/bin/scripts/name_parser/FontnameParser.py
+++ b/bin/scripts/name_parser/FontnameParser.py
@@ -269,6 +269,7 @@ class FontnameParser:
self.logger.debug('=====> {:18} ok ({:2} <={:2}): {}'.format(entry_id, len(name), max_len, name))
else:
self.logger.error('====-< {:18} too long ({:2} > {:2}): {}'.format(entry_id, len(name), max_len, name))
+ print("| {:50}".format(name), end='')
return name
def rename_font(self, font):
@@ -330,6 +331,7 @@ class FontnameParser:
p_sty = self.preferred_styles()
if len(p_sty):
sfnt_list += [( 'English (US)', 'Preferred Styles', self.checklen(31, 'PrefStyles (ID 17)', p_sty) )] # 17
+ print("|")
font.sfnt_names = tuple(sfnt_list)
diff --git a/font-patcher b/font-patcher
index 0434458ea..e7b55f941 100755
--- a/font-patcher
+++ b/font-patcher
@@ -721,6 +721,7 @@ class font_patcher:
short_family = projectNameAbbreviation + variant_abbrev if self.args.makegroups >= 4 else projectNameSingular + variant_full
# inject_suffix(family, ps_fontname, short_family)
n.inject_suffix(verboseAdditionalFontNameSuffix, ps_suffix, short_family)
+ print("NAMES | {:40}".format(os.path.basename(self.args.font)), end='')
n.rename_font(font)
font.comment = projectInfo
@@ -1977,7 +1978,7 @@ def main():
global logger
logger = logging.getLogger(os.path.basename(args.font))
- logger.setLevel(logging.DEBUG)
+ logger.setLevel(logging.CRITICAL)
log_to_file = (args.debugmode & 1 == 1)
if log_to_file:
try:
--
2.39.2
Hmm, 2 font files differently named |
[why]
Some fonts might have a non-standard (i.e. broken) weight naming scheme:
They put a blank or a dash between the modifier and the weight, for
example "Extra Bold" or "Demi-Condensed", when they mean "ExtraBold"
resp "DemiCondensed".
The former happens with CartographCF, the later with IBM3270.
[how]
Automatically allow a dash between modifier and weight, which comes up
as CamelCase boundary. Insert an optional dash (r'-?') into such
boundaries.
For the further lookup we need to remove the dash in the found keyword,
if there is any, to get back to standard naming.
This might break if the font name ends in a modifier. So we can not
really distinguish
Font Name Extra Bold Italic
=> Font Name - ExtraBold Italic
=> Font Name Extra - Bold Italic
The known modifiers are 'Demi', 'Ultra', 'Semi', 'Extra'.
It is possible but unlikely that a font name ends in one of these.
For example "Modern Ultra - Bold".
[note]
The question arises if we should not parse the PSname instead of the
Fullname; and stick to the dash there as boundary.
The problem might be prepatched fonts with broken naming, that would be
parsed completely wrong then. So maybe the current approach is still the
best, with the caveat given above (fontnames ending in a modifier).
[note 2]
Funny enough the variable allow_regex_token was not used at all :->
Some leftover? Anyhow we use it now.
[note 3]
We can still not remove the special handling for IBM3270, because the
font initially looks like a PSname and this is parsed as such, which
breaks the name in the incorrect place:
PSname template = "Name-StylesWeights"
Fullname of 3270 = "IBM 3270 Semi-Condensed"
Signed-off-by: Fini Jastrow <[email protected]>
Finii
force-pushed
the
bugfix/remove-filename-specials
branch
from
bb21d28 to
e4637d7
Compare
[why] Function get_name_token has a Cognitive Complexity of 12 (exceeds 9 allowed). Consider refactoring. [how] Remove not really needed special case. Signed-off-by: Fini Jastrow <[email protected]>
[why] The 'Text' weight of Plex is handled as 'other', means that this is added to the font's name and is a distrinct own family. But in the original font it is used as weight. [how] Remove special handling of 'text' in the font name. Add 'Text' to known_weights list. "Text" is not a standard naming, but I see no problems when we handle it as one. This keeps the family relationships in Blex like Plex. Signed-off-by: Fini Jastrow <[email protected]>
[why] IBM Plex uses some abbreviations also in the fullname and we do not try abbreviations when resolving weights. [how] As this is the only font that has such specials we handle it beforehand and do not try all combinations of abbreviated and long keywords. And then their abbreviations are also not standard - at least not used by us or Adobe, etc. For such a small amount of affected font files it seems in order to specifically just fix them instead of a general solution. Signed-off-by: Fini Jastrow <[email protected]>
Finii
force-pushed
the
bugfix/remove-filename-specials
branch
from
e4637d7 to
7a224c6
Compare
|
More specials of |
|
This PR introduces the following name changes (corrections): |
|
@sounak-kun Carto looks also good: |
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]
Patching CartographCF-Bold.ttf creates this naming:
[how]
The font-patcher historically used the file name of the to-be-patched font to come up with the new name. When the FontnameParser has been developed that mechanics has been copied at least for fallback. The earliest tests compared old and new naming with all the filenames.
Later, when the FontnameParser has been used to really apply name changes it has always based the parsing on the Fullname or the PSname, because they really hold the information (or at least should hold); while the filename might be completely random.
Still code the dealt with specific problems in FILEnames prevailed. The Ubuntu font for example has a file name like
Ubuntu-C.ttf, and we needed to convert theCtoCondensed.As that requirement vanished we can drop all the code that has been added specifically only for parsing the Ubuntu font filenames.
Side note: USUALLY font filenames should be roughly equal to the PSname.
Fixes: #1258
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Check all generated font names from prepatched fonts
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)