Add SchemaPrinter AST comments support #3287
Conversation
| } | ||
| } | ||
|
|
||
| String comments(java.util.List<graphql.language.Comment> comments) { |
| } | ||
| } | ||
|
|
||
| String comments(java.util.List<graphql.language.Comment> comments) { |
There was a problem hiding this comment.
private method unless its unit tested
| caliber: Int | ||
| # gun 'name' input value comment | ||
| name: String | ||
| } | ||
| ''' | ||
| } | ||
| } |
There was a problem hiding this comment.
graphql.schema.idl.SchemaGenerator.Options#useCommentsAsDescriptions exists so it would be nice to test that we can generate from SDL with comments AND round drip it back out via a schema printer
This may well discover bugs if we have them in the SchemaGenerator
There was a problem hiding this comment.
Good call! I've added a test for this. Looks like it's not printing comments for directive and scalar. Will take a look at that later
There was a problem hiding this comment.
From what I can tell, the schema parser is not capturing directive and scalar comments in getComments - the tokens seem to be ignored. I am not very familiar Antlr and don't see anything different for these two schema elements vs. all the other that seem to work fine. Any ideas?
There was a problem hiding this comment.
I dont off hand - what I would do is leave the SDL test in place BUT change the assertion string to leave our directives and scalars. We can circle back to fix SDL scalar comment capture say later but this code can progress in the meantime
Added a review recommended round-trip (parse > generate > print) test.
Very happy to contribute and really appreciate your help! |
…l schema parser can be fixed.
|
FYI I resolved a merge conflict from another PR that was just merged, which added a test |
|
😄 I tried being clever with the web UI and left an extra bracket... I'll fix it |
|
Thank you! |
This PR is related to #3250 discussion. by adding comments support when creating schema programmatically instead of using SDL.