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-83151: Make closure work on pdb by gaogaotiantian · Pull Request #111094 · python/cpython Conversation
|
|
Great work! I'm not sure I have the time (or even, at this point, the context) to review this. Maybe someone else can do this more justice? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Oh I invited you because you reopened #65360 (comment) and this fix involves some black magic of Python. But no worries I'm sure I can find great reviewers for feedbacks of the PR (a couple invited already)! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Been playing around with this locally for a while, and it seems like it works really well! Nice job.
Here's a first round of review. This is a tricky task, but I think the current solution may be overcomplicating it in places:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I just realized this might have some conflict with PEP 667 which will probably be merged today. I will rework the code. Also taking the notes from your reviews. I will also hopefully finish this tomorrow. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I had to make quite a bit of changes to make this work with PEP 667 -
This relies on the fix from #118583 so please do not merge it yet. I applied the change from that issue just to make the tests pass. I'll merge it in this PR when that one is merged in main. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I like how this is starting to look. Some more thoughts:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Brandt Bucher <[email protected]>
|
We need to put the other We should be able to get the value. But compiling this with our instrumentation would be: and returns an error that's confusing to users - That's why we need to fall back to normal. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Brandt Bucher <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Okay, this seems correct! Thanks for tackling such a longstanding and hairy issue, and for taking the time to walk me through the fix. :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'm glad this finally got fixed :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…from Python 3.13 (#14933) - [x] add a failing test case - [x] backport python/cpython#111094 into IPython codebase - fixes gotcha/ipdb#256 - solves the pdb-related comments on #62  ### Context IPython already includes bits of Pdb code from Python itself, as documented in: https://github.com/ipython/ipython/blob/1e9a2e8b5a584ffe1973318fdabf2e39395ec367/IPython/core/debugger.py#L92-L110 I am taking a more cautious approach and separating the backport to a new file to make it easy to delete in the (near) future (1 year from now).
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.