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

Improve error reporting for some failed match scenarios #142

Open
AndrewSav opened this issue Dec 20, 2021 · 9 comments
Open

Improve error reporting for some failed match scenarios #142

AndrewSav opened this issue Dec 20, 2021 · 9 comments

Comments

@AndrewSav
Copy link
Contributor

AndrewSav commented Dec 20, 2021

Consider that at a certain point during parsing I know that the parsing is not going to succeed. E.g. parser A matched (so I know that alternatives like C and and D won't match) but, after parser A matched, parser B failed. There is no point to continue here and we have an opportunity here to produce a good error message. Do we have a way to do this?

Consider this:

A.Then(x => B).Or(C).Or(D).Many().AtEnd()

When B fails then C and D will be tried (I only want them tried if A itself fails). If A matched I know that neither C nor D will match. Many will succeed and then At End will fail with a lame error message about something unexpected.

When I'm parsing B and it has failed I know why, this is because after a they had to specify b but they did not so I would like print out what B expected and how what they specified is different from that.

E.g. A.Then(x => B.FailOnError("you have to specify correct b after a")).Or(C).Or(D).Many().AtEnd()

Can we do this already by cleverly rewriting this chain of parsers?

@nblumhardt
Copy link
Member

Because Superpower doesn't backtrack, I'd expect A.Then(B).Or(C).Or(D).Many().AtEnd() to fail if B fails, without attempting any additional parses. Does that match what you see?

@AndrewSav
Copy link
Contributor Author

@nblumhardt no, not really, unfortunately. Consider the case when A is zero-width, e.g. Parse.Not(something)

@AndrewSav
Copy link
Contributor Author

Here is an example: https://github.com/AndrewSav/CommandLineParserPoc/blob/ca154fec085b96966d5cd45373270bef1b40536c/CommandLineParserPoC/CommandLineParser.cs#L161

The parser matches either binary switch list switch or value switch. The remaining type is value and it is invalid in the beginning. So I want to test for it and fail if that's so with a nice error message. Currently I'm getting the "At end" message that is not that nice.

@nblumhardt
Copy link
Member

Will have a closer look at this - thanks 👍

@nblumhardt
Copy link
Member

Closing this one as stale; still seems worth investigating but will need some concrete direction to move forwards. Thanks @AndrewSav

@AndrewSav
Copy link
Contributor Author

@nblumhardt Seriously? What have I done wrong?

@nblumhardt
Copy link
Member

Perhaps I misunderstood your conclusions in your PR - is it possible to implement this atop what we have in Superpower today?

@AndrewSav
Copy link
Contributor Author

@nblumhardt apologies, what PR are we talking about? I might have forgotten something.

is it possible to implement this atop what we have in Superpower today?

That's what this issue is about. Is it possible? If not, why? This seems to me as something that is reasonable. I understand that did not come up in your usage scenarios, so I wanted to know if there are ways to solve this I have not thought of with what is already there. If there are not, than solving it by improving Superpower seems as a good thing to do for wider users or Superpower, so I was looking for ideas on how we go about it, what is actually required to make this work.

If your resources constraint and time allocation simply does not allow for improving the library beyond your own needs, and you want to close issues on that basis, then just let me know. To me reading this thread feels like you said you would take a closer look and then closed it with a comment I could not really understand.

@nblumhardt
Copy link
Member

Sorry Andrew, I think it's me confused, I thought the linked PR which you closed was exploring this.

Time for working on this is low right now, but no doubt some opportunities will come up in the future and I'll have a chance to dig in.

The aim is to keep the open issues list fresh, so that when some time is available, I can skip any dead-ends or no-longer-needed requests. Between the Datalust projects, Serilog-related ones, and others, this can mean sorting through a lot of issues, and it's not a 100% scientific process :-)

On this issue in particular, it seems to need some deep digging; if anyone following along is able to get to the bottom of what's needed, that would help guide it towards a solution. Cheers!

@nblumhardt nblumhardt reopened this Jun 18, 2024
@nblumhardt nblumhardt changed the title Do we have a way to Break/Fail? Improve error reporting for some failed match scenarios Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants