Xterm.js: Search addon: Return the number of results of a search

Created on 5 Sep 2018  路  4Comments  路  Source: xtermjs/xterm.js

This request may impact performance, this should be evaluated.

Plan issue: #705

areaddosearch good first issue help wanted typenhancement

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

All 4 comments

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?

  • how should it be outputted?

@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

Hi @Tyriar,
Although I have some pretty straight forward implementation for this, I do have some wonderings:

  1. Why do we need this second iteration? (line 48)
    https://github.com/xtermjs/xterm.js/blob/9e446a9a0e9f62899c450b28d78877b81a19724d/src/addons/search/SearchHelper.ts#L39-L55

  2. 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:

  • First iteration: Search from the top of the _viewport_ to the bottom of the buffer
  • Second iteration: Search from the top of the buffer to the top of the viewport

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.

Was this page helpful?
0 / 5 - 0 ratings