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

relative include statements in UFO's features.fea #55

Closed
anthrotype opened this issue Feb 21, 2018 · 10 comments
Closed

relative include statements in UFO's features.fea #55

anthrotype opened this issue Feb 21, 2018 · 10 comments

Comments

@anthrotype
Copy link
Member

anthrotype commented Feb 21, 2018

The UFO spec says

If a feature file is present, it must be self-contained; for example, any glyph or mark classes must be defined within the file.

does that mean that references to external feature files should not be allowed? e.g. include(../features/MyFont.fea)

And if they are allowed, and they use a relative path, how should they be interpreted? Relative to the features.fea file itself (contained in the UFO subfolder) like fontmake is doing, or relative to the UFO folder itself?

Could/should the UFO spec be clearer/more explicit about this point, or is it out of scope?

See googlefonts/fontmake#157

/cc @typesupply @readroberts

@typesupply
Copy link
Contributor

does that mean that references to external feature files should not be allowed?

No. Those are allowed. That clumsy phrasing was in reference to editors inserting classes at compilation time using proprietary techniques. This led to UFOs not having the necessary data to compile the font outside of a specific environment. This rule may be too blunt. My own FeaPy syntax may be in violation of this rule, but it's publicly documented (and super useful).

And if they are allowed, and they use a relative path, how should they be interpreted? Relative to the features.fea file itself (contained in the UFO subfolder) like fontmake is doing, or relative to the UFO folder itself?

Relative to the UFO itself.

Could/should the UFO spec be clearer/more explicit about this point, or is it out of scope?

Yes, absolutely. Any suggestions would be greatly appreciated.

anthrotype added a commit to anthrotype/fonttools that referenced this issue Feb 21, 2018
subclass of FeatureLibError, only raised if IOError.errno == ENOENT (i.e. FileNotFoundError)

googlefonts/fontmake#157 (comment)

Will be useful in ufo2ft to detect when an included feature file doesn't exist and print a nicer error message
explaining that includes must be relative to the UFO itself, and not relative to the embedded features.fea file.

unified-font-object/ufo-spec#55
@justvanrossum
Copy link
Contributor

See also googlefonts/fontmake#157

@moyogo
Copy link
Collaborator

moyogo commented Jul 24, 2018

Fixed by #66.

@moyogo moyogo closed this as completed Jul 24, 2018
@simoncozens
Copy link

I want to question (for the upcoming meeting) the idea that relative includes ought to be relative to the surrounding directory by default.

Suppose I have two masters; some of the feature code is shared between them, but some of the feature code has positioning information in and has to be master-specific. So here is my directory:

Foo Thin.ufo/
Foo Thin.ufo/features.fea
Foo Thin.ufo/anchors.fea

Foo Thin.ufo/
Foo Bold.ufo/features.fea
Foo Bold.ufo/anchors.fea

gsub.fea

I would love my features.fea file to contain:

include(../gsub.fea);
include(anchors.fea);

for both cases - that is, relative to the directory "inside" the UFO. But because it is relative to the surrounding directory, I have to do this:

# Foo Thin.ufo/features.fea:
include(gsub.fea);
include(Foo Thin.ufo/anchors.fea);

# Foo Bold.ufo/features.fea:
include(gsub.fea);
include(Foo Bold.ufo/anchors.fea);

This strikes me as being the wrong way around. If you consider the UFO as being a "package" (which of course it is), then relative references should be to files inside the package.

@alerque
Copy link

alerque commented Jul 15, 2020

Definitely second that, the anchors should not be relative to the parent scope — that adds recursive data inside the file. Say you changed the name of the UFO package: you shouldn't have to go through and change a bunch of references inside itself to keep referencing itself.

@typemytype
Copy link
Contributor

typemytype commented Jul 15, 2020

there is no such thing as Foo Bold.ufo/anchors.fea

a UFO can only contain filename.ufo/feature.fea, your other .fea files will not be copied or saved in a save as operation when stored inside the UFO package.

@simoncozens
Copy link

That's certainly true at present. But as it becomes more common to have to deal with multiple masters with this kind of common/specific split, I wonder if it should be true in the future. Anyway, that's the point of having a discussion.

@benkiel
Copy link
Contributor

benkiel commented Jul 16, 2020

@simoncozens I'm still not seeing why this is needed.

Your concern is with the separation of UFO (or master) specific feature code from code that can be shared by a set of UFOs. This was addressed in a clean way above: your UFO specific code goes into feature.fea, the shared code is then included with a clear include(gsub.fea).

The current behavior also address @alerque's concern of UFOs changing name.

It seems your concern is with being able to further split local feature code into multiple files. If this is something that you want further discussion of, please open a new issue to solicit feedback on that, it's going to get buried on this closed issue.

@typesupply
Copy link
Contributor

I started to write some the rationale for why I made the rule what it is, but I agree with Ben: would you mind opening a new issue and tag me? You raise some interesting points that I think pair with something that I have on the agenda. It would be good to sketch out some of those thoughts as they relate to your issue. Thanks!

@khaledhosny
Copy link

One can add anchors.fea to the data directory, and then include (data/anchors.fea); should work, but with the current rule it wouldn’t.

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

No branches or pull requests

9 participants