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

Required is more like a suggestion #304

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Required is more like a suggestion #304

merged 1 commit into from
Jun 29, 2023

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Jun 29, 2023

This updates DesignSpaceDocument to allow inputs where sources do not have names; we will autogenerate names if they are missing.

@rsheeter
Copy link
Collaborator

Spec is wrong, fixed in fonttools/fonttools#3194

pub fn generate_missing_source_name() -> String {
static COUNTER: AtomicU64 = AtomicU64::new(0);
let next_n = COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
format!("unnamed_source_{next_n}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional, we could match the existing authoritative implementation (https://github.com/fonttools/fonttools/blob/main/Lib/fontTools/designspaceLib/__init__.py#L2311), though I cannot think of a reason it would matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

this would be more annoying, since we don't have access to serde's internal state (and hence don't know the actual index of the current source) and so we would need to do a post-processing step to assign the names after loading the whole designspace. 🤷

src/designspace.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@rsheeter rsheeter left a comment

Choose a reason for hiding this comment

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

LGTM with a few optional suggestions

This updates DesignSpaceDocument to allow inputs where sources do not
have names; we will autogenerate names if they are missing.
@cmyr cmyr merged commit 8df630c into master Jun 29, 2023
@cmyr cmyr deleted the optional-dspace-names branch June 29, 2023 16:45
@Hoolean Hoolean mentioned this pull request Jul 21, 2023
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.

2 participants