Open Tyriar opened 3 years ago
Would a comparo API be more stable (though less general)?
Like this?
interface IBufferCell {
attributesEqual(other: IBufferCell): boolean;
}
It would be more stable, I was thinking of using the fg/bg numbers in the serialize addon but maybe that's a bad idea and just this equals function would improve the code quality a bunch (without introducing somewhat unstable api into the addon).
@JavaCS3 @mmis1000 feedback welcome as you are most familiar with the serialize addon
IMO the attributesEqual
function should do the trick for most cases. I think the most common use case is to group / collect subsequent cells that share the same attributes.
Note: When implementing this feature, we have to make sure that the extended attributes (e.g. underline color) are also taken into account. This is easy to miss when only comparing the raw values.
I though batch compare only the attributes should be enough, because there isn't any case(?) that only part of the attributes changes naturally.
AFAIK, They only change when either
So they either
And a method to differ the 1st and 2nd case would be helpful, as there is no efficient way to differ it now (the giant equalFlags
function).
I though batch compare only the attributes should be enough, because there isn't any case(?) that only part of the attributes changes naturally.
Batch compare - do you mean like comparing multiple cells at once, e.g. in a row? Since comparing cells over and over is somewhat perf sensitive, this is indeed a good idea. This could be done much faster at Bufferline
level with a forward seach like "gimme pos of next changing attrs":
public indexOfNextAttribs(currentIdx: number = 0): number {
const initial = this.loadCell(currentIdx, new CellData());
const hasExtended = initial.bg & BgFlags.HAS_EXTENDED;
for (let i = currentIdx + 1; i < this.length; ++i) {
if (initial.fg != this._data[i * CELL_SIZE + Cell.FG] || initial.bg != this._data[index * CELL_SIZE + Cell.BG]) {
return i;
}
if (hasExtended) {
// here a reference equality check is enough
if (this._extendedAttrs[i] !== this._extendedAttrs[currentIdx]) {
return i;
}
}
}
return -1;
}
This should be the fastest I can think of for scanning through cells in a line (untested). But that API is a bit weird to use somewhat mimicking the behavior of indexOf
, which ppl often have problems to grasp. So just see this as an idea to keep things fast...
@jerch I mean compare multi attributes (bold, italic, reverse...etc with or without bg/fg) at same time. Just like mofux proposed.
I haven't thought about multi cell, but that could also be a good addition due to things I stated above (there are no case that the attributes changes spontaneously except full reset). And now you can just skip ahead to where you really need to handle instead of spending time diff two identical attributes.
But even if that exist, a method to compare only two slots' attribute is probably still required. Because you still need a quick way to determine whether you should ever bother to look into the big tones of attributes or you can just check bg/fg after you find which cell to diff (And in most case you only need to handle bg/fg anyway).
BTW, if something like .indexOf
exists, we probably also need something like .slice(start, end)
. Or we still need to load the cell one by one for texts
We've seen the code some users use to diff attributes of a cell and it's pretty ugly, we could just expose the raw fg/bg numbers that contain these and add some caveats to the jsdoc to make sure they're used correctly for example compatibility across versions isn't guaranteed
We would want this to be forward looking in case we add a third number.
Context https://github.com/xtermjs/xterm.js/issues/1883#issuecomment-907516829