Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
ambv merged 10 commits intopython:mainfromskirpichev:append-repl-history/127495
Apr 27, 2025

Conversation

skirpichev
Copy link
Member

@skirpichevskirpichev commentedApr 8, 2025
edited by bedevere-appbot
Loading

@skirpichevskirpichev marked this pull request as ready for reviewApril 9, 2025 01:22
@ambvambv changed the titlegh-127495: append to history file after every successful statement in new replgh-127495: Append to history file after every statement in PyREPLApr 10, 2025
@ambvambv added the topic-replRelated to the interactive shell labelApr 10, 2025
@ambv
Copy link
Contributor

Looks good to me. I'd like the following questions clarified before we proceed:

  • will bracketed pasting avoid appending to history until it's finished? (it should);
  • will this work with multiple concurrently open REPLs? (looks like it would, although the written history will be now intermingled, which I guess is better than entirely losing history of the REPL that shut down sooner).

@skirpichev
Copy link
MemberAuthor

skirpichev commentedApr 10, 2025
edited
Loading

will bracketed pasting avoid appending to history until it's finished? (it should);

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.)

will this work with multiple concurrently open REPLs? (looks like it would, although the written history will be now intermingled, which I guess is better than entirely losing history of the REPL that shut down sooner).

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.

@skirpichev
Copy link
MemberAuthor

Hmm, it seems the IPython stores statement rightbefore execution. Maybe I should adjust pr to do same?

@skirpichevskirpichev changed the titlegh-127495: Append to history file after every statement in PyREPLgh-127495: Append to history file before every statement in PyREPLApr 15, 2025
@skirpichevskirpichev requested a review fromambvApril 15, 2025 05:24
@ambv
Copy link
Contributor

Meanwhile, write_history_file() just save whole stuff, as before. Who will quit later - win.

Yeah, maybe we shouldn't dowrite_history_file() anymore sinceappend_history_file() ensures the file is up-to-date as it is? You're right that we can discuss this in a separate issue.

"#127495: Append to history file after every successful statement in PyREPL" seems to be more correct as a title.

"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.

Hmm, it seems the IPython stores statement right before execution.

I think that would be overdoing it. It would slow down getting the response from each command. Let's keep it as is.

@skirpichev
Copy link
MemberAuthor

maybe we shouldn't do write_history_file() anymore since append_history_file() ensures the file is up-to-date as it 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.

You're right that we can discuss this in a separate issue.

I openedhttps://discuss.python.org/t/87918 to discuss more big improvements in history handling.

Since this doesn't matter much, I think the current wording is less misleading.

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.

I think that would be overdoing it. It would slow down getting the response from each command.

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).

@ambv
Copy link
Contributor

But I think that above concern isn't valid.

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.

@skirpichev
Copy link
MemberAuthor

It is valid, you just don't see it.

Maybe) Below my 2c in defense.

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.

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.

@skirpichevskirpichev changed the titlegh-127495: Append to history file before every statement in PyREPLgh-127495: Append to history file after every statement in PyREPLApr 19, 2025
@ambvambv merged commit2762525 intopython:mainApr 27, 2025
47 checks passed
@skirpichevskirpichev deleted the append-repl-history/127495 branchApril 27, 2025 13:36
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@ambvambvAwaiting requested review from ambvambv is a code owner

Assignees
No one assigned
Labels
topic-replRelated to the interactive shell
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@skirpichev@ambv

[8]ページ先頭

©2009-2025 Movatter.jp