Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork298
Add found string as property#571
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Moved logic about the "found" string from __str__ to its own property in`ParseBaseException`.
ptmcg commentedSep 22, 2024
I'll give it a look. There are a few very popular projects that use pyparsing and rely on the specific format of exception messages, so I try to keep these messages from changing if I can help it. Adding a new property should not be a problem, and with that you might be able to monkeypatch And those projects might also benefit from this new property, so there is that possibililty as well (though pyparsing elements of most projects are often in the category of "that one guy wrote this 10 years ago, so let's not ever change it"). |
Uh oh!
There was an error while loading.Please reload this page.
ptmcg commentedSep 22, 2024
Ok, I think this would make for a nice change, and there are likely some (slight) performance improvements we can do, too. See my review comment. |
ptmcg commentedSep 23, 2024
Or if you'd rather, I can accept your PR as-is, and then make these adjustments afterward (and also add tests and CHANGES note). |
OdysseasKr commentedSep 23, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Makes sense, should have used cached_property for this. I will make the changes today! |
Use `cached_property` to prevent re-computing properties of `ParseBaseException`
ptmcg left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me, merging
A bit of a silly change, feel free to close if you don't accept these.
I have moved the logic of the "found" string out of the
__str__magic method. This allows the user to generate prettier messages from the exception. I also think it's a bit cleaner to avoid parsing logic in__str__Example
From the following exception
I would like to generate a message like this
With the existing implementation the user would have to call
__str__and then remove the character, line and column information.