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

font-parser: Allow patching without --ext #720

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Dec 15, 2021

[why]
Sometimes fontforge returns None for font.path. That should not be the
case according to specs:

font.path
    (readonly) Returns a string containing the name of the file from
    which the font was originally read (in this session), or if this
    is a new font, returns a made up filename in the current directory
    named something like “Untitled1.sfd”. See also font.sfd_path.

This seems to be the case for fonts that do not have a fullname set.
I did not search for nor file any issue at Fontforge.

[how]
In fact we already have the original font file name, and we want to
retain its extension anyhow (if nothing is specified), so we use the
filename that we opened to determine the extension.

[note]
Related: #412
Related: #641

[note]
This was the sole usage of font.path.
Has been introduced with commit d8b760a which looks uncritical.

Requirements / Checklist

  • Read the Contributing Guidelines
  • Read or at least glanced at the FAQ
  • Read or at least glanced at the Wiki
  • Scripts execute without error (if necessary):
    • If any of the scripts were modified they have been tested and execute without error, e.g.:
      • ./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons
      • ./gotta-patch-em-all-font-patcher\!.sh Hermit
  • Extended the README and documentation if necessary, e.g. You added a new font please update the table

What does this Pull Request (PR) do?

Use the specified filename directly to detect the filename instead of the round trip through Fontforge that somehow sometimes fails.

How should this be manually tested?

Patch for example SourceHanCodeJP-Regular.otf or Noto Sans CJK Bold.otf

Any background context you can provide?

What are the relevant tickets (if any)?

#412 #641

Screenshots (if appropriate or helpful)

Edit: Typo

[why]
Sometimes fontforge returns None for font.path. That should not be the
case according to specs:

font.path
    (readonly) Returns a string containing the name of the file from
    which the font was originally read (in this session), or if this
    is a new font, returns a made up filename in the current directory
    named something like “Untitled1.sfd”. See also font.sfd_path.

This seems to be the case for fonts that do not have a fullname set.
I did not search for nor file any issue at Fontforge.

[how]
In fact we already have the original font file name, and we want to
retain its extension anyhow (if nothing is specified), so we use the
filename that we opened to determine the extension.

[note]
Related: #412
Related: #641

[note]
This was the sole usage of font.path.
Has been introduced with commit d8b760a which looks uncritical.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii
Copy link
Collaborator Author

Finii commented Dec 15, 2021

I did try Fontforge HEAD which still returns None.

@ryanoasis ryanoasis merged commit 7eb5ce5 into master Dec 17, 2021
@ryanoasis ryanoasis deleted the bugfix/cant-detect-extension branch December 17, 2021 03:14
@ryanoasis
Copy link
Owner

Tested locally just to confirm. Seems to work well. Thanks! 🏅

LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
…tension

font-parser: Allow patching without --ext
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants