Skip to content

gh-118878: Pyrepl: show completions menu below the current line #118939

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 16 commits into from
Jan 21, 2025
Merged

gh-118878: Pyrepl: show completions menu below the current line #118939

merged 16 commits into from
Jan 21, 2025

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented May 11, 2024 •
edited
Loading

As proposed in #118878, it feels much more natural to me to display the completions menu below the current line, not above. The main advantage of this is that the UI is less janky,
since the current line that is being edited no longer jumps up and down.

This also allows fixing the behaviour of arrow keys when the menu is displayed, described in #119257.
The current behaviour is still not great when tab completing in the middle of multiline edit, but it is at least not worse as before AFAIK. I think ultimately this should be solved by separating out the completions menu, like ipython does it, but that seems outside of scope for this PR.

TODO:

  • Tests!
  • Add screencast to demonstrate the new behaviour

Closes #119257. Closes #118878

@danielhollas
pythongh-118877: Fix AssertionError crash in pyrepl
dc98b55 
A crash of the new pyrepl was triggered by
pressing up arrow when tab completion menu was displayed.
@bedevere-app bedevere-app bot mentioned this pull request May 11, 2024
Closed
danielhollas
danielhollas commented May 11, 2024
View reviewed changes
@danielhollas
pythongh-118878: pyrepl: Show completion menu below current line
eae92aa 
The main advantage of this is that the behaviour is less janky,
since the current line no longer jumps up and down.

This also allows fixing the behaviour of arrow keys when the menu is
displayed.
@danielhollas danielhollas force-pushed the pyrepl/completions-below branch from a1d95c9 to eae92aa Compare May 11, 2024 07:21
@danielhollas
WIP: Add test
9e2170c 
This test does not work yet :-(
@danielhollas danielhollas marked this pull request as ready for review May 20, 2024 19:39
@danielhollas danielhollas mentioned this pull request May 20, 2024
Closed
danielhollas
danielhollas commented May 20, 2024
View reviewed changes
@danielhollas danielhollas force-pushed the pyrepl/completions-below branch from 99af855 to dd90c2e Compare May 22, 2024 14:58
@danielhollas
Copy link
Contributor Author

@pablogsal would you mind adding the topic-repl label to this PR?

I am not sure there is enough time to get this in for next beta, but I am happy to work on this further if this is something people are happy with.

@pablogsal pablogsal added the topic-repl Related to the interactive shell label Jun 3, 2024
@pablogsal
Copy link
Member

CC @lysnikolaou as he was working on something like this for a bugfix

lysnikolaou
lysnikolaou reviewed Jun 4, 2024
View reviewed changes
Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @danielhollas!

@pablogsal
Copy link
Member

pablogsal commented Jun 4, 2024 •
edited
Loading

This has the problem that if the cursor is on the last line of the screen, completions are not shown (only when editing a multi-line block)

@danielhollas danielhollas force-pushed the pyrepl/completions-below branch from f29168e to af90634 Compare June 24, 2024 05:38
@danielhollas
Copy link
Contributor Author

This has the problem that if the cursor is on the last line of the screen, completions are not shown (only when editing a multi-line block)

@pablogsal was your comment meant on my version of the code or on @lysnikolaou patch proposed in #118939 (comment)? I can reproduce the issue you describe with his patch but I cannot with the current version of PR.

I merged in main and fixed up a test, and things still seem to be working.

pablogsal
pablogsal reviewed Jul 13, 2024
View reviewed changes
@danielhollas danielhollas force-pushed the pyrepl/completions-below branch from f1e76ce to aa9a3a3 Compare July 14, 2024 22:33
@ferdnyc ferdnyc mentioned this pull request Nov 15, 2024
Closed
@pablogsal pablogsal merged commit 29caec6 into python:main Jan 21, 2025
46 checks passed
@pablogsal
Copy link
Member

Thanks for your contribution @danielhollas

@danielhollas danielhollas deleted the pyrepl/completions-below branch January 21, 2025 21:10
@danielhollas
Copy link
Contributor Author

danielhollas commented Jan 21, 2025 •
edited
Loading

Thanks for merging @pablogsal!

I wonder if this should be back ported to 3.13? It is a somewhat large behavioural change but it does fix a couple of small bugs affecting cursor positioning while the completions menu is displayed (#119257 and #126851)

@pablogsal
Copy link
Member

@ambv what do you think?

@pablogsal pablogsal added the needs backport to 3.13 bugs and security fixes label Jan 21, 2025
@miss-islington-app
Copy link

Thanks @danielhollas 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
@danielhollas @pablogsal @miss-islington
pythongh-118878: Pyrepl: show completions menu below the current line (…
0c94af4 
…pythonGH-118939)

(cherry picked from commit 29caec6)

Co-authored-by: Daniel Hollas <daniel.hollas@bristol.ac.uk>
Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jan 21, 2025

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

@pablogsal
Copy link
Member

I will create the backport to see if it applies cleanly

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jan 21, 2025
ambv pushed a commit that referenced this pull request Jan 23, 2025
@miss-islington @danielhollas @pablogsal
[3.13] gh-118878: Pyrepl: show completions menu below the current line (
5c102a1 
GH-118939) (#129161)

gh-118878: Pyrepl: show completions menu below the current line (GH-118939)

(cherry picked from commit 29caec6)

Co-authored-by: Daniel Hollas <daniel.hollas@bristol.ac.uk>
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

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

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

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

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

Successfully merging this pull request may close these issues.

pyrepl: Cursor behaviour during tab-completion Tab completion behaviour in new REPL
3 participants
@danielhollas @pablogsal @lysnikolaou