adobe-type-tools / afdko

Adobe Font Development Kit for OpenType
https://adobe-type-tools.github.io/afdko/
Other
1.05k stars 167 forks source link

[comparefamily] doFamilyTest18 should skip rest of tests if there are no bluezones #1593

Open colinmford opened 1 year ago

colinmford commented 1 year ago

I accidentally ran a font through comparefamily without any blue zones. It correctly prints the error on Line 4093: https://github.com/adobe-type-tools/afdko/blob/feebd77d9b6507a0b32f837535511be3c94d9c6f/python/afdko/comparefamily.py#L4091-L4095

but then continues on to line 4104, which tries to take an index of an empty list.

Traceback (most recent call last):
  File "/Users/colinmford/MyFont/.venv/bin/compareFamily", line 8, in <module>
    sys.exit(main())
  File "/Users/colinmford/MyFont/.venv/lib/python3.10/site-packages/afdko/comparefamily.py", line 5028, in main
    doFamilyTests(singleTestList, familyTestList)
  File "/Users/colinmford/MyFont/.venv/lib/python3.10/site-packages/afdko/comparefamily.py", line 4377, in doFamilyTests
    doFamilyTest18()
  File "/Users/colinmford/MyFont/.venv/lib/python3.10/site-packages/afdko/comparefamily.py", line 4104, in doFamilyTest18
    if tempBlues[1] < font.fontBBox[1]:
IndexError: list index out of range

https://github.com/adobe-type-tools/afdko/blob/feebd77d9b6507a0b32f837535511be3c94d9c6f/python/afdko/comparefamily.py#L4104-L4105

The function should probably be restructured around line 4101 to not continue if any of the tests above it (no zones, zones with values of zero, more than 14 zones, or an odd number of zones) fail.

colinmford commented 1 year ago

Took the liberty of making PR #1599, looking forward to your review!

colinmford commented 1 year ago

I would like to fix this error, if possible — @frankrolf or @josh-hadley, could you take a look at PR #1599 when you have a minute?

josh-hadley commented 1 year ago

@colinmford apologies for overlooking this before. The branch with #1599 is out of date, would you mind syncing/re-basing from develop so it triggers the test suite? The code changes generally look okay to me and we should be able to get this rolled in once everything is synced up.

colinmford commented 1 year ago

@josh-hadley Thanks! I just synced the branches!