Closed Totktonada closed 2 years ago
@RunsFor Hi! Please, excuse me for the delay too. And thank you for the in-depth review!
I rebased the patch at top of current master and fixed all comments except ones about providing examples. I'll return there when time permits. I'll mark the PR as draft until I'll resolve all comments.
Updated with two changes.
Resolved ambiguity with <string>
.
The <string>
in my GraphQL query representation now means two different objects: an AST node for a value of the string type (it is written in double quotes in a query) and a Lua string. Replaced Lua strings with '...'
.
Also removed it is 'null' for <null>
comment, because here everything the same for all values type: the 'value' field contains a Lua string as it is wriiten in the query.
Squashed into the original commit.
Added generation of a picture with query AST.
Just tried it for fun. Don't know, whether it'll be useful.
Added as a separate commit.
@Totktonada Is this the same: https://github.com/tarantool/graphqlapi/blob/master/docs/types.md?
It is query AST.
@RunsFor Are you interested to look into the pull request again?
@olegrok What do you think on this? I would like to proceed in either way: drop at all, push first patch or push both patches.
@olegrok What do you think on this? I would like to proceed in either way: drop at all, push first patch or push both patches.
I like both patches. However currently I think it's better to place them to luagraphqlparser repo
I like both patches. However currently I think it's better to place them to luagraphqlparser repo
Seems meaningful. I'll close this pull request and will open another one in graphqlparser if time will permit.
Source: https://github.com/tarantool/graphql.0/commit/87bb0c8d602c88b2e38b316c95a7dba78b1ec6ae