search

googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

update GlyphData from Glyphs 3.2 #959

Closed schriftgestalt closed 4 months ago

schriftgestalt commented 7 months ago

Putting that up to see what the test are doing ;).

khaledhosny commented 7 months ago

A few tests need to be updated and most of the regression tests failures look like improvements, except may be this one?

 ! tests/data/gf/DotGothic16/DotGothic16-Regular.ufo/lib.plist - /tmp/tmpzlyzjnmk/DotGothic16-Regular.ufo/lib.plist--- 
+++ 
@@ -11190,9 +11190,9 @@
       <key>micro.rotat</key>
       <string>uni00B5.rotat</string>
       <key>middledot-kata.half</key>
-      <string>uniFF65</string>
+      <string>middledotkata.half</string>
       <key>middledot-kata.half.rotat</key>
-      <string>uniFF65.rotat</string>
+      <string>middledotkata.half.rotat</string>
       <key>miriSquare</key>
       <string>uni3349</string>
       <key>miriSquare.vert</key>
schriftgestalt commented 7 months ago

There was an altName in that is not in my data. there is one more thing that probably needs updating code in more places. The case information moved from subcategory="Upper/Lowercase" to case="upper/lower". Not sure if that is used anywhere.

khaledhosny commented 7 months ago

There was an altName in that is not in my data. there is one more thing that probably needs updating code in more places. The case information moved from subcategory="Upper/Lowercase" to case="upper/lower". Not sure if that is used anywhere.

I don’t think glyphsLib uses this. It only cares about GDEF, so Mark/[Nonspacing,Spacing Combining], and */Ligature: https://github.com/googlefonts/glyphsLib/blob/3554be43854fbfb3ccc6ade0d58e542f153748d4/Lib/glyphsLib/builder/features.py#L202-L219

schriftgestalt commented 7 months ago

It does seem to look at the altName. That way it can connect the glyph name "middledot-kata.half" to the info entry "dot-kata.half" and get its production name.

It could have used the fallback to search with the unicode, but that doesn't seem to work.

markhdavid commented 5 months ago

Can anyone summarize where this stands, and if anything is holding this PR up?

anthrotype commented 5 months ago

Was this copied from the files in https://github.com/schriftgestalt/GlyphsInfo? Usually we write the commit hash in the commit message when we update these files to know where they came from. Can you do that? Also there seem to be merge conflicts.

markhdavid commented 4 months ago

Was this copied from the files in https://github.com/schriftgestalt/GlyphsInfo? Usually we write the commit hash in the commit message when we update these files to know where they came from. Can you do that? Also there seem to be merge conflicts.

@schriftgestalt ping...

schriftgestalt commented 4 months ago

The regression tests need to be updated. There are a lot fails with the production name of the brevecomb-cy. It was brevecombcy and now is uni0306.cy. Not sure how to do that in CI.

So it needs to run tests/tools/generate_regression_test_files.py on with the latest GlyphData.

anthrotype commented 4 months ago

The regression tests will fix themselves automatically after you merge the PR

markhdavid commented 4 months ago

@schriftgestalt are you able to take any action to move this forward? E.g., along the lines of @anthrotype's comment https://github.com/googlefonts/glyphsLib/pull/959#issuecomment-1950032449 ?

schriftgestalt commented 4 months ago

I can’t do anything else on this. As I understand cosimos comment is that the test will work after it is merged?

anthrotype commented 4 months ago

As I understand cosimos comment is that the test will work after it is merged?

i think so, let's see

anthrotype commented 4 months ago

the regression tests passed