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

Possible bug with Any (or Seq)? #3

Open
austin-searchpilot opened this issue Jun 29, 2018 · 1 comment · May be fixed by #6
Open

Possible bug with Any (or Seq)? #3

austin-searchpilot opened this issue Jun 29, 2018 · 1 comment · May be fixed by #6

Comments

@austin-searchpilot
Copy link

Hi,

First of all - thanks for the great library! I've found a possible bug. I've tried to debug it to find the underlying cause but have struggled a bit. It can be reduced to the following case.

If we have the parser:

str := `str`
str1 := parsify.Seq(`str`, `1`)
str2 := parsify.Any(`str2`)
parser := parsify.Any(str1, str2, str)

I would expect this to be able to parse the string str (for the 3rd argument to the Any) and then try to parse the string str, I get the error goparsify.UnparsedInputError{remaining:"str"} .

My exploration of the code is guiding me to think that the Any from the str2 line is not errorring correctly but I can't see how to fix it.

Do you agree that the parser defined above should be able to parse str? And if so, could you give me a pointer of how I might go about fixing it?

Some additional information:
The exact same parser works with the library at commit 0f854720ca2bad30246020bb01cdb903a5f9406d. I noticed the main thing that has changed in between was the removal of the prediction in the Any function which my colleague suggested a couple of weeks ago. I can't see anything obvious in that commit which would trigger this new behaviour though.

@brandondyck
Copy link

brandondyck commented Jul 22, 2018

I'm getting a similar error. I also don't understand it, but it seems to have something to do with how error handling interacts between nested Any parsers. I got it to run correctly and still pass all tests by ignoring the existing ps.Error in Any:

https://github.com/brandondyck/goparsify/blob/c04253e6aa0f206f069c60c96d4daaef4a09228b/combinator.go#L36-L70

I'd submit a PR, but I'm not comfortable with applying fixes I don't understand.

EDIT: Fixed the link

@brandondyck brandondyck linked a pull request Aug 10, 2018 that will close this issue
superfell pushed a commit to superfell/goparsify that referenced this issue Dec 14, 2018
When nested the inner Any() can end up seeing Error.pos set, but Error.expecting to be "". This gets copied into longestError.  This can subsequently lead it to not update longestError after calling a child parser (because the child error is before longestError). This results in it incorrectly indicating there was no error.
oec added a commit to oec/goparsify that referenced this issue Jun 18, 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

Successfully merging a pull request may close this issue.

2 participants