-
Notifications
You must be signed in to change notification settings - Fork 72
[builder] add --experimental-extra-arg flag to pass down extra arguments to compilers #1106
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
Conversation
… and fontmake this is useful for ttx_diff.py to control e.g. whether to pass the --no-production-names flag to fontc and fontmake, we want to rename glyphs to their final production names by default when comparing fontc vs fontmake output but sometimes it useful to keep original human-readable glyph names for debugging.
since it works with both compilers...
requires change in gftools builder googlefonts/gftools#1106
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks cosimo, not pretty but it should do the trick.
@@ -65,9 +66,8 @@ def modify_config(self, config: dict): | |||
config["cleanUp"] = True | |||
# disable running ttfautohint, because we had a segfault | |||
config["autohintTTF"] = False | |||
# set --no-production-names, because it's easier to debug | |||
extra_args = config.get("extraFontmakeArgs") or "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this already a way to get extra args to the compiler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, that's the way if you start with a config. here ttx_diff wants to override the user config and to pass down some extra command line options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is where gftools builder gets called from ttx_diff.py
we want to inject some extra flag for fontc/fontmake. Are you suggesting that we modify the input yaml before passing it to gftools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that's how I would do it - it would avoid special-case stuff like this. But either way works. So long as you had considered it and thought this was easier...
Revert "Merge pull request #1106 from googlefonts/experimental-extra-args@
In ttx_diff.py we'd like control whether to pass the
--no-production-names
flag down to fontc and fontmake or not..Which means, renaming glyphs to their final production names by default, but sometimes when useful keep original human-readable glyph names for debugging.
I added a catch-all flag which means we won't need extra flags if we'd like to support any additional options.