GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||
GeezerVison is showing github.com — gh-95778: Use a note for the max digits error message by vstinner · Pull Request #96878 · python/cpython Conversation
|
|
@iritkatriel @Zac-HD: This PR adds the I prefer to call directly I'm not sure about the API in case of error. Currently, the function ignores any exception raised while trying to add a note to the exception :-( Do you think that an unraisable exception should be logged here? Or should we replace the currently raised exception with this new one? Maybe it would be acceptable if it's chained with the previous exception? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'm looking at which existing code might benefit from _PyErr_Note(). "Suggestions" added by PyErr_Display() with _Py_Offer_Suggestions() (issue #82711) is not a good fit: this code doesn't modify the exception on purpose. Or do you want to add suggestions in notes? If yes, we should maybe make sure that the note is not added multiple times if PyErr_Display() is called multiple times on the same exception ;-) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Maybe _PyPegen_number_token() can use _PyErr_AddNote() for this code? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| // Call add_note(note) on the current exception. | ||
| // Return -1 on error, or 0 on success. | ||
| // Save and restore the current exception. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Might be good to mention in the comment that there must be a current exception. I think otherwise it crashes or some assertion fails, not sure.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The intention with exception notes was that you use it to add information that is not available at the time the exception is created and raised, but only after you caught it. This doesn't follow that pattern. Why not just add one or two newlines to the msg? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Oh, that's far from issue #95778 scope, so I created the new issue #96958 to discuss that instead. I propose to continue the discussion there. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I close the issue to continue discussing it in #96958. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.