Name type and related API changes #240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Glyph and layer names cannot be arbitary strings. This adds a name module and a Name type that handles validation. This validation is always performed when loading a font. At runtime, we expect the user to ensure that they are using a valid name, since invalid names are generally hard to construct.
madig
reviewed
Jan 10, 2022
madig
reviewed
Jan 10, 2022
madig
reviewed
Jan 10, 2022
madig
reviewed
Jan 10, 2022
madig
approved these changes
Jan 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a
Nametype, shared between glyphs and layers. This enforces the constraints on names documented in the spec and also hides the concrete type we use to represent names, leaving us free to change that in future.This also attempts to standardize the various places where names are used in the API. With this patch, most API that takes a name (for retrieving a glyph or a layer, for instance) now just takes a
&str. In places where we are creating a glyph, this is converted into aName, and panics if the name is malformed. This is unlikely for the reasons outlined at #191 (comment). Methods that may panic in this way are all documented.There are a few exceptions:
NamingErrorwill returnNamingError::Invalidif the name is invalid, since it's easy enough and doesn't increase the user burden.Component::newtakes aNamedirectly, because we expect you to take the name from an existing glyph.There are a few other little tweaks included in this patch, which were related to these changes and would have been annoying to extract.