Move to array-based instead of linked list, simplify, add tests

[Tyriar]

xtermjs#1853

[PerBothner]

A higher-level note about using arrays is xtermjs#1853 (comment) .

The dispose implementations seem a bit fragile. What if dispose is called more than once? The indexOf returns -1, which means splice removes the last (most recent) element.

Instead of:

for (j = handlers.length - 1; j >= 0; j--)

I usually use the following idiom:

for (j = handlers.length; --j >= 0; )

[Tyriar]

I have no objection to using arrays instead of linked list. That is certainly simpler and more readable. The downside is it might use slightly more memory (an extra array per handler index) and be slightly slower (especially in the simple cases when "add" hasn't been called). If you and jerch are happy, I'm happy.

I don't think memory is an issue here since in the end this will just result in like < 100 more pointers. @jerch comments on perf? I think it's basically a vs a[a.length - 1] thoughts on the impact? If this won't work we will need to change things up so that they're typed, maybe moving to a plain LinkedListNode<CsiHandler>vs the old union type (CsiHandler | LinkedListNode<CsiHandler>) would make it easier to read.

The dispose implementations seem a bit fragile. What if dispose is called more than once? The indexOf returns -1, which means splice removes the last (most recent) element.

Good point, will fix this up.

I usually use the following idiom:

I changed this intentionally, I don't think we use that style anywhere else 😄

[PerBothner]

"I don't think memory is an issue here since in the end this will just result in like < 100 more pointers."

It's on the order 100 more array objects (typically single-element), which is a lot more than 100 pointers. Still minor.

[jerch]

@PerBothner, @Tyriar: My benchmarks show ~ 7% decrease for CSI commands (~ 2.5 MB/s slower in throughput measuring). Imho this is neglectible taking into account that the input stream hardly contains more than 10% CSI commands (only drawing intensive programs like curses based will send tons of them, but those operate most of the time on alt buffer with partial viewport updates and dont depend that much on total throughput).
Imho memory is no concern at this stage, as it adds far below 1k bytes per terminal instance and no one would really try to attach thousands of handlers.

So I think we are good to go with the easier to maintain array based approach. 👍

Edit: To keep things halfway in one place I will wait with the code review for the PR against xterm.js.