Skip to content

gh-123024: Correctly prepare/restore around help and show-history commands #124485

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Jan 21, 2025
Merged

gh-123024: Correctly prepare/restore around help and show-history commands #124485

merged 8 commits into from
Jan 21, 2025

Conversation

lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Sep 25, 2024 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Sep 25, 2024
Closed
@lysnikolaou lysnikolaou added skip news topic-repl Related to the interactive shell needs backport to 3.13 bugs and security fixes labels Sep 25, 2024
emilyemorehouse
emilyemorehouse reviewed Sep 25, 2024
View reviewed changes
Copy link
Member

@emilyemorehouse emilyemorehouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, confirmed that it fixes the issue! Can we add a test? I was toying around with a quick one but got a bit stuck on getting a clean output, lmk if you want a starting point.

@lysnikolaou
Copy link
Member Author

Thanks for the review! I thought about tests for a while, but I couldn't figure out how to test the REPL_COMMANDS fix and then got distracted. If you've got something, I'd be happy to see it.

@emilyemorehouse
Merge branch 'main' into fix-help-double-prompt
4cdb3ae 
* main: (69 commits)
  Add "annotate" SET_FUNCTION_ATTRIBUTE bit to dis. (python#124566)
  pythongh-124412: Add helpers for converting annotations to source format (python#124551)
  pythongh-119180: Disallow instantiation of ConstEvaluator objects (python#124561)
  For-else deserves its own section in the tutorial (python#123946)
  Add 3.13 as a version option to the crash issue template (python#124560)
  pythongh-123242: Note that type.__annotations__ may not exist (python#124557)
  pythongh-119180: Make FORWARDREF format look at __annotations__ first (python#124479)
  pythonGH-58058: Add quick reference for `ArgumentParser` to argparse docs (pythongh-124227)
  pythongh-41431: Add `datetime.time.strptime()` and `datetime.date.strptime()` (python#120752)
  pythongh-102450: Add ISO-8601 alternative for midnight to `fromisoformat()` calls. (python#105856)
  pythongh-124370: Add "howto" for free-threaded Python (python#124371)
  pythongh-121277: Allow `.. versionadded:: next` in docs (pythonGH-121278)
  pythongh-119400:  make_ssl_certs: update reference test data automatically, pass in expiration dates as parameters python#119400  (pythonGH-119401)
  pythongh-119180: Avoid going through AST and eval() when possible in annotationlib (python#124337)
  pythongh-124448: Update Windows builds to use Tcl/Tk 8.6.15 (pythonGH-124449)
  pythongh-123884 Tee of tee was not producing n independent iterators (pythongh-124490)
  pythongh-124378: Update test_ttk for Tcl/Tk 8.6.15 (pythonGH-124542)
  pythongh-124513: Check args in framelocalsproxy_new() (python#124515)
  pythongh-101100: Add a table of class attributes to the "Custom classes" section of the data model docs (python#124480)
  Doc: Use ``major.minor`` for documentation distribution archive filenames (python#124489)
  ...
@emilyemorehouse
Add starter for test function
316e6db
@emilyemorehouse
Copy link
Member

Okay bummer news – I found a bug. Likely due to removing the context manager for suspending while help is running, the history from the help command leaks into the main prompt once help is exited.

@lysnikolaou
Copy link
Member Author

Okay bummer news – I found a bug.

Maybe that's an accidental feature? I can imagine a situation where people want to use an API, can't remember exactly how to do it, help for it and then auto-complete from history after exiting help. What do you think?

@emilyemorehouse
Copy link
Member

Ehh, I don't think so because the commands you're using in help aren't valid Python code (unless there's another feature in there that I'm missing). It looks something like this:

help-history-leak.mov

@lysnikolaou
Copy link
Member Author

A test for the double prompt after help case has been added. Unfortunately, writing a test for the double prompt after history case is harder, because the history is show from a pager in a subprocess.

I'd suggest to merge this and figure out how to test history after.

@pablogsal pablogsal mentioned this pull request Oct 2, 2024
Open
@pablogsal
Copy link
Member

See also #124874

@pablogsal
Copy link
Member

Landing this for now. We can add more tests in a separate issue. Thanks a lot @lysnikolaou !

@pablogsal pablogsal merged commit 5a9afe2 into python:main Jan 21, 2025
49 checks passed
@miss-islington-app
Copy link

Thanks @lysnikolaou for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 21, 2025
@lysnikolaou @emilyemorehouse
@pablogsal @miss-islington
pythongh-123024: Correctly prepare/restore around help and show-histo…
420d44b 
…ry commands (pythonGH-124485)

(cherry picked from commit 5a9afe2)

Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Emily Morehouse <emily@cuttlesoft.com>
Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jan 21, 2025

GH-129155 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jan 21, 2025
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x has failed when building commit 5a9afe2.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/2399) and take a look at the build logs.
  4. Check if the failure is related to this commit (5a9afe2) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/2399

Summary of the results of the build (if available):

Click to see traceback logs 
remote: Enumerating objects: 13, done.        
remote: Counting objects:   9% (1/11)        
remote: Counting objects:  18% (2/11)        
remote: Counting objects:  27% (3/11)        
remote: Counting objects:  36% (4/11)        
remote: Counting objects:  45% (5/11)        
remote: Counting objects:  54% (6/11)        
remote: Counting objects:  63% (7/11)        
remote: Counting objects:  72% (8/11)        
remote: Counting objects:  81% (9/11)        
remote: Counting objects:  90% (10/11)        
remote: Counting objects: 100% (11/11)        
remote: Counting objects: 100% (11/11), done.        
remote: Compressing objects:  10% (1/10)        
remote: Compressing objects:  20% (2/10)        
remote: Compressing objects:  30% (3/10)        
remote: Compressing objects:  40% (4/10)        
remote: Compressing objects:  50% (5/10)        
remote: Compressing objects:  60% (6/10)        
remote: Compressing objects:  70% (7/10)        
remote: Compressing objects:  80% (8/10)        
remote: Compressing objects:  90% (9/10)        
remote: Compressing objects: 100% (10/10)        
remote: Compressing objects: 100% (10/10), done.        
remote: Total 13 (delta 1), reused 1 (delta 1), pack-reused 2 (from 2)        
From https://github.com/python/cpython
 * branch                    main       -> FETCH_HEAD
Note: switching to '5a9afe23620aadea30013076d64686be8bf66f7e'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 5a9afe23620 gh-123024: Correctly prepare/restore around help and show-history commands (#124485)
Switched to and reset branch 'main'

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)
configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

In file included from ../../Python/ceval.c:909:
../../Python/generated_cases.c.h:875:31: warning: code will never be executed [-Wunreachable-code]
                for (int _i = oparg; --_i >= 0;) {
                              ^~~~~
../../Python/generated_cases.c.h:757:31: warning: code will never be executed [-Wunreachable-code]
                for (int _i = oparg*2; --_i >= 0;) {
                              ^~~~~
2 warnings generated.

configure: WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)
configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

../../Modules/_hacl/Lib_Memzero0.c:52:6: warning: "Your platform does not support any safe implementation of memzero -- consider a pull request!" [-W#warnings]
    #warning "Your platform does not support any safe implementation of memzero -- consider a pull request!"
     ^
../../Modules/_hacl/Lib_Memzero0.c:40:10: warning: unused variable 'len_' [-Wunused-variable]
  size_t len_ = (size_t) len;
         ^
2 warnings generated.

Found more than one new device: {'461613D7-607A-40C3-9E72-88D9F94D918D', '74ADCBEB-BE1F-4FB4-B94E-1189089A8516'}
make: *** [testios] Error 1

pablogsal added a commit that referenced this pull request Jan 21, 2025
@miss-islington @lysnikolaou
@emilyemorehouse @pablogsal
[3.13] gh-123024: Correctly prepare/restore around help and show-hist…
0ddcb61 
…ory commands (GH-124485) (#129155)

gh-123024: Correctly prepare/restore around help and show-history commands (GH-124485)
(cherry picked from commit 5a9afe2)

Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Emily Morehouse <emily@cuttlesoft.com>
Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilyemorehouse emilyemorehouse emilyemorehouse left review comments

@pablogsal pablogsal pablogsal approved these changes

@ambv ambv Awaiting requested review from ambv ambv is a code owner

Assignees
No one assigned
Labels
skip news topic-repl Related to the interactive shell
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lysnikolaou @emilyemorehouse @pablogsal @bedevere-bot