-
-
Notifications
You must be signed in to change notification settings - Fork 790
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
Add JsonGeneratorDecorator
to allow decorating JsonGenerator
s
#1051
Conversation
Cool, I like this. Let me think a bit about it, just to be sure, but I think this could work about as-is. |
One thing that comes to my mind: Is there anything else in the factory which could use this? Also, is the |
Forgot to ask: any chance for a simple test or two, to verify decoration actually works... |
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.
Looks good overall; added notes but will address them post-merge.
JsonGeneratorDecorator
to allow decorating JsonGenerator
s
I think it's in the right class -- it could not really go in factory builder (as that is only part of constructing factory itself, not generator). One bigger thing we should tackle is to change all format backends to allow decoration. In 3.0 (master) this is relatively easy due to full refactoring of TokenStreamFactory hierarchy. But in 2.x there's much more duplication so pretty much every I did at least add tests for |
Support to register decorators for JsonGenerator in the builder and factory.