Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-111201: A new Python REPL#111567
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-111201: A new Python REPL #111567
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner 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.
You add many env vars. Can you document them in Doc/using/cmdline.rst?
Usually, env var names start with PYTHON. Should the variables be renamed?
Also, Python ignores env var starting with PYTHON if sys.flags.ignore_environment.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| manually. | ||
| Note that there is also a built-in module _minimal_curses which will | ||
| hide this one if compiled in. |
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.
Is it part of this PR? If not, do you consider adding it or not?
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.
Yes, sounds like this comment was carried over from pypy but doesn't apply here.
pablogsal commented Nov 1, 2023
@vstinner the PR is still draft, please wait until we mark it as finished before reviewing as most of this code may change |
c34adb8 to 702b59eCompare
hugovk 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.
Please also add a What's New entry.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
pablogsal commented May 5, 2024
There is already one (Misc/NEWS.d/next/Core and Builtins/2024-04-28-00-41-17.gh-issue-111201.cQsh5U.rst) |
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
pablogsal commented May 5, 2024
@hugovk@JelleZijlstra I have implemented your suggestions. If you have some time, do you mind reviewing again? |
hugovk 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.
Docs changes look good.
One suggestion: this PR adds the main docs to the tutorial, we should probably add something to the reference, but this could be a followup.
Please also add a What's New entry.
There is already one (Misc/NEWS.d/next/Core and Builtins/2024-04-28-00-41-17.gh-issue-111201.cQsh5U.rst)
That's the changelog, this change definitely needs highlighting in https://docs.python.org/3.13/whatsnew/3.13.html but it can also be in a followup PR.
cfbolz 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.
Cool to see this!
Just to check, was it intentional that you didn't port PyPy's _pyrepl tests? There's not a huge amount of tests there, admittedly.
I am philosophically a little bit worried that people will start using and relying on _pyrepl internal details, which are now slightly different between PyPy and CPython, right? This could lead to PyPy's life getting somewhat harder in this area. As usual, it's hard to stop people from doing that, of course.
| self.restore() | ||
| yield | ||
| finally: | ||
| forargin ("msg", "ps1", "ps2", "ps3", "ps4", "paste_mode"): |
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.
why is there a discrepancy between how prev_state is constructed (which uses fields(self) and how it is used in the finally block (listing the attributes explicitly)?
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.
It evolved into this, which you're right would now be cleaner to just reuse the list of fields.
I originally started with just dict(__dict__) but when I added typing with slots=True the __dict__ was gone so I modified it to use fields() instead.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
bedevere-bot commented May 5, 2024
|
bedevere-bot commented May 5, 2024
|
bedevere-bot commented May 5, 2024
|
Co-authored-by: Łukasz Langa <lukasz@langa.pl> Co-authored-by: Marta Gómez Macías <mgmacias@google.com> Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.