Skip to content
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

CartographCF becomes CartographF Nerd Font Condensed #1258

Closed
3 tasks done
sounak-kun opened this issue May 24, 2023 · 14 comments · Fixed by #1259
Closed
3 tasks done

CartographCF becomes CartographF Nerd Font Condensed #1258

sounak-kun opened this issue May 24, 2023 · 14 comments · Fixed by #1259

Comments

@sounak-kun
Copy link

🗹 Requirements

  • I have searched the issues for my issue and found nothing related and/or helpful
  • I have searched the FAQ for help
  • I have searched the Wiki for help

🎯 Subject of the issue

Experienced behavior:
Some weights of CartographCF is converted to Nerd Fonts as CartographF Nerd Font Condensed.
While I can't the share the font, the issue is with the metadata. Changing the metadata of any font similar to the attached image would result in this behavior.

Expected behavior:
All weights of CartographCF gets converted to CartographCF Nerd Font.

Example symbols:
None

🔧 Your Setup

  • Which font are you using (e.g. Anonymice Powerline Nerd Font Complete.ttf)?
    • CartographCF-Bold.otf
  • Where did you get the file from (download link, self patched, source downloaded from link...)
    • self patched
  • Which terminal emulator are you using (e.g. iterm2, urxvt, gnome, konsole)?
    • alacritty
  • Are you using OS X, Linux or Windows? And which specific version or distribution?
    • Linux, Fedora Workstation 38

★ Screenshots (Optional)

Screenshot from 2023-05-25 02-27-54
Pasted image

@sounak-kun
Copy link
Author

Note: This is a regression from v2.3.3, where it used to become Cartograph CF Bold Nerd Font Complete.

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

Thanks for reporting, will try to reproduce.

You could share the debug information, that you get when patching with --debug 2.
But probably that would not help me. There is some debug missing in the FontnameParser 🤔


ah, just from re-reading that. It most probably takes the C of CF as the extremely shortened 'condensed' moniker.
I believe we do not need that anymore anyhow, that is a leftover from 'parse the filename' times fallback and Ubuntu-C.

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

$ fontforge ~/git/nerd-fonts/font-patcher CartographCF-Bold.ttf --debug 2 2>/dev/null
Nerd Fonts Patcher v3.0.1-5 (4.3.2) (ff 20230101)
DEBUG: Naming mode 1
DEBUG: Monospace check: Panose is invalid ([0, 0, 8, 9, 0, 0, 0, 0, 0, 0]); glyph-width-mono True
INFO: Setting Panose 'Family Kind' to 'Latin Text and Display' (was 'Any')
DEBUG: Extended glyphs wider bounding box than basic glyphs
DEBUG: Final font cell dimensions 1230 w x 2725 h
DEBUG: 32/160 box drawing glyphs will be replaced
Adding 180 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
Adding 6 Glyphs from Heavy Angle Brackets Set
╢████████████████████████████████████████╟ 100%
Adding 160 Glyphs from Box Drawing Set
╢████████████████████████████████████████╟ 100%
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%
Done with Patch Sets, generating font...
DEBUG: =====> Family (ID 1)      ok       (31 <=31): CartographF Nerd Font Condensed
DEBUG: =====> SubFamily (ID 2)   ok       ( 4 <=31): Bold
DEBUG: =====> Fullname (ID 4)    ok       (36 <=63): CartographF Nerd Font Condensed Bold
DEBUG: =====> PSN (ID 6)         ok       (27 <=63): CartographFNF-CondensedBold
DEBUG: =====> PrefFamily (ID 16) ok       (21 <=31): CartographF Nerd Font
DEBUG: =====> PrefStyles (ID 17) ok       (14 <=31): Condensed Bold
DEBUG: Tweaking 1/1
   CartographF Nerd Font Condensed Bold
   \===> 'CartographFNerdFont-CondensedBold.ttf'

Finii added a commit that referenced this issue May 25, 2023
[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]>
@Finii
Copy link
Collaborator

Finii commented May 25, 2023

Will be fixed with 'in parens version number' 4.3.3 (see PR above).

Thanks again for bringing this up.
Helped to throw out obsolete code 👍

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

When you patch with default naming the names are not strictly conforming (i.e. too long):

$ find . -name '*.ttf' -exec fontforge ~/git/nerd-fonts/font-patcher --dry {} \; 2>/dev/null | grep ERR 
ERROR: ====-< Family (ID 1)      too long (33 > 31): CartographCFExtra Nerd Font Light
ERROR: ====-< Family (ID 1)      too long (33 > 31): CartographCFExtra Nerd Font Light

If that is a concern for you (which is unlikely), you need to specify either --makegroup 3 or --makegroup 4 depending on your preference.

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

Hmm, did you patch different fonts or is font-patcher breaking the version :-o

Ah, seems you renamed JetBrains Mono to do an additional test 💡

image

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

Hmm,

CartographCFExtra Nerd Font Light

What is Extra? This is another bug? Should be ExtraLight/ExtraBold?
I smell a general problem.

$ fontforge ~/git/nerd-fonts/font-patcher CartographCF-ExtraBold.ttf --debug 2 --dry 2>/dev/null
Nerd Fonts Patcher v3.0.1-6 (4.3.3) (ff 20230101)
DEBUG: Naming mode 1
DEBUG: Monospace check: Panose is invalid ([0, 0, 9, 9, 0, 0, 0, 0, 0, 0]); glyph-width-mono True
INFO: Setting Panose 'Family Kind' to 'Latin Text and Display' (was 'Any')
DEBUG: Extended glyphs wider bounding box than basic glyphs
DEBUG: Final font cell dimensions 1230 w x 2743 h
DEBUG: 32/160 box drawing glyphs will be replaced
Done with Patch Sets, generating font...
DEBUG: =====> Family (ID 1)      ok       (27 <=31): CartographCFExtra Nerd Font
DEBUG: =====> SubFamily (ID 2)   ok       ( 4 <=31): Bold
DEBUG: =====> Fullname (ID 4)    ok       (32 <=63): CartographCFExtra Nerd Font Bold
DEBUG: =====> PSN (ID 6)         ok       (24 <=63): CartographCFExtraNF-Bold
DEBUG: =====> Filename 'CartographCFExtraNerdFont-Bold.ttf'

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

|-------------------------------                                                                                                                               
| Cartograph CF Extra Bold                                                                                                                                     

| Family
|-------------------------------
| Cartograph CF Extra Bold
 
| Subfamily                      
|------------------------------- 
| Extra Bold  

Ah yes, they insert a blank where they should not. Maybe we should handle that also, although that is not really 'conforming' to have Extra Bold instead of ExtraBold. Modifiers ought to stick to what they modify.

@sounak-kun
Copy link
Author

ah, just from re-reading that. It most probably takes the C of CF as the extremely shortened 'condensed' moniker. I believe we do not need that anymore anyhow, that is a leftover from 'parse the filename' times fallback and Ubuntu-C.

That makes sense. That makes me wonder, why did that issue not occur in 2.3.3 though?

Will be fixed with 'in parens version number' 4.3.3 (see PR above).

Thank you for such a quick resolution!

When you patch with default naming the names are not strictly conforming (i.e. too long):
...
If that is a concern for you (which is unlikely), you need to specify either --makegroup 3 or --makegroup 4 depending on your preference.

Yeah, I'm on Linux, so that's not an issue.

Ah yes, they insert a blank where they should not. Maybe we should handle that also, although that is not really 'conforming' to have Extra Bold instead of ExtraBold. Modifiers ought to stick to what they modify.

If I may suggest an alternative, we could pass the name of the font as a command line argument, like --name Cartograph CF, which could help the parser to handle fonts with non-conforming names.

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

That makes sense. That makes me wonder, why did that issue not occur in 2.3.3 though?

The default was --makegroups 0 for pre v3.0.0 releases, meaning old/broken re-naming system where the patcher does not try to 'understand' the font name but just sticks Nerd Font at some place in some name fields (but not all 😒 )

If I may suggest an alternative, we could pass the name of the font as a command line argument, like --name Cartograph CF, which could help the parser to handle fonts with non-conforming names.

That would not exactly help, as all those Cartograph fonts have different weights or styles, and if we do not autodetect them you would need to specify them specifically for each of the fonts.
And it is not very hard to be blank-tolerant there, I just need to write that down in code. We never had such a font before, that's why it's not handled.

@Finii
Copy link
Collaborator

Finii commented May 25, 2023

Addendum:

    # --makegroup has an additional undocumented numeric specifier. '--makegroup' is in fact '--makegroup 1'.
    # Original font name: Hugo Sans Mono ExtraCondensed Light Italic
    #                                                             NF  Fam agg.
    # 0  turned off, use old naming scheme                        [-] [-] [-]           
    # 1  HugoSansMono Nerd Font ExtraCondensed Light Italic       [ ] [ ] [ ]
    # 2  HugoSansMono Nerd Font ExtCn Light Italic                [ ] [X] [ ]    
    # 3  HugoSansMono Nerd Font XCn Lt It                         [ ] [X] [X]
    # 4  HugoSansMono NF ExtraCondensed Light Italic              [X] [ ] [ ]
    # 5  HugoSansMono NF ExtCn Light Italic                       [X] [X] [ ]
    # 6  HugoSansMono NF XCn Lt It                                [X] [X] [X]

@sounak-kun
Copy link
Author

That would not exactly help, as all those Cartograph fonts have different weights or styles, and if we do not autodetect them you would need to specify them specifically for each of the fonts. And it is not very hard to be blank-tolerant there, I just need to write that down in code. We never had such a font before, that's why it's not handled.

I could probably run it as

for FONT in cartograph-cf/*; do fontforge -script font-patcher --complete --name "Cartograph CF" $FONT; done

but I do understand the value of a proper solution. Thank you again. For the time being, I guess I'll patch it with --makegroup 0.

Finii added a commit that referenced this issue May 26, 2023
[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]>
@Finii
Copy link
Collaborator

Finii commented May 26, 2023

Patcher with the fix available now as docker or zip.

LNKLEO pushed a commit to LNKLEO/Nerd that referenced this issue Nov 24, 2023
[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: ryanoasis#1258

Signed-off-by: Fini Jastrow <[email protected]>
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants