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

Builder subset operation: exclude glyphs by name/codepoint/file #1020

Merged
merged 7 commits into from
Sep 11, 2024

Conversation

RickyDaMa
Copy link
Contributor

@RickyDaMa RickyDaMa commented Aug 21, 2024

Adds support both on commandline and through YAML for glyphs to be excluded by either their name or codepoint.

All of the following now work (and yes, they would work together if something possessed you to do so):

# -- snip --
subsets:
  - from:
      repo: myorg/myfont
      path: source/myfont.designspace
    force: true
    name: GF_Latin_Core
    # ✨ new ✨ 👇
    exclude_codepoints: U+0032 U+0033  # space delimited
    exclude_codepoints_file: exclude_codepoints.txt
    exclude_glyphs: space a b c  # space delimited
    exclude_glyphs_file: exclude_glyph_names.txt

Usage docs for the CLI:

options:
  --exclude-codepoints EXCLUDE_CODEPOINTS
                        Space-delimited unicodes to exclude
  --exclude-codepoints-file EXCLUDE_CODEPOINTS_FILE
                        Newline delimited file with unicodes to exclude. Allows for comments with either #
                        or //
  --exclude-glyphs EXCLUDE_GLYPHS
                        Space-delimited glyph names to exclude
  --exclude-glyphs-file EXCLUDE_GLYPHS_FILE
                        Newline delimited file with glyph names to exclude. Allows for comments with
                        either # or //

The file format I've tried to be considerate/flexible: it's newline delimited, allowing either // or # style comments (both in-line and on thier own line)


Considerations:

Happy to hear thoughts, feedback, emotions

Based on logic from ufo_merge
@simoncozens
Copy link
Contributor

I've just fixed the ufomerge bug and am happy with this if you are - ready for review?

@RickyDaMa
Copy link
Contributor Author

RickyDaMa commented Sep 10, 2024

I don't think the failing test runs are on me, but let me know if I'm mistaken there

Pushed documentation on the subset operation (see it rendered here)

I wanted to pin ufomerge to >1.8.0* so we don't pull in the bugged version to gftools, but as far as I can see gftools doesn't even have an explicit dependency on ufomerge? If so, I think we should add it

(*note: the fix has not yet been released)

@RickyDaMa RickyDaMa marked this pull request as ready for review September 10, 2024 11:00
@simoncozens
Copy link
Contributor

Yes, please add a dependency on ufomerge>=1.8.1

@Hoolean
Copy link
Contributor

Hoolean commented Sep 11, 2024

@simoncozens Thanks Simon, all pinned now: 📌 1e0521a 📌

@simoncozens
Copy link
Contributor

Nice, thank you!

@simoncozens simoncozens merged commit cf3e553 into googlefonts:main Sep 11, 2024
1 check passed
@Hoolean Hoolean deleted the exclude-glyphs-subset branch September 11, 2024 17:06
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.

3 participants