Skip to content
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

Do not capture Ignored characters by default #2394

Merged
merged 3 commits into from
Jun 23, 2021

Conversation

bbakerman
Copy link
Member

See #2207

This will save memory by default. graphql-java will not capture ignored characters by default.

A static is provided to allow a JVM wide default (set to false) or new parse methods will allow it to be explicitly set
on the parse method call

@bbakerman bbakerman requested a review from andimarek June 21, 2021 12:31
*/
@Deprecated
protected GraphqlAntlrToLanguage getAntlrToLanguage(CommonTokenStream tokens, MultiSourceReader multiSourceReader) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was left to preserve old behavior - if some one overrides this then they can.

@@ -69,11 +71,11 @@ class ParserTest extends Specification {


Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class was littered with IDEA warnings. I clean up a lot because broken windows and all that

@@ -842,6 +844,26 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases"""
document.getAdditionalData().get("key") == "value"
document.children[0].getAdditionalData().get("key") == "value"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the old method is tested here but the diff does not show it

@@ -727,7 +729,7 @@ triple3 : """edge cases \\""" "" " \\"" \\" edge cases"""
def input = "{,\r me\n\t} ,\n"

when:
Document document = new Parser().parseDocument(input)
Document document = new Parser().parseDocument(input, true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again the diff does not show it but this is the old ignored chars test - now needs to be ON explcitly

@andimarek andimarek added this to the 17.0 milestone Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants