Conversation
vstinner
left a comment
There was a problem hiding this comment.
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.
| 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.
Is it part of this PR? If not, do you consider adding it or not?
There was a problem hiding this comment.
Yes, sounds like this comment was carried over from pypy but doesn't apply here.
|
@vstinner the PR is still draft, please wait until we mark it as finished before reviewing as most of this code may change |
…importing too much
c34adb8 to
702b59e
Compare
hugovk
left a comment
There was a problem hiding this comment.
Please also add a What's New entry.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
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>
|
@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.
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.
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: | ||
| for arg in ("msg", "ps1", "ps2", "ps3", "ps4", "paste_mode"): |
There was a problem hiding this comment.
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.
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.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
|
|
|
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.