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

[bug] Parser.desc() causes loss of error information, simple fix #77

Open
genovese opened this issue Aug 20, 2023 · 2 comments
Open

[bug] Parser.desc() causes loss of error information, simple fix #77

genovese opened this issue Aug 20, 2023 · 2 comments

Comments

@genovese
Copy link

Parser.desc causes loss of error information on failure of the original parser. So when a component parser (say in a generate) with a desc fails, the super-parser will not show the correct position or expected message in the result. I can give an example if you like, but it's easier just to look at the code because the problem is fixed with a small change: aggregate the original parser's failed result into the return value.

In Parser.desc, the original wrapped function looks like

        @Parser
        def desc_parser(stream, index):
            result = self(stream, index)
            if result.status:
                return result
            else:
                return Result.failure(index, description)

In the case of failure, the error information in result is lost because it is not used, giving the unexpected behavior. Instead, just change the last line as follows:

        @Parser
        def desc_parser(stream, index):
            result = self(stream, index)
            if result.status:
                return result
            else:
                return Result.failure(index, description).aggregate(result)

and the error information is correct.

Thanks. I'm enjoying using parsy.

@genovese
Copy link
Author

I think the original intent (the description supplanting the automatically generated one) has a use case, when applying .desc() at the "leaf" level. For example, in regex(r'horrible regex').desc('a thing'), we do not want the horrible regex in the expected output. But when applying desc() on a combined parser, we don't want to lose the index. The latter is a real problem because the error message becomes insensitive to component failures; I think that strongly motivates the fix above.

Still, these two cases are in conflict. Perhaps there needs to be two ideas here: the label for the thing and where the expected label comes from?

@spookylukey
Copy link
Member

Hi @genovese I think your analysis is correct. I haven't looked into it yet - how easy is it to get both accurate location information and the desired specified description in the error message?

Some test cases would be helpful to see exactly the issue. Thanks!

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

No branches or pull requests

2 participants