googlefonts / googlefonts/glyphsLib
default "gasp Table" custom parameter (empty "") crashes conversion
- Dominant language
- Python
- Stars
- 201
- Forks
- 56
- Avg merge
- 17m
- Merged PRs (30d)
- 1
Description
There are a few issues with the "gasp Table" custom parameter.
First of all, the table name is lowecase, at least in the Glyphs.app version I'm currently looking at which is 3.2 (3217).
Was it ever called "GASP Table" all uppercase in the past? If so we might have to support both. /cc @schriftgestalt
Or are the custom parameter names treated as case-insensitive?
Secondly, if the parameter is set from Glyphs.app with default values, it is serialised to .glyphs file as an empty "" string, not a dict as glyphsLib expects it... So we get an AttributeError crash by the time we attempt to convert to UFO:
```
File "/Users/clupo/.local/pipx/venvs/fontmake/lib/python3.10/site-packages/glyphsLib/builder/custom_params.py", line 528, in to_ufo_gasp_table
value = {int(k): int(v) for k, v in value.items()}
AttributeError: 'str' object has no attribute 'items'
```
Finally, if the "gasp Table" is set to non-default values, it is written out by Glyphs.app as a dict in the .glyphs source, e.g.:
```
{
name = "gasp Table";
value = {
9 = 10;
21 = 7;
65535 = 15;
};
```
~~however glyphsLib seems to completely ignore it, I cannot find any trace in the generated UFOs.~~ (CORRECTION: it does go to fontinfo.plist)
We have some unit-tests in custom_params_test.py that deal with "GASP Table" but they do not run end-to-end and may be missing something.
Oh and one more thing, which definitely looks suspiciously wrong is this line:
https://github.com/googlefonts/glyphsLib/blob/8c0157e4fba7a97fc9fbd4e4b532bcf92796c06f/Lib/glyphsLib/classes.py#L1380
should it not be `frozenset(("GASP Table",))` (plus perhaps the lowercased name)? otherwise one gets `frozenset({' ', 'A', 'G', 'P', 'S', 'T', 'a', 'b', 'e', 'l'})`
Contributor guide
Research direction
Start with Lib/glyphsLib/custom_params.py, especially to_ufo_gasp_table, and inspect the related handling in Lib/glyphsLib/classes.py. Run the existing custom_params_test.py tests and add coverage for the lowercase "gasp Table" name and the empty-string default; done means conversion no longer crashes and the parameter is handled consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100