Expand Glyphs multiple languages syntax when building UFOs

[tomekthewo]

Fixes #1109.

Glyphs lets a single language statement carry several tags and applies everything that follows to each of them:

language AZE CRT KAZ TAT TRK;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;

The FEA spec allows exactly one tag, so feaLib stops at the second one and the UFOs glyphsLib writes cannot be compiled at all — <features>:N:14: Expected ';', column 14 being where CRT starts. The syntax is documented in the Glyphs Handbook.

This expands the shorthand into one statement per tag in _to_ufo_features, right next to the variable feature conversion. Doing it there rather than in feaLib means what reaches the UFO is plain spec-compliant FEA that any compiler reads — feaLib, fea-rs and makeotf alike; expanding downstream would leave the UFO itself non-portable.

The block is emitted once for the first tag and repeated for every remaining one. Named lookups cannot simply be duplicated — that would redefine them — so the repeats reference the lookup defined under the first tag:

language AZE;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;
language CRT;
lookup idotaccent;

Glyph class and markClass definitions are likewise emitted only once; bare rules and lookupflags are repeated verbatim, which is what the shorthand means. Trailing keywords (exclude_dflt and friends) are kept on every tag. A statement that is already spec-compliant, or that cannot be parsed with confidence, is left untouched for feaLib to report rather than guessed at.

Scanning is done on a copy with comments and string literals blanked out — reusing _blank_comments_and_strings from variable_features.py — so a brace, semicolon or keyword inside a comment is never taken for code. That matters more than it sounds: a } in a comment would otherwise end the block early and silently bind the following rules to the last tag only, producing output that parses cleanly and ships the wrong locl mapping.

variable_features.py was the model for the placement, the shape of the module and the scanning approach.

One-way. The expansion is not reversed on the way back, so .glyphs → UFO → .glyphs yields the expanded form rather than the original statement. test_expansion_does_not_round_trip pins this down. UFO → .glyphs → UFO is unaffected, since the original feature text is recovered from ORIGINAL_FEATURE_CODE_KEY. Happy to restore the shorthand in to_glyphs_features instead if you would rather keep it lossless.

18 tests in tests/builder/multi_language_test.py, including one through to_ufos and one through the round trip. Full suite: 1355 passed, 64 skipped, 4 xfailed. black --check and flake8 over Lib tests are clean.

[khaledhosny]

Test the whole output against an expectation. Counting matching lines does not guarantee the output is as expected.

[tomekthewo]

Good point, done in b354bdb — every case now compares the full expanded text, which doubles as documentation of what the expansion produces. It caught nothing new, but it is a much better regression net: the counting assertions would not have noticed the order of the emitted statements. The two round-trip tests assert the whole feature text as well.

[anthrotype]

So.. I’m not convinced a physical line is the right unit for this transformation. FEA defines line endings as ordinary whitespace therefore valid statements may span lines or share one.

Unlike with the existing variable_features.py, expanding multiple languages cannot operate on locally delimited syntax because it needs to understand the type and scope of the following FEA statements, which means reconstructing statement boundaries, nesting, definitions and lookup references, all through a bunch of fragile regexes...

E.g. this would be valid Glyphs syntax that the current code leaves unexpanded because another statement follows on the same line:

language AZE CRT; sub i by idotaccent;

Or take this one:

language AZE CRT;
lookup idot { sub i by idotaccent; } idot;
sub Scedilla by Scommaaccent;

here the scanner can't detect a lookup's opening and closing braces that occur on the same line (brace depth is 0) and treats the lookup as continuing to the end of the language section, so CRT only receives lookup idot; while the inline sub rule gets silently ignored.

There are more of these all stemming from conflating line with statements. E.g. @Ced = [Scedilla]; sub @Ced by Scommaaccent; would be classified as one definition and the sub not replayed. Or line break after lookup or markClass would prevent the definition from being recognized.

Fixing these (or even just rejecting them with a loud error to avoid silent wrong output) risks turning this into a partial second FEA parser inside glyphsLib, needing knowledge of lookup, class/markClass semantics. Besides, this operates on incomplete feature text before includes gets resolved, so there's that too.

So I’m not yet convinced glyphsLib can transform the extension safely without duplicating parts of feaLib. And maybe feaLib would not necessarily be the wrong layer, despite not being official FEA... I honestly don't know, sorry for being inconclusive.

[anthrotype]

(and then there is the question of also needing to implement this in fontc/fea-rs which just hit 1.0 this week.. at some point I would rather stop introducing new features in both compilers)

[tomekthewo]

You're right, and the line-based scanning isn't salvageable. Thanks a lot for writing the counterexamples out — all four reproduced exactly as you described, including the two that fail silently.

I've rewritten the expansion on feaLib's own Lexer instead of on the text:

input	before	now
language AZE CRT; sub i by idotaccent;	not expanded	expanded
lookup idot { … } idot; closed on one line	CRT lost the rule that followed	both statements replayed
@Ced = [Scedilla]; sub @Ced by Scommaaccent;	CRT got nothing	rule replayed, class defined once
line break after lookup	block duplicated → redefinition	replayed as lookup idot;

The part I'd gently push back on is the "partial second FEA parser" worry — tokenizing is precisely what removes the need for one. Lexer already gives me statement boundaries (a ; at brace depth zero, wherever it sits on the page), and it already tells a brace inside a comment or a string from a real one. What's left is classifying the head of each statement, and that grammar is five cases:

• lookup NAME [useExtension] { → definition: emitted once, replayed as lookup NAME;
• @Class = … → definition: emitted once
• markClass … → definition: emitted once
• include(…) → bail out (below)
• anything else, lookup NAME; references included → repeated verbatim

with the scope ending at the next language/script or an unmatched }. No glyph classes to resolve, no lookup bodies to understand, no name resolution. The emitted text is sliced out of the original by offset, so comments, indentation and blank lines survive untouched.

TokenExpander in builder/tokens.py already runs inside _to_ufo_features and rewrites Glyphs-only $[…] / ${…} / $name syntax into plain FEA — with a hand-rolled predicate subparser and an eval() for the arithmetic. In my opinion, it is more Glyphs-specific machinery than the five heads above. I know those tokens are locally delimited and the shorthand isn't — that's a fair distinction. My point is only that lexing moves that line: on a token stream a statement boundary is as local as the } that closes a ${.

On includes: agreed, they can't be replayed, so a multi-tag scope containing one is left alone entirely. The shorthand is invalid FEA, so feaLib then reports it — the failure stays loud instead of becoming a silently partial expansion.

What this does not address is your second comment about fontc parity, because it still fails unless the same expansion is implemented there too. The portability argument in my PR description ("the UFO gets plain FEA, so any compiler reads it") doesn't cover fontc, and I shouldn't have framed it that broadly. The parity cost you're describing is real and this patch doesn't remove it.

One thing that might change the calculus: Glyphs 4 emits it from automatic feature generation. I took two of our Glyphs 3 sources (appVersion 3517 and 3526), opened and saved each in Glyphs 4 (appVersion 4004), and in both the automatic locl came back rewritten the same way:

# before, appVersion 3517
language AZE;
lookup locl_latn_0 {
    sub i by idotaccent;
} locl_latn_0;
language CRT;
lookup locl_latn_0;
language KAZ;
lookup locl_latn_0;
...

# after, appVersion 4004
language AZE CRT KAZ TAT TRK;
lookup idotaccent {
    sub i by idotaccent;
} idotaccent;

No edits by the designer, the feature still flagged automatic before and after, and both fonts went from ten single-tag statements to two single plus the same two shorthand ones (AZE CRT KAZ TAT TRK and ROM MOL). So this isn't an opt-in extension that some sources happen to use — it's ordinary Glyphs 4 output, and a source that built yesterday stops building after a save, in feaLib and fea-rs alike. Two fonts against one Glyphs 4 build is still not a survey, but it reproduces rather than looking like a fluke.

As I read it, there are three ways this could go:

1. Token-based expansion as described — I can push it to this branch.
2. Detect and reject, no expansion. A language statement with more than one tag becomes a clear error instead of Expected ';' pointing at column 14. Needs no statement semantics at all, needs no fontc counterpart, and is strictly better than today.
3. Drop it — it belongs in feaLib/fea-rs, or nowhere.

I can only guess which one you'd prefer. But if Glyphs emits the shorthand on save, everything that reads .glyphs sources ends up having to solve it, and for the UFO path this is the one place to do it once — fontc would still need its own, which is the cost you flagged, though I don't think that argues for nobody doing it.

Happy to push (1) here, or cut it back to (2) — whichever you'd merge. For what it's worth on the approach itself: I ran the tokenizer over ~1100 feature code blocks from 35 production .glyphs files and all of them lex cleanly on the raw pre-include text, which is what convinced me it holds up outside the test suite.

[anthrotype]

Happy to push (1) here

thanks, since you have done it already and sounds like it's better than the line-base scanner, you may as well push it

Glyphs 4 emits it from automatic feature generation

oh, great. That means we can't dismiss this and will have to support it in fontc anyway (googlefonts/fontc#2126). And maybe in feaLib as well, which would make the current PR redundant.

[schriftgestalt]

We documented our fea additions. There is a branch for each topic: https://github.com/schriftgestalt/feature_file_workshops (This is not meant as a request to add it to the spec as is, just to have written it down properly.)

[anthrotype]

Thanks Georg, I briefly checked Adobe's upstream repo the other day but couldn't find your multi-lang extension proposal. I found it now here: adobe-type-tools/feature_file_workshops#8