Repository navigation
[pigeon] Adds StructuredGenerator class and subclasses - #3037
Merged
Merged
Conversation
gaaclarke
approved these changes
Jan 12, 2023
gaaclarke
left a comment
gaaclarke
left a comment
Member
There was a problem hiding this comment.
LGTM, I have a few nits below. I also think the namespace code could have space in the abstract base class and that encode/decode implementations are empty but that could be addressed later if we want.
Collaborator
Sorry I didn't get a chance to do this today; I started but didn't get very far. Hopefully it wasn't too painful to integrate my change. |
stuartmorgan-g
approved these changes
Jan 13, 2023
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
Collaborator
There was a problem hiding this comment.
I have a few small comments, and one larger comment for later consideration, but overall this looks great. Having this coherent structure will be a really big win for maintainability even before we potentially start extracting more shared logic.
I also really, really like having the checked in output files for this kind of PR. Being able to see the (trivial) output changes makes it really easy to have confidence that nothing got accidentally changed in the reorganizing.
| ) { | ||
| final Indent indent = Indent(sink); | ||
|
|
||
| writeFilePrologue(generatorOptions, root, sink, indent); |
Collaborator
There was a problem hiding this comment.
(At some point I'd like to revisit whether these other things that are passed to literally every method could be instance variables instead, but since they can't be constructor arguments due to the way generate is set up let's have that discussion later so it doesn't block this PR.)
Contributor
Author
There was a problem hiding this comment.
I would prefer to avoid the repeating parameters also
tarrinneal
added
the
autosubmit
creatorpiyush
pushed a commit
to creatorpiyush/packages
that referenced
this pull request
Jun 10, 2026
* Rename generator class to Adapter * create new generator class and dart subclass * cpp and dart test gen * added files * Adds Generator class to all generators * adds swift * Updates tests to use new Adapter naming scheme * Dart generate methods * convert all generate functions to use new method * chagngelog * remove Generator class fields * move paths to options * remove dartTestOptions * Moves write header to generator class method * Updates tests to use new generator class * source -> header * correct options * header -> source * header -> prefix, source -> header * remove headers from generateTestDart * changelog * Nits and combines source and header generators * renames Adapter to GeneratorAdapter * Update version number for breaking changes * nits * more personal nits * update tests to match new merged generators * cleaner header methods * Fixes dart header bug * add gen files for clarity * better field naming * better field naming * removed unneeded dart test generator * Add filetype to generator * Adds filetype as field to generatorAdapters * merge * analyze * add import method * re-remove DartTestGenerator * Moves imports to new method * adds writeEnum method to generator class * nits * assert * objc enum * fix code order issues * add writeDataClass method * remove writeMainClass from java * java + kotlin * remove dead code * swift * fix dart test error * cpp + objc * objc + cpp * Move all migrated methods into class * Creates writeHeader method on Generator classes * private unique methods and reorder * changelog * changelog * changelog * changelog * prologue * gen * dart * java * kotlin * swift * cpp * objc * remove unneeded java method * analyze * single file gens * objc * Cpp * analyze * vocab * vocab * typo * less comma * wrap write methods * move code from writeGeneralUtilities * update changelog * adds open and close namespace and fixes nits * makes writeEnum optional * remove unneeded sink * remove unneeded namespace from guardname * [indent] * clean up
bisor0627
pushed a commit
to bisor0627/packages
that referenced
this pull request
Jun 19, 2026
* Rename generator class to Adapter * create new generator class and dart subclass * cpp and dart test gen * added files * Adds Generator class to all generators * adds swift * Updates tests to use new Adapter naming scheme * Dart generate methods * convert all generate functions to use new method * chagngelog * remove Generator class fields * move paths to options * remove dartTestOptions * Moves write header to generator class method * Updates tests to use new generator class * source -> header * correct options * header -> source * header -> prefix, source -> header * remove headers from generateTestDart * changelog * Nits and combines source and header generators * renames Adapter to GeneratorAdapter * Update version number for breaking changes * nits * more personal nits * update tests to match new merged generators * cleaner header methods * Fixes dart header bug * add gen files for clarity * better field naming * better field naming * removed unneeded dart test generator * Add filetype to generator * Adds filetype as field to generatorAdapters * merge * analyze * add import method * re-remove DartTestGenerator * Moves imports to new method * adds writeEnum method to generator class * nits * assert * objc enum * fix code order issues * add writeDataClass method * remove writeMainClass from java * java + kotlin * remove dead code * swift * fix dart test error * cpp + objc * objc + cpp * Move all migrated methods into class * Creates writeHeader method on Generator classes * private unique methods and reorder * changelog * changelog * changelog * changelog * prologue * gen * dart * java * kotlin * swift * cpp * objc * remove unneeded java method * analyze * single file gens * objc * Cpp * analyze * vocab * vocab * typo * less comma * wrap write methods * move code from writeGeneralUtilities * update changelog * adds open and close namespace and fixes nits * makes writeEnum optional * remove unneeded sink * remove unneeded namespace from guardname * [indent] * clean up
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds StructuredGenerator class and subclasses
fixes flutter/flutter#117416