FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-118878: Pyrepl: show completions menu below the current line by danielhollas · Pull Request #118939 · python/cpython · GitHub

/ cpython Public

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

Merged
pablogsal merged 16 commits into
python:mainfrom
danielhollas:pyrepl/completions-below
Jan 21, 2025
ConversationCommits (16)ChecksFiles changed
Merged

gh-118878: Pyrepl: show completions menu below the current line#118939
pablogsal merged 16 commits into
python:mainfrom
danielhollas:pyrepl/completions-below

Conversation

danielhollas commented May 11, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

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 Bot mentioned this pull request May 11, 2024
Closed
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 force-pushed the pyrepl/completions-below branch from a1d95c9 to eae92aa Compare May 11, 2024 07:21
WIP: Add test
9e2170c 
This test does not work yet :-(
danielhollas marked this pull request as ready for review May 20, 2024 19:39
danielhollas mentioned this pull request May 20, 2024
Closed
danielhollas commented May 20, 2024
View reviewed changes
Comment thread Lib/_pyrepl/commands.py
danielhollas force-pushed the pyrepl/completions-below branch from 99af855 to dd90c2e Compare May 22, 2024 14:58

danielhollas commented Jun 3, 2024

Copy link
Copy Markdown
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 added the topic-repl Related to the interactive shell label Jun 3, 2024

pablogsal commented Jun 3, 2024

Copy link
Copy Markdown
Member

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

lysnikolaou reviewed Jun 4, 2024
View reviewed changes

lysnikolaou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for working on this @danielhollas!

Comment thread Lib/_pyrepl/completing_reader.py

pablogsal commented Jun 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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 force-pushed the pyrepl/completions-below branch from f29168e to af90634 Compare June 24, 2024 05:38

danielhollas commented Jun 24, 2024

Copy link
Copy Markdown
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 reviewed Jul 13, 2024
View reviewed changes
Comment thread Lib/test/test_pyrepl/test_pyrepl.py Outdated
danielhollas force-pushed the pyrepl/completions-below branch from f1e76ce to aa9a3a3 Compare July 14, 2024 22:33
ferdnyc mentioned this pull request Nov 15, 2024
Closed
pablogsal merged commit 29caec6 into python:main Jan 21, 2025

pablogsal commented Jan 21, 2025

Copy link
Copy Markdown
Member

Thanks for your contribution @danielhollas

danielhollas deleted the pyrepl/completions-below branch January 21, 2025 21:10

danielhollas commented Jan 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 commented Jan 21, 2025

Copy link
Copy Markdown
Member

@ambv what do you think?

pablogsal added the needs backport to 3.13 bugs and security fixes label Jan 21, 2025

miss-islington-app Bot commented Jan 21, 2025

Copy link
Copy Markdown

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
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 Bot commented Jan 21, 2025

Copy link
Copy Markdown

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

pablogsal commented Jan 21, 2025

Copy link
Copy Markdown
Member

I will create the backport to see if it applies cleanly

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
[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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ambv Awaiting requested review from ambv ambv is a code owner
lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
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

Back | FazBrowse Home | New Git URL