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

Fail if zero contours found when contours expected #4239

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

simoncozens
Copy link
Collaborator

Description

Fixes #4137

(Provide a list of all the noteworthy changes you've done. Elaborate on any decisions you had to make, and include links and/or screenshots, if applicable)

Checklist

  • update CHANGELOG.md
  • wait for the tests to pass
  • request a review

@felipesanches
Copy link
Collaborator

I dislike that this PR makes the entire check FAIL in that scenario. I think it would be more useful to report those FAIL cases separately on their own log messages, so that better context can be given to the user.

@RosaWagner's original proposal at #4137 (comment) included wording that could be used on the log messages for those special case FAIL results:

  • non-space glyphs have 0 contours. Empty glyphs should be removed from the font.
  • space glyphs have 1 or more contours.

All other glyph contour counts that diverge from expected values should remain as a WARN message.

Also, this PR does not yet implement the "space glyphs with 1+ contours FAIL" suggested by Rosalie.

com.google.fonts/check/contour_count (Universal profile)
This check now FAILs if a glyph which should have contours is found to have no contours.

(issue #4137)
com.google.fonts/check/contour_count (universal profile)

(issue #4137)
@felipesanches
Copy link
Collaborator

Also, this PR does not yet implement the "space glyphs with 1+ contours FAIL" suggested by Rosalie.

com.google.fonts/check/whitespace_ink already checks that.

@felipesanches felipesanches merged commit 7ec5289 into main Sep 21, 2023
34 checks passed
@khaledhosny khaledhosny deleted the fail-zero-contours branch October 9, 2023 13:29
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.

[code: contour-count] FAIL if contour count = 0 when ≥1 is expected
2 participants