Add method to AstPrinter to allow supplying Appendable #3853
Conversation
| @@ -1,9 +1,11 @@ | |||
| package graphql.language; | |||
|
|
|||
| import com.google.common.io.CharStreams; | |||
There was a problem hiding this comment.
Note: we only shade in selected classes of Guava, before merging in this PR the Gradle config should be changed to shade in this new class
There was a problem hiding this comment.
Our copy the code in -
Hmmm com.google.common.io.AppendableWriter is kinda large.
But we dont really want the wholecom.google.common.io classes in here - just some select ones say
Can we tighten this down some how in shadow jar config ??
There was a problem hiding this comment.
relocate('com.google.common', 'graphql.com.google.common') {
include 'com.google.common.collect.*'
include 'com.google.common.base.*'
include 'com.google.common.math.*'
include 'com.google.common.primitives.*'
}
is what we have today - I wonder if we can do specific classes say rather than all of common.io
There was a problem hiding this comment.
Thanks @kilink for updating the PR to use a non-Guava alternative, that's a great idea
Add printAstTo to AstPrinter, which allows the caller to supply their own target Appendable. This can be useful for reusing StringBuilders for instance.
| @@ -757,8 +781,11 @@ public static String printAst(Node node) { | |||
| */ | |||
| public static void printAst(Writer writer, Node node) { | |||
| String ast = printAst(node); | |||
| PrintWriter printer = new PrintWriter(writer); | |||
| printer.write(ast); | |||
| try { | |||
There was a problem hiding this comment.
@kilink @bbakerman @dondonz why did we change that? What is the consequence of that?
There was a problem hiding this comment.
PrintWriter never throws an exception but just drops data.
This is better really. Change in behavior yes... but much better behavior
There was a problem hiding this comment.
Is that a breaking change needs to be documented?
There was a problem hiding this comment.
We can call out that it's a change in the release notes, I can add the label
Add printAstTo to AstPrinter, which allows the caller to supply their own target Appendable. This can be useful for reusing StringBuilders for instance.