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.

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

Handle screen buffer scrolling correctly for dynamic in-line help rendering #2951

Merged
merged 1 commit into from
Nov 4, 2021
Merged

Handle screen buffer scrolling correctly for dynamic in-line help rendering #2951

merged 1 commit into from
Nov 4, 2021

Conversation

daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Nov 4, 2021 •
edited
Loading

PR Summary

Fix #2950

Major changes:

  1. Update the variable name to make it more clear in meaning.
  2. Adjust for buffer scrolling in case there are extra physical lines.

It works as expected after the fix:

Animation

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Make sure you've added one or more new tests
    - Cannot add tests because we cannot mimic buffer scrolling with the testing console, but I manually validated the fix
  • Make sure you've tested these changes in terminals that PowerShell is commonly used in (i.e. conhost.exe, Windows Terminal, Visual Studio Code Integrated Terminal, etc.)
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed at PowerShell-Docs
      • Doc Issue filed:
Microsoft Reviewers: Open in CodeFlow

@daxian-dbw
Copy link
Member Author

@adityapatwardhan @andschwa, can one of you please do a sanity check for me? Thanks!

andyleejordan
andyleejordan approved these changes Nov 4, 2021
View reviewed changes
Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

PSReadLine/DynamicHelp.cs
if (extra > 0)
{
// Extra physical lines may cause buffer to scroll up.
AdjustForPossibleScroll(extra);
Copy link
Member

Choose a reason for hiding this comment

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

👍

@daxian-dbw
Copy link
Member Author

Thanks @andschwa!

@daxian-dbw daxian-dbw merged commit e02b0da into PowerShell:master Nov 4, 2021
@daxian-dbw daxian-dbw deleted the help branch November 4, 2021 22:44
@adityapatwardhan
Copy link
Member

LGTM

@ghost
Copy link

ghost commented Jan 7, 2022

🎉 v2.2.0-beta5 has been released which incorporates this pull request. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyleejordan andyleejordan andyleejordan approved these changes

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dynamic help rendering behaves incorrectly when buffer scrolling is required
3 participants
@daxian-dbw @adityapatwardhan @andyleejordan