Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menu completion doesn't work properly when scrolling happens #2948

Closed
3 tasks done
daxian-dbw opened this issue Nov 4, 2021 · 2 comments · Fixed by #2949
Closed
3 tasks done

Menu completion doesn't work properly when scrolling happens #2948

daxian-dbw opened this issue Nov 4, 2021 · 2 comments · Fixed by #2949
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Resolution-Fixed

Comments

@daxian-dbw
Copy link
Member

daxian-dbw commented Nov 4, 2021

Prerequisites

  • Write a descriptive title.
  • Make sure you are able to repro it on the latest released version
  • Search the existing issues, especially the pinned issues.

Exception report

No exception report.

Screenshot

Animation

Environment data

PS Version: 7.1.3
PS HostName: ConsoleHost (Windows Terminal)
PSReadLine Version: 2.2.0-beta4
PSReadLine EditMode: Windows
OS: 10.0.19041.1 (WinBuild.160101.0800)
BufferWidth: 133
BufferHeight: 30

Steps to reproduce

  1. Press Enter a number of times until the prompt is at the last line of the terminal window
  2. Copy and paste the following command to the terminal: $instMods = (Get-InstalledModule | ConvertTo-Html | pandoc12.exe --from=html-auto_identifiers-native_spans-native_divs --to=gfm-gfm_auto_identifiers) -replace [reg] $env:HOMEPATH ,"`$env:HOMEPATH"
  3. Move cursor to the closing bracket ] of the [reg].
  4. Press Ctrl+Space to trigger the menu completion
  5. Move the selection to RegisterPSSessionConfigurationCommand

Here is the GIF for reproducing this:

Animation

Expected behavior

The menu completion should work correctly when moving selection around, even though necessary screen scrolling is needed.

Actual behavior

As is shown, the menu completion rendering is broken in this case, where screen buffer scrolling was needed when initially rendering the menu.

@ghost ghost added the Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. label Nov 4, 2021
@daxian-dbw daxian-dbw added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. labels Nov 4, 2021
@daxian-dbw
Copy link
Member Author

The root cause of the issue in this case lies in the following code:

PreviousTop = bufferEndPoint.Y;

// if the menu has moved, we need to clear the lines under it
if (bufferEndPoint.Y < PreviousTop)

PreviousTop should be set with the actual Top value before changing the Top value. And to decide whether menu has moved, we should compare the current Top with PreviousTop, instead of using bufferEndPoint.Y, which is a snapshot value that doesn't get updated when screen buffer scrolls.

@ghost ghost added the In-PR A PR is opened targeting the issue label Nov 4, 2021
@daxian-dbw daxian-dbw added this to the 2.2.0-Consider milestone Nov 4, 2021
@ghost ghost added Resolution-Fixed and removed In-PR A PR is opened targeting the issue labels Nov 4, 2021
@ghost
Copy link

ghost commented Jan 7, 2022

🎉 This issue was addressed in 2949, which has now been successfully released in v2.2.0-beta5. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Resolution-Fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant