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

Improve compatibility checker #1052

Merged
merged 4 commits into from
Nov 2, 2023
Merged

Improve compatibility checker #1052

merged 4 commits into from
Nov 2, 2023

Conversation

simoncozens
Copy link
Contributor

This PR:

  • adds checking for the compatibility of GPOS contexts between contextual anchors (which can cause very confusing merge errors as entire lookups end up not aligning).
  • improves the display of compatibility error messages

Before:

ERROR:fontmake.compatibility:Fonts had differing point type in glyph D, contour 0, point 9:
ERROR:fontmake.compatibility: * Incompatible Sans Regular had curve
ERROR:fontmake.compatibility: * Incompatible Sans Bold had line
ERROR:fontmake.compatibility:Fonts had differing point type in glyph D, contour 0, point 10:
ERROR:fontmake.compatibility: * Incompatible Sans Regular had None
ERROR:fontmake.compatibility: * Incompatible Sans Bold had line
ERROR:fontmake.compatibility:Fonts had differing point type in glyph D, contour 0, point 11:
ERROR:fontmake.compatibility: * Incompatible Sans Regular had None
ERROR:fontmake.compatibility: * Incompatible Sans Bold had line

After:

ERROR:fontmake.compatibility:
Fonts had differing point type in glyph D, contour 0, point 9:
 * Incompatible Sans Regular had: curve
 * Incompatible Sans Bold had: line

ERROR:fontmake.compatibility:
Fonts had differing point type in glyph D, contour 0, point 10:
 * Incompatible Sans Regular had: None
 * Incompatible Sans Bold had: line

ERROR:fontmake.compatibility:
Fonts had differing point type in glyph D, contour 0, point 11:
 * Incompatible Sans Regular had: None
 * Incompatible Sans Bold had: line

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@simoncozens simoncozens merged commit dae37e6 into main Nov 2, 2023
14 checks passed
@khaledhosny khaledhosny deleted the context-compatibility branch December 27, 2023 19:51
@arrowtype
Copy link

@simoncozens THANK YOU for the readability upgrade here! This is a major UX improvement. Well done!

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.

3 participants