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

Always retain the default layer #342

Merged
merged 3 commits into from
Mar 11, 2024
Merged

Always retain the default layer #342

merged 3 commits into from
Mar 11, 2024

Conversation

madig
Copy link
Collaborator

@madig madig commented Mar 11, 2024

Previously, you could drop the default layer if it had been renamed.

Code suggested by @RickyDaMa :)

@madig madig requested a review from RickyDaMa March 11, 2024 12:16
src/layer.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@RickyDaMa RickyDaMa left a comment

Choose a reason for hiding this comment

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

Not disagreeing necessarily with Cosimo, but given that currently norad upholds the invariant of the default layer being first, this fix works

@anthrotype
Copy link
Collaborator

The correct fix that doesn't depend on any specific layer name or ordering is to treat as default the layer whose path points to the default glyphs directory

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

looks good, thanks!

@cmyr cmyr merged commit 18a1f8e into master Mar 11, 2024
4 checks passed
@madig madig deleted the fix-default-layer-retain branch March 11, 2024 19:30
@madig madig changed the title Always retain the first (default) layer Always retain the default layer Mar 11, 2024
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.

4 participants