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

Type1 macroman #53

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Type1 macroman #53

merged 3 commits into from
Jun 8, 2023

Conversation

typemytype
Copy link
Member

Improve by trying to read ascii, if that fails read the fontfile with macroman encoding.

Fontographer had macroman as default.

  • simplify glyphOrder extractor: python 3 dict keys are ordered

Fontographer had macroman as default.

+ simplify glyphOrder extractor: python 3 dict keys are ordered
source = T1Font(pathOrFile, encoding="ascii")
source["FontInfo"]
except UnicodeDecodeError:
source = T1Font(pathOrFile, encoding="macroman")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you gain by first trying with ascii? It is a subset of macroman anyway, so there's zero difference in behavior with just using macroman straight away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, fixed "macroman" it is

@anthrotype
Copy link
Member

Fontographer had macroman as default.

should we change the default encoding to macroman in fontTools' T1Font as well?

@justvanrossum
Copy link
Contributor

Fontographer had macroman as default.

should we change the default encoding to macroman in fontTools' T1Font as well?

Perhaps, yes. Not sure why I didn't do so back when I wrote fonttools/fonttools#1234...

@benkiel benkiel merged commit 7929250 into master Jun 8, 2023
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

4 participants