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

Terminal find match count is incorrect #148109

Closed
joyceerhl opened this issue Apr 26, 2022 · 14 comments · Fixed by #148222, xtermjs/xterm.js#3831 or #150494
Closed

Terminal find match count is incorrect #148109

joyceerhl opened this issue Apr 26, 2022 · 14 comments · Fixed by #148222, xtermjs/xterm.js#3831 or #150494
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug terminal-find Relating the terminal's find widget upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded windows VS Code on Windows issues
Milestone

Comments

@joyceerhl
Copy link
Contributor

Testing #148060

  1. Create a new terminal
  2. Run a script which logs the same text 2000 times
  3. Run a query, ✅ find widget says there are 2000 matches
  4. Resize the terminal (e.g. collapse and expand the side bar)
  5. 🐛 the number of matches is now less even though the same amount of scrollback is visible, moreover deleting and reentering the query doesn't reset the match count

image

@meganrogge meganrogge added bug Issue identified by VS Code Team member as probable bug terminal-find Relating the terminal's find widget upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Apr 26, 2022
@meganrogge meganrogge added this to the April 2022 milestone Apr 26, 2022
@meganrogge meganrogge added the upstream-issue-linked This is an upstream issue that has been reported upstream label Apr 26, 2022
@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2022

Maybe this isn't released yet, but I'm still getting the same problem in today's insiders.

@joyceerhl
Copy link
Contributor Author

Still repros for me :(

@joyceerhl joyceerhl reopened this Apr 27, 2022
@joyceerhl joyceerhl added the verification-found Issue verification failed label Apr 27, 2022
@meganrogge
Copy link
Contributor

Can one of you pls provide a gif? it's working for me with the above example as expected and with others

@meganrogge
Copy link
Contributor

search.mov

@meganrogge
Copy link
Contributor

You are both on Windows - I wonder if that has to do with this. Could someone on mac pls verify this?

@meganrogge meganrogge removed the verification-found Issue verification failed label Apr 27, 2022
@joyceerhl
Copy link
Contributor Author

Recording 2022-04-27 at 16 57 53

@meganrogge
Copy link
Contributor

meganrogge commented Apr 28, 2022

from @joyceerhl
Recording 2022-04-27 at 16 57 53

@meganrogge meganrogge modified the milestones: April 2022, May 2022 Apr 28, 2022
@meganrogge meganrogge added the windows VS Code on Windows issues label May 2, 2022
@meganrogge meganrogge removed their assignment May 6, 2022
@Tyriar
Copy link
Member

Tyriar commented May 25, 2022

Something weird going on here:

image

This is after pasting in " Hi Hi" many times, once the command wraps to the end the terminal freaks out (conpty reprinting), but at some point it breaks and find only finds one result. Finding the next works, but it still says 1 of 1.

@Tyriar
Copy link
Member

Tyriar commented May 26, 2022

More weird behavior
Recording 2022-05-26 at 13 18 56

@Tyriar
Copy link
Member

Tyriar commented May 26, 2022

All results aren't getting included when search results get highlighted
image

@Tyriar
Copy link
Member

Tyriar commented May 26, 2022

Problem here seems to be the search addon gets confused because translateToString considers the second character whitespace:

https://github.com/xtermjs/xterm.js/blob/fd5f5e3204843263c13bda850502d1d77be2b3e5/src/common/buffer/BufferLine.ts#L436

But in the search algorirthm it's treated as a null character, '' of length 0, as opposed to ' '.

@Tyriar Tyriar reopened this May 26, 2022
Tyriar added a commit that referenced this issue May 26, 2022
This brings in fixes to search xtermjs/xterm.js#3834 and xtermjs/xterm.js#3831

Fixes #148109
@Tyriar Tyriar mentioned this issue May 26, 2022
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 26, 2022
@joyceerhl joyceerhl added the verified Verification succeeded label Jun 2, 2022
@joyceerhl
Copy link
Contributor Author

Still reproing this. Initially:

image

After making the terminal panel taller:

image

@joyceerhl joyceerhl added verification-found Issue verification failed and removed verified Verification succeeded labels Jun 2, 2022
@joyceerhl joyceerhl reopened this Jun 2, 2022
@vscodenpa vscodenpa removed the insiders-released Patch has been released in VS Code Insiders label Jun 2, 2022
@Tyriar
Copy link
Member

Tyriar commented Jun 2, 2022

@joyceerhl this is on Windows? That's actually acting as expected because of the way conpty works, it ends up losing lines just above the bottom of the buffer.

@Tyriar
Copy link
Member

Tyriar commented Jun 3, 2022

Pretty sure you were seeing #150886 so closing this again for verification

@Tyriar Tyriar closed this as completed Jun 3, 2022
@Tyriar Tyriar removed the verification-found Issue verification failed label Jun 3, 2022
@joyceerhl joyceerhl added the verified Verification succeeded label Jun 3, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2022
lemanschik pushed a commit to code-oss-dev/code that referenced this issue Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug terminal-find Relating the terminal's find widget upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded windows VS Code on Windows issues
Projects
None yet
5 participants