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-patcher: Ensure PostScript font name does not contain spaces #820

Closed
wants to merge 1 commit into from

Conversation

torarnv
Copy link
Contributor

@torarnv torarnv commented Apr 10, 2022

When pulling the subfamily out of the sfnt_names SubFamily property,
we will get a subfamily with possible spaces, e.g. 'Bold Italic'.

When constructing the final unique font name (PostScript name), we
need to remove those spaces, to make the font name valid, otherwise
the font will fail validation with a warning when installing.

Fixes #413

When pulling the subfamily out of the sfnt_names SubFamily property,
we will get a subfamily with possible spaces, e.g. 'Bold Italic'.

When constructing the final unique font name (PostScript name), we
need to remove those spaces, to make the font name valid, otherwise
the font will fail validation with a warning when installing.

Fixes ryanoasis#413
@torarnv
Copy link
Contributor Author

torarnv commented Apr 10, 2022

@all-contributors add

@allcontributors
Copy link
Contributor

@torarnv

We had trouble processing your request. Please try again later.

@Finii
Copy link
Collaborator

Finii commented Apr 10, 2022

The SubFamily Naming in broken in multiple ways anyhow, I do not know if fixing individual aspects is useful anymore.
Naming is completely rewritten by #717.
To try 717 it is part of 723; 706 is 717's predecessor.

On the other hand this change is small ;-)

@torarnv
Copy link
Contributor Author

torarnv commented Apr 10, 2022

The SubFamily Naming in broken in multiple ways anyhow, I do not know if fixing individual aspects is useful anymore. Naming is completely rewritten by #717. To try 717 it is part of 723; 706 is 717's predecessor.

Yeah, I noticed :) Probably better to focus the work in #717 :)

@torarnv torarnv closed this Apr 10, 2022
@torarnv torarnv deleted the fix-font-name branch April 10, 2022 13:56
@Finii
Copy link
Collaborator

Finii commented Apr 10, 2022

I'm not against merging THIS, as I fear it will still be a long time until 717's --makegroups is the default. At the moment the fixed mechanism in the PR is optional...

Just noticed --makegroups is only lousily documented in the PRs :-(

And then, is the option name a good name?

@Finii
Copy link
Collaborator

Finii commented Apr 10, 2022

It's not documented at all, just mentioned in the commit 251de5a :-(

@torarnv torarnv restored the fix-font-name branch April 10, 2022 14:11
@torarnv
Copy link
Contributor Author

torarnv commented Apr 10, 2022

Okey, reopening in case we want to merge it

@torarnv torarnv reopened this Apr 10, 2022
@Finii
Copy link
Collaborator

Finii commented Aug 23, 2022

Merged directly, because I wanted a FF merge and could not push to your repo.

Commit 6acec45

image

@Finii Finii closed this Aug 23, 2022
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.

font validation failed, installing Meslo font
2 participants