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

take_until with tuple parser #168

Closed
constituent opened this issue Jun 29, 2018 · 10 comments
Closed

take_until with tuple parser #168

constituent opened this issue Jun 29, 2018 · 10 comments
Labels

Comments

@constituent
Copy link

I'm new to parser combinator so pardon if this is a naive question.
This works as expected:
println!("{:?}", take_until::<String, _>(try(string("ab"))).easy_parse(r#"aaab"#)); // Ok(("aa", "ab"))
But this behaves surprisingly:
println!("{:?}", take_until::<String, _>(try((char('a'), char('b')))).easy_parse(r#"aaab"#)); // Ok(("aaa", "b"))

I want to use tuple because the closing string is calculated at runtime. For example, to parse the closing "quote" of Rust raw string, I count the number of # in r#", but
take_until::<String, _>(try((char('"'), count_min_max::<String, _>(1, 1, char('#'))))).easy_parse("aa\"ab\"#")
gives aa\"ab\" instead of aa\"ab

@Marwes Marwes added the bug label Jun 29, 2018
Marwes pushed a commit that referenced this issue Jun 29, 2018
(All non-partial parsers are always in "first mode").

Since this wasn't checked before, a sequence parser that returned an
error after consuming input would start from the middle of the sequence
if it parsing was attempted with it again as the partial state isn't
reset).

Fixes #168
@Marwes
Copy link
Owner

Marwes commented Jun 29, 2018

Fix coming in b7285e3 (going to fix #167 as well before publishing)

@constituent
Copy link
Author

Thanks for the immediate fix!

Marwes pushed a commit that referenced this issue Jun 29, 2018
(All non-partial parsers are always in "first mode").

Since this wasn't checked before, a sequence parser that returned an
error after consuming input would start from the middle of the sequence
if it parsing was attempted with it again as the partial state isn't
reset).

Fixes #168
@constituent
Copy link
Author

@Marwes
take_until works fine now, but recognize(skip_until()) is still broken:

println!("{:?}", recognize(skip_until(try(string("ab")))).parse("aaab")); // Ok(("aa", "ab"))
println!("{:?}", recognize(skip_until(try((char('a'), char('b'))))).parse("aaab")); // Ok(("aaa", "b"))

@Marwes
Copy link
Owner

Marwes commented Jun 30, 2018

Ouch, that is a rather bad bug. It's root cause was that the parser! macro generated the wrong parse method which actually affects a lot of parsers. However, it is only the combination of how take_until/skip_until works in conjunction with try that causes it to manifest. Remove any of parser!, take_until or try and everything works as expected.

Fix released as 3.3.4

@constituent
Copy link
Author

I have tried version 3.34, it seems that my recognize(skip_until()) bug still exists:
println!("{:?}", recognize(skip_until(try((char('a'), char('b'))))).parse("aaab")); // Ok(("aaa", "b"))

@Marwes
Copy link
Owner

Marwes commented Jun 30, 2018

Damn, I may have either published the wrong version or recognize somehow has a similar bug all on its own. Does it work of you use take_until instead of skip_until and just ignore the result of take_until? ( can't check atm)

@Marwes
Copy link
Owner

Marwes commented Jun 30, 2018

Or test it with the git master

@constituent
Copy link
Author

take_until(try()) is broken in version 3.3.2 and fixed in 3.3.3 (with no change in 3.3.4).
recognize(skip_until(try())) is broken in all these 3 versions.

git master version behaves the same as 3.3.4.

I'm just learning combine and now trying hard figuring out how to implement Parser trait for custom types. I'm not urgent and feel free to check it once you have vacant time.

@Marwes
Copy link
Owner

Marwes commented Jun 30, 2018

Seema that range::recognize has the same issue that skip_until had (but combinator::recognize does not have this issue which may have been why i didnt notice)

@Marwes
Copy link
Owner

Marwes commented Jun 30, 2018

Fix in #173 . I checked all other uses of parse_partial to see if they needed to be updated but it looks like that was the last one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants