-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fixes improve search addon behavior when there are > 1000 results #4504
Conversation
Incremental means if you search "test" and then search "a", the search will find "testa" if the current search term's next letter is a, as opposed to non-incremental where it searches from the end of the current search term. This is typing in vscode's find widget (incremental) vs pressing enter (non-incremental). |
So it's broken in master branch?
|
Yes it does seem to be broken, incremental should not jump to the result at the bottom |
I fixed the incremental code but the flag is redundant
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior seems mostly correct now, I do notice the following issues though:
- Pressing shift, ctrl or alt will do a non-incremental search
- Doing an incremental search will not apply the white activeMatchBorder defined here https://github.com/jeanp413/xterm.js/blob/a1e9105ce78154eafce3500ace119843a72c46f7/demo/client.ts#L146
Are you referring to this ?, as I mentioned the incremental flag does not have any effect now (it can be removed in a separate PR), if there's a selection in the terminal then search will be incremental, but I also found a bug I fixed in the last commit and added a test, so maybe that was it
It's a bug in the decorations rendering, it does not take into account if it's on the |
I thinks that's an issue with the dom events used in the demo, it's always using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👏
Fixes #4444
If there's more than 1k matches, it will highlight the first 1k matches