This request may impact performance, this should be evaluated.
Plan issue: #705
Hi @Tyriar
I'm trying to understand this enhancement. Currently the search addon only supports find next & find previous. what is your expectation regarding the number of results?
@noamyogev84 currently the search addon returns a boolean, I think we should return a number instead:
It's probably also a good idea to return only 0 or 1 unless a new option to return number of results (countResults: boolean?) is set:
Hi @Tyriar,
Although I have some pretty straight forward implementation for this, I do have some wonderings:
Why do we need this second iteration? (line 48)
https://github.com/xtermjs/xterm.js/blob/9e446a9a0e9f62899c450b28d78877b81a19724d/src/addons/search/SearchHelper.ts#L39-L55
Currently for multiple matches in the same line it only returns (and counts) the first occurrence. do you think this should first be redesigned before we return the number of results? In reference to #1654
@noamyogev84 sorry about the delay. 1: the second iteration is because findNext does not start from the top:
2: I think the code to collect the total should look a little different to the current code, if it's true we want to continue searching until we've wrapped around to the start.
Most helpful comment
@noamyogev84 currently the search addon returns a boolean, I think we should return a number instead:
https://github.com/xtermjs/xterm.js/blob/9e446a9a0e9f62899c450b28d78877b81a19724d/src/addons/search/SearchHelper.ts#L26
It's probably also a good idea to return only 0 or 1 unless a new option to return number of results (
countResults: boolean?) is set:https://github.com/xtermjs/xterm.js/blob/9e446a9a0e9f62899c450b28d78877b81a19724d/src/addons/search/Interfaces.ts#L24-L28