gh-102895 Add an option local_exit in code.interact to block exit() from terminating the whole process#102896
Conversation
|
@gvanrossum, @brettcannon (as the ones who committed to |
Sorry, something went wrong.
|
Looks like a fine idea. I won’t be able to review. Make sure to add tests. |
Sorry, something went wrong.
|
Test is already added to make sure |
Sorry, something went wrong.
Me, really?!? I can't even remember when I last touched the code, so I don't feel up for doing a code review. |
Sorry, something went wrong.
|
Hi, I know this file has not been actively maintained for a while, but the code change is pretty straightforward and it's backward compatible. It will be at least beneficial for |
Sorry, something went wrong.
|
@brandtbucher if you could take a look at this when you have some time :) |
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
This isn't a full review, sorry.
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
Looks like several review comments didn't see action yet?
Regarding the name for the flag, was it already decided that we can't just always overwrite those two builtins? (Why are they closing stdin anyways? I don't recall; maybe you can find a clue in the code? It may be too long ago for the commit history to be useful.)
If we do need this feature, since the name has proved tricky, let's go with local_exit. The name probably doesn't matter too much since the feature feels pretty obscure.
Sorry, something went wrong.
Yes, I waited for the name confirmation as almost all files require that name. I updated all the files to use the name By overwriting those two builtins, what do you mean? Change the builtin classes? They close stdin because
|
Sorry, something went wrong.
|
@brandtbucher: Could you take over review here? I don't want this to languish for another 5 months (but neither is it urgent). |
Sorry, something went wrong.
|
Yep, I can. |
Sorry, something went wrong.
MaxwellDupre
left a comment
There was a problem hiding this comment.
The changes LGTM but I see Merging is blocked, hence suggest this is moved up to near the top.
Sorry, something went wrong.
|
Alright, I think this has marinated for long enough. :) |
Sorry, something went wrong.
…it() from terminating the whole process (pythonGH-102896)
…it() from terminating the whole process (pythonGH-102896)
edited by bedevere-bot
LoadingUh oh!
There was an error while loading. Please reload this page.
Copy link Copy MarkdownSorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.