Skip to content

||| should resolve a tie by choosing this parser rather than q #72

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

Closed
jeffrey-aguilera opened this issue Oct 14, 2015 · 3 comments · Fixed by #166
Closed

||| should resolve a tie by choosing this parser rather than q #72

jeffrey-aguilera opened this issue Oct 14, 2015 · 3 comments · Fixed by #166
Milestone

Comments

@jeffrey-aguilera
Copy link

if (next2.pos < next1.pos) should be if (next2.pos <= next1.pos) (or equivalently if (next1.pos >= next2.pos), which I feel is more readable and consistent with the specification that the ''longest'' is chosen.)

@toddobryan-amazon
Copy link
Contributor

I think you're probably right. Based on the comments, you'd expect it to only use q0 if this parser consumed fewer characters, but with the code the way it is, it accepts q0 if they consume the same number.

Have you tried putting together a pull request with some test cases?

@hrj
Copy link
Contributor

hrj commented Jun 24, 2016

Possible to alter the comment rather than the code, in the interest of maintaining the current behaviour?

Philippus added a commit to Philippus/scala-parser-combinators that referenced this issue Aug 15, 2018
Philippus added a commit to Philippus/scala-parser-combinators that referenced this issue Aug 15, 2018
Philippus added a commit to Philippus/scala-parser-combinators that referenced this issue Jan 29, 2019
@SethTisue
Copy link
Member

PR #166

Philippus added a commit to Philippus/scala-parser-combinators that referenced this issue Apr 10, 2019
@SethTisue SethTisue added this to the 1.2.0 milestone Apr 10, 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.

4 participants