Skip to content

Do not capture Ignored characters by default#2394

Merged
bbakerman merged 3 commits intomasterfrom
ignore-chars-optimisation
Jun 23, 2021
Merged

Do not capture Ignored characters by default#2394
bbakerman merged 3 commits intomasterfrom
ignore-chars-optimisation

Conversation

@bbakerman
Copy link
Copy Markdown
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 - really should use {@link #getAntlrToLanguage(CommonTokenStream, MultiSourceReader, Boolean)}
*/
@Deprecated
protected GraphqlAntlrToLanguage getAntlrToLanguage(CommonTokenStream tokens, MultiSourceReader multiSourceReader) {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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


when:
Document document = new Parser().parseDocument(input)
Document document = new Parser().parseDocument(input, true)
Copy link
Copy Markdown
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.

2 participants