Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fea-rs] Support 'pos a 20 b' pairpos syntax #992

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Oct 3, 2024

This syntax is not documented, but it is treated as equivalent to 'pos a b 20' by both afdko and feaLib, and exists in at least one noto font.

Opened adobe-type-tools/afdko#1757 to see if this is intended or just works by accident; in either case I think supporting it is expedient.

This syntax is not documented, but it is treated as equivalent to
'pos a b 20' by both afdko and feaLib, and exists in at least one noto
font.
@anthrotype
Copy link
Member

exists in at least one noto font.

which one precisely?

@cmyr
Copy link
Member Author

cmyr commented Oct 3, 2024

@anthrotype
Copy link
Member

hm honestly i've never encountered such a syntax, and also believe it just works by happenstance in feaLib.

image

maybe @simoncozens (who git says authored those files) could tell us why he did this instead of pos na_ahom aaSign_ahom 70;?

@anthrotype
Copy link
Member

looks like feaLib handles this here:

https://github.com/fonttools/fonttools/blob/682d72ab6a12bbdd04b2c37fbacef83501327054/Lib/fontTools/feaLib/parser.py#L742-L754

it looks like it's trying to handle both format A and B in one go, see the if values[0] is None: values.reverse(). In format A, both values are not None, in format B the value following the first glyph is omitted (None) and the one following the second glyph is assumed to refer to the first glyph (hence the reverse()).

But if the value after the first glyph is not omitted (values[0] is not None) and the value for the second one is omitted, then the reverse() is skipped and the values are used in that order for first/second glyphs.

I also can't find an explicit test case for this.

@simoncozens
Copy link
Contributor

simoncozens commented Oct 3, 2024

Yeah, I don't know why that works. :-) The reason I wrote it like that is probably because I can never get my head around the fact that pos foo bar 20 affects the foo, when every other time there's a valuerecord/anchor/lookup it affects the glyph after it. Pair Pos Format B is another one of those unusual language decision decisions: it doesn't really fit with AFDKO syntax but it's how it worked in another format (AFM) so it got grandfathered it in.

Or possibly I just forgot a '.

But either way, the syntax is oddball Cozens brainfart and probably shouldn't be supported.

@khaledhosny
Copy link
Contributor

khaledhosny commented Oct 3, 2024

If the other two implementations support it, I think fea-rs should. AFDKO feature file specification” is essentially user documentation, it often omits things the implementation supports (either intentionally or by oversight).

@rsheeter
Copy link
Contributor

rsheeter commented Oct 3, 2024

@simoncozens noted in IM that this works in Glyphs FeaKit as well. I agree with @khaledhosny, we should probably support this.

@cmyr cmyr added this pull request to the merge queue Oct 3, 2024
Merged via the queue into main with commit 37212d9 Oct 3, 2024
10 checks passed
@cmyr cmyr deleted the fea-funky-pairpos-syntax branch October 3, 2024 19:15
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.

5 participants