graphql-rust / graphql-parser

A graphql query language and schema definition language parser and formatter for rust
Apache License 2.0
355 stars 77 forks source link

Nom query #35

Closed That3Percent closed 3 years ago

That3Percent commented 4 years ago

I'd like to have an API something like pub fn nom_query<'a, S>(s: &'a str) -> Result<(Document<'a, S>, &'a str), ParseError> which consumes a query from the input and returns the remaining string to be consumed. The use-case is that I need to parse a query that is contained within a larger document which also needs to be parsed.

Would this be difficult to add? I forked the repo to tinker a bit and see if I could add it but it looked less straightforward than I imagined.

tailhook commented 4 years ago

Does the document contain input verbatim or quoted? I.e. I would expect something like (markdown):

    ```graphql
    query X {}
So you can feed a slice between triple backticks to the parser.

Otherwise, it's not clear where the document ends:

query X { } mutation Y { } some other text


Here document might end after the first `}` (i.e. when query ends). Or maybe after `mutation Y`. Or maybe "some other text" is also meant to be graphql, but braces or something else was missing, so we better show the error rather than stop the parser at this arbitrary point.
That3Percent commented 4 years ago

Does the document contain input verbatim or quoted?

Verbatim.

Here document might end after the first } (i.e. when query ends). Or maybe after mutation Y. Or maybe "some other text" is also meant to be graphql, but braces or something else was missing, so we better show the error rather than stop the parser at this arbitrary point.

Taking a look at the latest GraphQL spec to make sure I understand... I think what you are saying is that parse_query actually parses a subset of Document where Definition is limited to ExecutableDefinition. So, the Document may contain one or more ExecutableDefinition and it's unclear when the document ends.

I agree that this is a problem. I think what I need then is to be able to nom a single ExecutableDefinition. For the given example, it would return Ok(SomeQuery{name:"X", .. }, "mutation Y { } some other text").

tailhook commented 4 years ago

Parsing single definition should work. Our parsers are structured mostly similarly to the rules of grammar. So this should be easy to make a wrapper function around this specific rule. I think I'm okay with merging such PR.

But you can also try another approach: make a simple tokenizer that counts quotes and braces to quickly scan for the end of the graphql piece and then use the parser on the whole piece. This way skipping over a block of GraphQL is probably faster if you don't need to process it later. And also you can detect consecutive definitions and parse them together with a parser.

That3Percent commented 4 years ago

As you can see from the PR, I went with the approach that re-uses your existing tokenizer rather than build a separate one. I do actually need to process the query later.