search

googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
152 stars 43 forks source link

Refactor argument passing #533

Closed simoncozens closed 3 years ago

simoncozens commented 3 years ago

I've always found ufo2ft hard to follow, because so much of the main code is dedicated to passing arguments to pre/post/outline-processor classes. It's just a horrible mess of foo=foo, bar=bar which obscures the differences and commonalities between each function. And if a new argument needs adding, it has to get copy-and-pasted in everywhere.

This PR refactors argument passing using the **kwargs dictionary. This also allows refactoring out common calls to the pre/post/outline-processors, which I've done.

behdad commented 3 years ago

Nice work!

I think inherit_dict should be inlined. Instead of, eg:

        otf = postProcessor.process(**inherit_dict(kwargs, **overrides))

just write:

        otf = postProcessor.process(**{**kwargs, **overrides})

filter_kwargs I have opinions about as well. Currently, unknown arguments are ignored. Was that intended? Other than that, my understanding is that filter_kwargs is just filling in default argument values. Instead, I suggest using .get default values. Eg, instead of

  kwargs["preProcessorClass"]

write:

  kwargs.get("preProcessorClass", 'OTFPreProcessor')

in the case that uses OTFPreProcessor as default.

simoncozens commented 3 years ago

Yes, the point of filter_kwargs is that it drops unknown arguments, so the function only gets stuff it cares about.

belluzj commented 3 years ago

Hello! I'm starting to implement a configuration utility in fontTools, and I would like to use my draft code (at https://github.com/fonttools/fonttools/pull/2416) in ufo2ft. I think I should work on top of this PR, because I'll be adding an argument for the config object, or I'll be populating the fontTools config object with some of the ufo2ft arguments, and this PR is the way forward for argument passing. Is that right?

anthrotype commented 3 years ago

silently ignoring arguments is what makes me uncomfortable about using **kwargs, it risks opening the door for typos or other bugs. E.g. compileTTF(font, bananas=123) will no longer raise TypeError as before.

anthrotype commented 3 years ago

let's continue on https://github.com/googlefonts/ufo2ft/pull/544