gh-82378 fix sys.tracebacklimit in pyrepl #123062
Conversation
make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook
remove the undocumented new keyword arguments colorize and limit that were added only for pyrepl
this removes the extra keyword argument colorize in showtraceback and showsyntaxerror again, which should hopefully fix the problems in gh-5004.
There was a problem hiding this comment.
I feel discomfort both from the current code and from this PR.
The current code violates the Liskov substitution principle: the signatures of showsyntaxerror() and showtraceback() in subclasses are not compatible with signatures in the base class.
In this PR the overridden _showtraceback() has compatible signature, but ignores its arguments. And what if we will need to pass additional arguments to format_exception() in a subclass? We will need to modify the base class.
|
@serhiy-storchaka thanks for your super thoughtful comments. I share your sense of discomfort, there's definitely too tight coupling between |
|
@serhiy-storchaka I did your suggestions, let me know what you think. |
|
@serhiy-storchaka thanks for all your help! |
|
Thanks @cfbolz for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Sorry, @cfbolz and @pablogsal, I could not cleanly backport this to |
…ythonGH-123062) Make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook (cherry picked from commit 63603bc) Co-authored-by: CF Bolz-Tereick <cfbolz@gmx.de>
…23062) Make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook
…23062) Make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook
…ythonGH-123062) Make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook (cherry picked from commit 63603bc) Co-authored-by: CF Bolz-Tereick <cfbolz@gmx.de>
…ythonGH-123062) Make sure that pyrepl uses the same logic for sys.tracebacklimit as both the basic repl and the standard sys.excepthook (cherry picked from commit 63603bc) Co-authored-by: CF Bolz-Tereick <cfbolz@gmx.de>
|
GH-123252 is a backport of this pull request to the 3.13 branch. |
|
Thanks @cfbolz for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
Sorry, @cfbolz and @pablogsal, I could not cleanly backport this to |
|
GH-123252 is a backport of this pull request to the 3.13 branch. |
pablogsal
changed the title
|
it doesn't really make sense to backport this to 3.12, because it's about pyrepl, does it? it still has the "needs backport" label. |
This is a slightly more invasive approach to #82378 than the one in #122452. In addition to the changes there, the PR also changes the way that
_pyrepl.console.InteractiveColoredConsoleinteracts withcode.InteractiveConsole. The PR removes new keyword argumentscolorize(andlimit) of the methodscode.InteractiveConsole.showsyntaxerrorand.showtraceback. The keyword arguments were added only for pyrepl, and not documented and only indirectly tested. Instead of using these keyword arguments, pyrepl now overwrites the new_showtracebackhelper method.The new undocumented keyword arguments broke some third-party libraries on PyPy pypy/pypy#5004 (comment), however in ways that are extremely unlikely to affect CPython.
I cannot decide whether this is a too radical change this late in the release process. @serhiy-storchaka what do you think?