Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-130698: Add safe methods to get prompts for new REPL#131110
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
gh-130698: Add safe methods to get prompts for new REPL#131110
Conversation
- and for `unrecoverable` exceptions
Uh oh!
There was an error while loading.Please reload this page.
Lib/_pyrepl/utils.py Outdated
| DEFAULT_PS1 = ">>> " | ||
| DEFAULT_PS2 = "... " | ||
| DEFAULT_PS3 = "... " | ||
| DEFAULT_PS4 = "... " |
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.
Should these match?
Lines 196 to 199 in95800fe
| ps1:str="->> " | |
| ps2:str="/>> " | |
| ps3:str="|.. " | |
| ps4:str=R"\__ " |
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.
No, I believe this is should be like:
DEFAULT_PS1 = ">>> "DEFAULT_PS2 = ">>> "DEFAULT_PS3 = "... "DEFAULT_PS4 = "... "I had misread this
Lines 171 to 179 in71da68d
| * ps1, ps2, ps3, ps4: | |
| prompts. ps1 is the prompt for a one-line input; for a | |
| multiline input it looks like: | |
| ps2> first line of input goes here | |
| ps3> second and further | |
| ps3> lines get ps3 | |
| ... | |
| ps4> and the last one gets ps4 | |
| As with the usual top-level, you can set these to instances if |
Lib/test/test_pyrepl/test_reader.py Outdated
| def prepare_reader_keep_prompts(*args, **kwargs): | ||
| reader = prepare_reader(*args, **kwargs) | ||
| del reader.get_prompt | ||
| reader.ps1 = Prompt() | ||
| reader.ps2 = "... " | ||
| reader.ps3 = "... " | ||
| reader.ps4 = "" | ||
| reader.can_colorize = False | ||
| reader.paste_mode = False | ||
| return reader | ||
| events = code_to_events("a=1") | ||
| reader, _ = handle_events_narrow_console( | ||
| events, | ||
| prepare_reader=prepare_reader_keep_prompts, | ||
| ) |
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.
We could use a slightly simpler setup code:
| defprepare_reader_keep_prompts(*args,**kwargs): | |
| reader=prepare_reader(*args,**kwargs) | |
| delreader.get_prompt | |
| reader.ps1=Prompt() | |
| reader.ps2="... " | |
| reader.ps3="... " | |
| reader.ps4="" | |
| reader.can_colorize=False | |
| reader.paste_mode=False | |
| returnreader | |
| events=code_to_events("a=1") | |
| reader,_=handle_events_narrow_console( | |
| events, | |
| prepare_reader=prepare_reader_keep_prompts, | |
| ) | |
| defprepare_reader(events): | |
| console=FakeConsole(events) | |
| config=ReadlineConfig(readline_completer=None) | |
| reader=ReadlineAlikeReader(console=console,config=config) | |
| reader.can_colorize=False | |
| returnreader | |
| events=code_to_events("if some_condition:\nsome_function()") | |
| reader=prepare_reader(events) |
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.
Unfortunately this doesn't simplify code. I have added more simpler one on my sight. Note, we should use handle_all_events (or handle_events_narrow_console) to process whole input string and get it in thereader.buffer to check if '\n' contains or not in buffer.
Misc/NEWS.d/next/Library/2025-03-13-00-39-54.gh-issue-130698.o2aU3e.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
…2aU3e.rstCo-authored-by: Tomas R. <tomas.roun8@gmail.com>
# Conflicts:#Lib/_pyrepl/reader.py
…gey-miryanov/cpython intopythongh-130698-pyrepl-ps1-exception
| @@ -10,6 +10,12 @@ | |||
| ZERO_WIDTH_TRANS = str.maketrans({"\x01": "", "\x02": ""}) | |||
| DEFAULT_PS1 = ">>> " | |||
| DEFAULT_PS2 = ">>> " | |||
chris-eiblApr 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
| DEFAULT_PS2=">>> " | |
| DEFAULT_PS2="... " |
Seehttps://docs.python.org/3/library/sys.html#sys.ps1
sys.ps1
sys.ps2
Strings specifying the primary and secondary prompt of the interpreter. These are only defined if the interpreter is in interactive mode. Their initial values in this case are '>>> ' and '... '.
>>> import sys>>> sys.ps1'>>> '>>> sys.ps2'... '>>> def foo():... passMaybe here theDEFAULT_PS* shall be used, too? Are there more places?
cpython/Lib/_pyrepl/simple_interact.py
Lines 134 to 135 in246ed23
| ps1=getattr(sys,"ps1",">>> ") | |
| ps2=getattr(sys,"ps2","... ") |
IMHO, the comment inreader.pyis wrong (or at least confusing):
Lines 171 to 178 in246ed23
| * ps1, ps2, ps3, ps4: | |
| prompts. ps1 is the prompt for a one-line input; for a | |
| multiline input it looks like: | |
| ps2> first line of input goes here | |
| ps3> second and further | |
| ps3> lines get ps3 | |
| ... | |
| ps4> and the last one gets ps4 |
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.
sergey-miryanovApr 20, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Maybe here the DEFAULT_PS* shall be used, too? Are there more places?
Yes, but I'm not sure that usage of DEFAULT_PS* approved overall.
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.
IDK either, but IMHO we shouldstick with what is documented and the status quo, i.e.DEFAULT_PS2 = "... ".
The reason why
Lines 196 to 199 in95800fe
| ps1:str="->> " | |
| ps2:str="/>> " | |
| ps3:str="|.. " | |
| ps4:str=R"\__ " |
is not in effect:
Lines 49 to 54 in95800fe
| # set sys.{ps1,ps2} just before invoking the interactive interpreter. This | |
| # mimics what CPython does in pythonrun.c | |
| ifnothasattr(sys,"ps1"): | |
| sys.ps1=">>> " | |
| ifnothasattr(sys,"ps2"): | |
| sys.ps2="... " |
and then later
cpython/Lib/_pyrepl/simple_interact.py
Lines 134 to 139 in246ed23
| ps1=getattr(sys,"ps1",">>> ") | |
| ps2=getattr(sys,"ps2","... ") | |
| try: | |
| statement=multiline_input(more_lines,ps1,ps2) | |
| exceptEOFError: | |
| break |
and then
cpython/Lib/_pyrepl/readline.py
Lines 375 to 387 in95800fe
| defmultiline_input(self,more_lines:MoreLinesCallable,ps1:str,ps2:str)->str: | |
| """Read an input on possibly multiple lines, asking for more | |
| lines as long as 'more_lines(unicodetext)' returns an object whose | |
| boolean value is true. | |
| """ | |
| reader=self.get_reader() | |
| saved=reader.more_lines | |
| try: | |
| reader.more_lines=more_lines | |
| reader.ps1=ps1 | |
| reader.ps2=ps1 | |
| reader.ps3=ps2 | |
| reader.ps4="" |
So IMHO we should consistently useDEFAULT_PS1 andDEFAULT_PS2 in those code parts above, but let@pablogsal decide on it.
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.
Moreover, this part
reader.ps1=ps1reader.ps2=ps1reader.ps3=ps2
IMHO explains this confusion
Lines 171 to 178 in246ed23
| * ps1, ps2, ps3, ps4: | |
| prompts. ps1 is the prompt for a one-line input; for a | |
| multiline input it looks like: | |
| ps2> first line of input goes here | |
| ps3> second and further | |
| ps3> lines get ps3 | |
| ... | |
| ps4> and the last one gets ps4 |
ISTM, this is an implementation detail, andps2=ps1, etc, maps to what we are used to in the cPython REPL.
Most probably, because the new PYREPL comes from PyPy?
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.
Moreover, this part
Yeah, I saw this and this is a reason that I changed DEFAULT_PS2 yesterday.
Most probably, because the new PYREPL comes from PyPy?
I'm not familiar with PyPy, so 🤷♂️
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
Uh oh!
There was an error while loading.Please reload this page.
It fixes cases when ps1, ps2, ps3, ps4 raise exception from
__str__.This PR still misses tests for Reader.arg, but maybe you can review the whole solution - do I drive in the right direction?Fixed for Reader.arg.