Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-127495: Append to history file after every statement in PyREPL#132294
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Looks good to me. I'd like the following questions clarified before we proceed:
|
skirpichev commentedApr 10, 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.
history entry will be appended only if the statement was finished. (Well, at least this was an intention and it seems working in the current implementation.)
Good catch, thanks! I was thinking about multiple repls and this IMO deserve own issue. We can't just write to thesame file in a plain text. IPython-like approach seems most sane if we would like to share history file for all repls. Meanwhile, write_history_file() just save whole stuff, as before. Who will quit later - win. Edit: "gh-127495: Append to history file after every successful statement in PyREPL" seems to be more correct as a title. |
Misc/NEWS.d/next/Library/2025-04-08-14-50-39.gh-issue-127495.Q0V0bS.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Hmm, it seems the IPython stores statement rightbefore execution. Maybe I should adjust pr to do same? |
Yeah, maybe we shouldn't do
"Successful" suggests it executed without an exception. I would want to avoid suggesting too much. In fact, you are asserting that sleep() is in history while it's not a "successful" statement since the process got killed before it finished. Since this doesn't matter much, I think the current wording is less misleading.
I think that would be overdoing it. It would slow down getting the response from each command. Let's keep it as is. |
Yes, it seems working. One my concern here is that for multiple interpreters history will be interleaved. Maybe it's even better - all entries should be saved, just in some random order.
I openedhttps://discuss.python.org/t/87918 to discuss more big improvements in history handling.
Yes. I think we should kept more simple wording and just decide if we would like to store statement before (as now) of after it's executed.
Sure, I'll revert it (unless you had a second thought). But I think that above concern isn't valid. User can't distinguish this slowdown from time, required to execute and show output. Same will be true if statement will be saved after it's executed (as before). The only real difference: in the later case we might loose last statement (i.e. which crashed the interpreter). |
It is valid, you just don't see it. When you save the history before execution, the time to save is spentbefore the command is executed and therefore before the user sees output. When you save the history after execution, the user already sees the output and only then the time to save is spent. So it's only the prompt that returns later. If your $PYTHON_HISTORY is on a network-connected drive, this will make theexecution in the REPL appear slower to the user. |
Maybe) Below my 2c in defense.
But user has only one unquestionable signal that the output is complete - it's the new prompt. Youcan suspect some additional lag, but only if youknow how exactly looks like finished output. I'm not sure if there is a big difference (and compatibility with IPython looks more important to me), but possible lost of history entry seems more severe, than the lag you might guess. |
2762525
intopython:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.