Mis-parsing GraphQL identifiers when it starts with keywords (fix #20) #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 24, 2020
sordina
changed the title
|
@ecthiender can you review? Once this has been passed I'll be able to reference from graphql-engine. |
sordina
changed the title
ecthiender
reviewed
Feb 24, 2020
test/primitiveColumns.hs
Outdated
| @@ -0,0 +1,45 @@ | |||
| {-# LANGUAGE OverloadedStrings #-} | |||
There was a problem hiding this comment.
Name of the file should be PrimitiveColumns.
There was a problem hiding this comment.
Also, I think we should call it something else. It is not really related to columns, but primitive values of GraphQL.
ecthiender
reviewed
Feb 24, 2020
package.yaml
Outdated
| @@ -78,6 +78,7 @@ benchmarks: | |||
| tests: | |||
| graphql-parser-test: | |||
| main: Spec.hs | |||
| other-modules: PrimitiveColumns | |||
There was a problem hiding this comment.
In the hspec format, I don't think we have to include them like this. Do you think this is necessary?
ecthiender
reviewed
Feb 24, 2020
ecthiender
requested changes
Feb 24, 2020
ecthiender
changed the title
…a/graphql-parser-hs into sordina/lyndon/issue-20-column-misparsing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues:
Ensures that primitive identifiers end rather than appear as a prefix of a broader identifier, ala
nullColumnTwo.Unit-tests added to ensure that the issue does not re-occur.
Currently, the implementation may be inefficient due to lookup of char-class:
This could also be wrong!
Ideally lexing and parsing will be separated and lexing can be compared to the syntax spec more easily.