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

Fix the 'ArgumentOutOfRangeException' and 'IndexOutOfRangeException' when searching history with ctrl+r or ctrl+s #1256

Merged
merged 3 commits into from
Dec 21, 2019

Conversation

daxian-dbw
Copy link
Member

Fix #1083

When using ctrl+r and ctrl+s, it's not uncommon to find a history item with multi-line text, and the matching string in the text, where the cursor is supposed to be moved to, will be scrolled up-off the buffer after rendering.
After rendering, if the cursor position to be set is already scrolled up-off the buffer (point.Y < 0), we should just move the cursor to the left-most position of the first line in buffer.

Before this fix, only 2 very specific scenarios where this may happen was covered. This fix replaces that with a general solution.

…when searching history with ctrl+r or ctrl+s
@msftrncs
Copy link
Collaborator

This seems to also fix the F8/Shift-F8 issue in #1202, except the cursor's final resting place seems to be a bit baffling, sometimes its on the first line of the buffer, sometimes on the middle of a line in the middle of the buffer, or if the history item returned is again small enough to fit entirely in the buffer, it will be on the last line of the history, where normally it would be on the first line. .... it would seem this is a result updating _current, the search still seems to work correctly, but it hadn't expected the cursor to get moved away from the text entry point.

In this case, as I have run some various Pester tests in the console, I used describe as the feed value for an F8 search and it returned several long scripts. I sampled this in Windows Terminal and VS Code's terminal.

@daxian-dbw
Copy link
Member Author

@msftrncs Good catch! I pushed another commit to fix the cursor issue in HistorySearchBackward and HistorySearchForward, can you please take another look?

Copy link
Collaborator

@msftrncs msftrncs left a comment

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw daxian-dbw merged commit d648aed into PowerShell:master Dec 21, 2019
@daxian-dbw daxian-dbw deleted the backwardsearch branch December 21, 2019 07:01
@daxian-dbw
Copy link
Member Author

Thanks @msftrncs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bck-i-search (Ctrl-R) quits accepting input when it finds a very long command
2 participants