Skip to content

[glyphs] Add support for localized font properties#1705

Merged
anthrotype merged 3 commits into
mainfrom
fontc-localized-properties
Oct 21, 2025
Merged

[glyphs] Add support for localized font properties#1705
anthrotype merged 3 commits into
mainfrom
fontc-localized-properties

Conversation

@anthrotype

Copy link
Copy Markdown
Member

meant to match googlefonts/glyphsLib#1108

Fixes #1695

Comment thread glyphs-reader/src/font.rs Outdated
pub virtual_masters: Vec<BTreeMap<String, OrderedFloat<f64>>>,
pub features: Vec<FeatureSnippet>,
pub names: BTreeMap<String, String>,
pub localized_names: BTreeMap<String, Vec<(String, String)>>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggest a comment or two about the relationship between names and localized names. Immediate questions that pop into mind for me:

  • is names the english subset of localized names (equivalent to localized_names['some key']`?)
  • Why is the type of names not the same as the value type for localized names?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i'm not super happy about the distinction between bare names and localized_names myself, I did that initially to keep diffs to a minumum but I'll see if we can have one map to for all the names, English or not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Naively, perhaps it becomes .names() that just returns a value from localized_names? Or does it not fit together that way?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, I think that should work. thanks

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I unified the two maps in 5542ca3

Comment thread glyphs-reader/src/font.rs
Comment thread resources/testdata/glyphs3/LocalizedNames.glyphs
- Refactor assert_fonts_equal_sans_localized_names() for better maintainability
- Add test case demonstrating duplicate language code handling (we pick the last matching glyphsLib which uses python dict assignment semantics)
@anthrotype anthrotype force-pushed the fontc-localized-properties branch from 25b9701 to c7e73b6 Compare October 20, 2025 17:29

@cmyr cmyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks like this gets us back a bunch of name table matches :)

merge Font.names and Font.localized_names into all_names: BTreeMap<String Vec<(lang, value)>>

Add helper methods to extract/iterate over default vs localized names,
using consistent dflt > default > ENG > first priority for default names
for default names.

Use IndexMap when deduplicating inside update_names to preserve insertion order,
which is critical for correct no-english-names fallback behavior (added
an explicit test for that one).
@anthrotype anthrotype added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit e893fec Oct 21, 2025
12 checks passed
@anthrotype anthrotype deleted the fontc-localized-properties branch October 21, 2025 17:13
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.

new localized properties support in glyphsLib 6.12 causes name table diffs in crater

3 participants