search

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

Upgrade to combine 4.x.y #55

Closed DoumanAsh closed 1 year ago

DoumanAsh commented 3 years ago

Subj Most notable change is combine::parser doesn't accept functions that return combine::ParseResult so you have to convert it each time. Probably could just write own version that accepts combine::ParseResult to avoid conversion

tailhook commented 3 years ago

Hi! Thanks for the PR. And sorry for late reply. There are two issues with the PR.

  1. unexpected_format should probably be replaced by unexpected_static_message instead where message in static (pretty minor)
  2. Instead of parse_stream(input).into(), I think we should use -> impl Parser<..> return everywhere (there was no -> impl at the point this was originally implemented, this is why it's not used). The change should be pretty mechanical, although we need to use opaque! macro at some place where nested nodes are used.
DoumanAsh commented 3 years ago

Got it Although it doesn't seem to me that replacing parser functions with just returning impl Parser is all that simple to be honest. But then again I'm not all that familiar with combine

LegNeato commented 1 year ago

This has been superceded.