googlefonts / pyfontaine

Python tool to check font files for language/character set support
https://github.com/googlefonts/pyfontaine
GNU General Public License v3.0
103 stars 22 forks source link

JSON output is invalid #110

Closed alerque closed 4 years ago

alerque commented 4 years ago

I just tried using pyfontaine --json's output for something (in my case piping it to jq for processing) and discovered the output is not valid JSON.

[  {    'Filename': "MyFont.otf", } ]

Note that the only valid way to enter a string in JSON is with double quotes, and the array keys are being output with single quotes. Even Github's syntax highlighter is catching this, and jq refuses to parse it at all.

They should be like this:

[  {    "Filename": "MyFont.otf", } ]

The culprit seems to be here:

https://github.com/googlefonts/pyfontaine/blob/397c0bbe4dc86b0c721d7db7699a6c385945c377/fontaine/builder.py#L308

This is just outputting a Python array structure, it is not actually a JSON encoder. The result happens to be a rough approximation of JSON but it is not the real thing.

This should be replaced with something that outputs valid JSON. This could be cobbled together with an array iterator and some manual print functions or an existing JSON dump function could be used.

alerque commented 4 years ago

Part II: The JSON is invalid for other reasons as well, for example it outputs null values as blanks

{ 'percentCoverage':        'missingValues': "", }

Even having manually corrected the string quoting style, this key has no value, and also no comma separator before the next key.

I'm not opening a separate issue for this because I think #111 will fix it properly, but in the event somebody wanted to manually fix the output instead this case would need to be considered.

alerque commented 4 years ago

Thanks for the merge @felipesanches.

Will there be a release tag for this any time soon or should I look into how to hake the git head a dependency?

felipesanches commented 4 years ago

please open an issue asking for a new release and I may do so (asap)