Skip to content

fix base_has_width and add test#5008

Merged
felipesanches merged 2 commits intofonttools:mainfrom
bobh0303:fix_base_has_width
Mar 19, 2025
Merged

fix base_has_width and add test#5008
felipesanches merged 2 commits intofonttools:mainfrom
bobh0303:fix_base_has_width

Conversation

@bobh0303
Copy link
Copy Markdown
Contributor

@bobh0303 bobh0303 commented Mar 11, 2025

Description

Fixes concerns raised in #5007

Update checks/base_has_width.py to:

  • Ignore PUA characters
  • Reverse the mark class test so that it:
    • Ignores glyphs that are in GDEF mark class
    • Checks glyphs that are not in the GDEF mark class

and add tests/test_checks_base_has_width.py to verify correct behavior

Checklist

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

@bobh0303
Copy link
Copy Markdown
Contributor Author

@felipesanches What additional is needed to have this reviewed?

@felipesanches
Copy link
Copy Markdown
Collaborator

I'm on it!

]
or 0xE000 <= codepoint <= 0xF8FF
or 0xF0000 <= codepoint <= 0xFFFFD
or 0x100000 <= codepoint <= 0x10FFFD
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@simoncozens, I think this (ignoring PUA) would be a change to be ported to fontspector as well.

if codepoint == 0 or codepoint is None:
continue

if advance == 0 and not gid not in mark_glyphs(font.ttFont):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yeah! This double usage of the not operator looks like a silly coding mistake, indeed!

@felipesanches felipesanches merged commit 50b9cc5 into fonttools:main Mar 19, 2025
49 checks passed
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.

2 participants