Skip to content

Rework parser and writer (G3 prep)#690

Merged
madig merged 4 commits intogooglefonts:mainfrom
simoncozens:rework-parser-writer
Aug 5, 2021
Merged

Rework parser and writer (G3 prep)#690
madig merged 4 commits intogooglefonts:mainfrom
simoncozens:rework-parser-writer

Conversation

@simoncozens
Copy link
Collaborator

I'm going to do the Glyphs 3 branch again, now I know what I'm doing.

This PR is preparatory work for Glyphs 3 support, but there is nothing G3 specific about it at this stage. It consists of three commits, each of which passes tests independently when applied in sequence.

  1. The first commit replaces the existing parser with one which uses openstep-plist. It has a facility which allows classes to deserialise their properties.
  2. The second adds a facility to the writer allowing classes to serialize themselves, and then uses this to write out the data "by hand" instead of using magic properties (_classesForName/shouldWriteValueForKey/_keyOrder). This seems like a backwards step, but it really helps when adding Glyphs 3 support.
  3. The third removes those properties which were there to support the automagic serialization.

@simoncozens simoncozens force-pushed the rework-parser-writer branch from 9df67ab to 2faf779 Compare June 28, 2021 09:54
@simoncozens
Copy link
Collaborator Author

Well, the first commit is a bad rebase, but the other three are as described...

_classesForName, _wrapperKeysTranslate, shouldWriteValueForKey, _keyOrder
@simoncozens simoncozens force-pushed the rework-parser-writer branch from 2faf779 to 572f7ba Compare June 28, 2021 12:26
@simoncozens simoncozens mentioned this pull request Jun 28, 2021
"showMeasurement": bool,
"filter": str,
"name": str,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy to see this stuff gone!

@jenskutilek
Copy link
Contributor

I don't think this is G3-specific, but TrueType hints can have 3 values in origin and target. That happens when the hinted point is one that will only be inserted when converting to TrueType, like an inflection point.

Here's an example file that fails when trying to parse the hints: S.glyphs.zip

@simoncozens
Copy link
Collaborator Author

The hints in your file don't parse using the main branch of glyphsLib, even if I convert it to Glyphs 2 file format, so the fact that the hints don't parse is not specific to this PR. I think what you're saying is a duplicate of #446.

@jenskutilek
Copy link
Contributor

Right! Yes, they didn’t ever parse. I just thought you might want to fix it for G3 files :)

@simoncozens
Copy link
Collaborator Author

Sure. Happy to fix for G2 and G3, but I'd rather do things in separate steps to help reviewers; the "rejigging the parser" component is big enough to be a single PR by itself, we can fix unconnected open issues later...

@madig madig merged commit 572f7ba into googlefonts:main Aug 5, 2021
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.

4 participants