[pigeon] Creates new Generator classes for each language (v3) #3022
Conversation
| /// Generates Cpp files with specified [CppOptions] | ||
| @override | ||
| void generate(CppOptions languageOptions, Root root, StringSink sink, | ||
| {required FileType fileType}) { |
There was a problem hiding this comment.
Yea, this looks good to me. I'd just pull fileType into the CppOptions since it isn't applicable to all Generators.
There was a problem hiding this comment.
It applies to two already, and will apply to three when we add Linux, so whatever solution we have shouldn't be specific to C++.
If we put it in options, we won't be able to pass the same immutable options to each generate call, which undermines the idea that all calls to generate need to happen with identical options (so that the generated code matches).
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
LGTM thanks. I think this is much better than where we were before vacation. Now at every step of the calculation there isn't superfluous input to any of the procedures. No "ignore this parameter since it isn't applicable" and the input to the procedures is limited in scope to just what the generator needs (for the most part). Thanks so much for looking into this Tarrin.
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
One small naming issue, but otherwise LGTM.
tarrinneal
added
the
autosubmit
3 participants
Slightly modified version of #2996.
Adds NA option to FileType Enum. Uses it as default value in (newly) name parameter - fileType - in relevant Generators.
Creates new Generator superclass and new language specific subclasses.
Also renames former Generator classes to GeneratorAdapter and updates tests accordingly.
Initial set up for flutter/flutter#117416