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

Clarify glyph name ranges (2.g.i) #1221

Merged
merged 2 commits into from
Sep 9, 2020

Conversation

PeterCon
Copy link
Contributor

@PeterCon PeterCon commented Sep 4, 2020

The current description of glyph name ranges is unclear in a few critical points.

Fixes #1222

Description

This is a proposed enhancement to the feature file spec documentation. The current
wording in section 2.g.i regarding ranges is unclear. At several points, a reader can be
left scratching their heads wondering what was meant. This is intended to provide
greater clarity in that section.

Checklist:

  • [X ] I have followed the Contribution Guidelines
  • I have added test code and data to prove that my code functions correctly
  • I have verified that new and existing tests pass locally with my changes
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

The current description of glyph name ranges is unclear in a few critical points.
@frankrolf
Copy link
Member

The new examples added imply that glyph ranges can be given without spaces (such as A-Z).
This is no longer the case. A glyph range must be specified A - Z (with spaces), while A-Z is interpreted as a glyph name.

The reason for this change was a font editor’s decision to allow glyph names such as be-cyr.

@PeterCon
Copy link
Contributor Author

PeterCon commented Sep 5, 2020

No examples were added.

@frankrolf
Copy link
Member

No?
image

@PeterCon
Copy link
Contributor Author

PeterCon commented Sep 5, 2020

Those are not (intended as) examples of usage or syntax specification but rather are simply describing the characters that are significant for this mechanism. The syntax is stated after this paragraph, and is clear about requiring spaces.

Revising the proposed change to description of ranges to avoid potential confusion between the prose description of the mechanism and the syntax required for the mechanism.
@PeterCon
Copy link
Contributor Author

PeterCon commented Sep 5, 2020

Revised the change to avoid potential confusion as suggested by @frankrolf .

@frankrolf
Copy link
Member

Thanks!

@khaledhosny
Copy link
Collaborator

A-Z is interpreted as a glyph name only if the font has a glyph with that name, otherwise it is interpreted as a glyph range.

@frankrolf
Copy link
Member

I tested my statement with a feature file that has

sub A-Z by A.sc-Z.sc;

and received the complaint “font does not contain a glyph named A-Z.“
At any rate, I think it’s better to stay consistent?

EDIT: just tried again:

makeotfexe [ERROR] <Test> Glyph "A-Z" not in font. [/Users/fgriessh/Desktop/test/features.fea 2]
makeotfexe [ERROR] <Test> Glyph "A.sc-Z.sc" not in font. [/Users/fgriessh/Desktop/test/features.fea 2]
makeotfexe [FATAL] <Test> aborting because of errors

@khaledhosny
Copy link
Collaborator

What I said is described in https://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#2fi-glyph-name:

In order disambiguate whether a text token is a range argument or a glyph name which contains a hyphen, the implementation should first assume that the token is a glyph name. If there is no glyph in the source data with a name that matches the token, then the implementation should interpret the token as a range argument.

If makeotf does not actually implement this, then text should be removed from the spec (this would also help simplifying FontTools’ feaLib implementation, which requires a font or glyph list just to disambiguate glyph names as described above).

@khaledhosny
Copy link
Collaborator

khaledhosny commented Sep 7, 2020

I tested my statement with a feature file that has

sub A-Z by A.sc-Z.sc;

and received the complaint “font does not contain a glyph named A-Z.“

Ah, yes. There is no ambiguity here as a range can’t possibly work. The range needs to be in a glyph class:

sub [A-Z] by [A.sc-Z.sc];

@frankrolf
Copy link
Member

Thanks for the clarification! I agree allowing both notation formats creates ambiguity.

@PeterConstable
Copy link

Just noting: the issue you're discussing is orthogonal to the one my PR addresses.

@frankrolf
Copy link
Member

That’s correct, however I think the change you made is good (thanks again!).
I will leave the acceptance of the PR up to higher powers, since I don’t want to overrule whatever concerns others may have.
(Since it’s labor day in the US today, we probably won’t get another reaction until tomorrow).
Thanks for your patience.

Copy link
Collaborator

@josh-hadley josh-hadley left a comment

Choose a reason for hiding this comment

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

👍 Thank you!

@josh-hadley josh-hadley merged commit 728284b into adobe-type-tools:develop Sep 9, 2020
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.

[fea syntax doc] description of glyph name ranges is unclear
5 participants