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

Parser Progress #6

Closed
2 of 4 tasks
svmnotn opened this issue Mar 20, 2016 · 9 comments
Closed
2 of 4 tasks

Parser Progress #6

svmnotn opened this issue Mar 20, 2016 · 9 comments

Comments

@svmnotn
Copy link
Contributor

svmnotn commented Mar 20, 2016

  • Lexer (done once Lexer #8 is merged?)
  • AST Representation (Should be nearly done, but might need some work due to types, also needs tests)
  • Parser (Look here for inspiration? Repo seems abandoned)
  • Errors?

To me the lexer looks finished but I have not tested, next would be an AST representation of the types, then the parser?

I just created this issue to track the progress of the parser as @calebmer and I need it for a different project. On that note I think splitting the graphql implementation into separate crates and then having a single executable server that uses them will provide the best organization.

@svmnotn svmnotn changed the title Progress Parser Progress Mar 20, 2016
@svmnotn
Copy link
Contributor Author

svmnotn commented Mar 20, 2016

Any input as to where the progress of the lexer actually is would be really appreciated.

@cksac
Copy link
Owner

cksac commented Mar 20, 2016

@svmnotn the lexer required more tests and include the location information in the token.
yes, next would be AST representation, I am not sure is it better to compatible with libgraphqlparser.
Agree that we need to separate parser into different crate. Do you think that we need to create a github organization for that?

@svmnotn
Copy link
Contributor Author

svmnotn commented Mar 20, 2016

  1. What do you mean by compatible with libgraphqlparser?
  2. No need for a github organization for that, a single repo can host multiple crates. If you want I can make that setup as a pr so that you can see how it would work.

@cksac
Copy link
Owner

cksac commented Mar 20, 2016

  1. I mean the parser can be replaced by libgraphqlparser rust binding without change in AST.
  2. Sure, thanks for your help

@svmnotn
Copy link
Contributor Author

svmnotn commented Mar 20, 2016

There is no libgraphqlparser rust binding other than the really awful one I made, that I know of, and I really do not wish to support that one. If you do know cpp or c and want to deal with Rust FFI feel free to take a chance.

Actually that repo does have multiple crates in one repo, so you can look at it for reference if you want.

@calebmer wrote an AST that I was planning on simply dropping into here, so if you want compatibility with libgraphqlparser, I'm not entirely sure that it is, but I'll make a pr with it just in case.

For reference this is my libgraphqlparser binding.

@cksac
Copy link
Owner

cksac commented Mar 20, 2016

ok, let's build one in rust first

@calebmer
Copy link
Contributor

A note on libgraphqlparser: by default, it only builds/installs a dynamic library. This means it must be installed on the user's computer. If you look at the Python and elixir projects using it, you'll see they both require a libgraphqlparser installation before being able to use the bindings.

The Ruby gem doesn't require this, however, Ruby can include assets like a dylib in its distributable.

I agree it would be nice to use libgraphqlparser, but if it can't statically link, we can't use it. I opened an issue here for guidance.

Until then a Rust parser will be way more fun and likely more performant 😊

@svmnotn
Copy link
Contributor Author

svmnotn commented Mar 22, 2016

I think the merge of #8 finishes the lexer, is the AST finished?

@calebmer
Copy link
Contributor

With #11 the AST is done for now. Since it's just types it's pretty much tested by the compiler. The one thing that could be useful is an integration test of what the parser should return.

@svmnotn svmnotn closed this as completed Jul 8, 2019
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

No branches or pull requests

3 participants