gh-130698: Add safe methods to get prompts for new REPL

[sergey-miryanov]

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.

• Issue: REPL exits with traceback when str(sys.ps1) raises #130698

[sergey-miryanov]

I updated this PR because it had merge conflicts. Not sure what to do next though.

[sergey-miryanov]

@chris-eibl @tomasr8 Could you please take a look? Is there anything else I can do to move this PR forward?

[chris-eibl]

Had another look and agree that this is correct:

No, I believe this is should be like:

DEFAULT_PS1 = ">>> "
DEFAULT_PS2 = ">>> "
DEFAULT_PS3 = "... "
DEFAULT_PS4 = "... "

As pointed out in #131110 (comment), this stems from the fact that cPython only uses a primary and secondary prompt:
See https://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 '... '.

and we have to map that to ps1 ... ps4 used in the implementation (coming from PyPy).

I still think that your new DEFAULT_PS* should not be used just in case of an Exception, but throughout the code base, i.e. be the single source of truth for the defaults.

E.g.

cpython/Lib/_pyrepl/simple_interact.py

Lines 134 to 135 in 246ed23

	ps1 = getattr(sys, "ps1", ">>> ")
	ps2 = getattr(sys, "ps2", "... ")

can become

ps1 = getattr(sys, "ps1", DEFAULT_PS1)
ps2 = getattr(sys, "ps2", DEFAULT_PS3)

and all the other places @tomasr8 and I have mentioned above?

Since that mapping seems to be confusing, only using ps1 and ps2 would be preferable, but most likely out of scope for this PR.

Because AFAIR PyPy sometimes downstreams our changes, most probably ditching ps3 and ps4 is just churn for them and us, but having the defaults at a single place helps everybody.

[chris-eibl]

#130698 (comment)

@skirpichev I don't know what is the best way - replicate old behavior, show default prompt (as I did in PR), or emit warning. Maybe @pablogsal can point to the right direction.

I have no preference here. Maybe a warning would be nice, that can easily be added if others want it.

Your PR definitely fixes the issue, so LGTM 🚀

[sergey-miryanov]

@chris-eibl Thanks a lot for such detailed answer! Working on this ...

[sergey-miryanov]

I left only DEFAULT_PS1 and DEFAULT_PS2 and added MULTILINE_PS*. I think it should be a bit clearer.

I'm not sure should we use DEFAULT_PS1 in the IDLE module, because it uses a slightly different prompt (>>>\n instead of >>> ).

Also, I think we should leave as-is C modules, where >>> and ... used to pre-initialize ps1 and ps2.

[sergey-miryanov]

@chris-eibl Could you please take a look?

[chris-eibl]

I left only DEFAULT_PS1 and DEFAULT_PS2 and added MULTILINE_PS*

Ups, quite a splash radius. Let's see how others like it.

I think it should be a bit clearer.

Yes, it is, but chances are high this is rejected or shall go into its own PR?

Still unsure about @tomasr8's comment

Should these match?

cpython/Lib/_pyrepl/reader.py

Lines 196 to 199 in 95800fe

	ps1: str = "->> "
	ps2: str = "/>> "
	ps3: str = "|.. "
	ps4: str = R"\__ "

Most probably it is better to leave them alone. The way cPython uses it, they are always overwritten in multiline_input, anyway.

If we'd like to strive for a least minimal change, then all defaults can be removed again and just return an empty string (like the old REPL did):

    def __get_prompt_str(prompt: object) -> str:
        try:
            return str(prompt)
        except (MemoryError, SystemError):
            raise
        except Exception:
            return ""

That way the user at least get's a hint that something is flaky by neither seeing their expected prompt nor the default one, since atm no warnings are printed here. And we don't reinvent the wheel, there is already precedence ...

See also #130698 (comment) of @skirpichev in the issue.

Sorry for the back and forth, but simpler might be better?

Up to you, just stopping here and waiting for the REPL maintainers to chime in is also an option ...

[sergey-miryanov]

The way cPython uses it, they are always overwritten in multiline_input, anyway.

Yes, I left it as it was because we always rewrite all prompts in multiline_input.

quite a splash radius

Yes, I think returning to the old behavior is a good idea now. At least it's simple and minimal. I'll wait a while for the pyrepl maintainer's opinion. If I don't hear back from anyone, I'll implement the old behavior.

Thanks!