Current search index changing when new lines are written

[AndreSilva1993]

I began using the search addon to benefit from the newly implemented onDidChangeResults which allows us to get the currently highlighted match out of the total number of matches. The only thing that I'm wondering is why this line is being called as it causes this behaviour:

https://user-images.githubusercontent.com/10833491/177368718-63a0f761-02dd-441f-a6a7-fd2021282ef0.mov

For context I'm watching a stress-test Docker container which writes a lot of new lines and I'm pressing the Enter key which calls the search addon's findNext method. With that line, the current index get constantly overwritten with the total size of results? I'm a bit confused if I'm doing something wrong, if adding lines in the middle of a search might break this? Let me know if you need more information.

Details

• Browser and browser version: Electron 17.2.0
• OS version: macOS Monterey 12.4
• xterm.js version: 4.19.0

Steps to reproduce

Created a sandbox to demonstrate what I'm saying. Even though the findNext is correctly highlighting the next match, the X of Y is constantly incorrect.

[starpit]

Curious: how are you using the onDidChangeResults API? I cannot get it to call me back, so I must be missing something obvious?

In this codesandbox, the "Got search update" console message never appears...

[AndreSilva1993]

Changed your codesandbox, you have to enable decorations on the findNext / findPrevious as stated here.

[starpit]

Nice! It wasn't clear to me whether decorations were enabled by default, since, without "decorations" enabled, I still see some... decorations in the xterm canvas, at least for the current hit.

[AndreSilva1993]

@Tyriar

[Tyriar]

That line updates the index only when output comes in the buffer is resized:

https://github.com/xtermjs/xterm.js/blob/0befb9c2513af83bd1fecf9f8c15b1330a2a70c5/addons/xterm-addon-search/src/SearchAddon.ts#L80-L81

So you should only be seeing this if there are data changes coming in, I agree the behavior is a little off right now. Ideally it would set _resultIndex to whatever the previously selected result was (the index may have changed if the buffer's scrollback is full for example).

[AndreSilva1993]

Exactly that! I'm using this to display the logs from a running Docker container and having the the current match always changing basically renders this feature unusable to me since we're constantly receiving new data. So I think in theory, like you said, the index should be preserved.