Provide an API to diff cell attributes

[Tyriar]

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 #1883 (comment)

[starpit]

Would a comparo API be more stable (though less general)?

[Tyriar]

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).

xterm.js/addons/xterm-addon-serialize/src/SerializeAddon.ts

Lines 58 to 77 in cee8f67

	function equalFg(cell1: IBufferCell, cell2: IBufferCell): boolean {
	return cell1.getFgColorMode() === cell2.getFgColorMode()
	&& cell1.getFgColor() === cell2.getFgColor();
	}
	function equalBg(cell1: IBufferCell, cell2: IBufferCell): boolean {
	return cell1.getBgColorMode() === cell2.getBgColorMode()
	&& cell1.getBgColor() === cell2.getBgColor();
	}
	function equalFlags(cell1: IBufferCell, cell2: IBufferCell): boolean {
	return cell1.isInverse() === cell2.isInverse()
	&& cell1.isBold() === cell2.isBold()
	&& cell1.isUnderline() === cell2.isUnderline()
	&& cell1.isBlink() === cell2.isBlink()
	&& cell1.isInvisible() === cell2.isInvisible()
	&& cell1.isItalic() === cell2.isItalic()
	&& cell1.isDim() === cell2.isDim();
	}

[Tyriar]

@JavaCS3 @mmis1000 feedback welcome as you are most familiar with the serialize addon

[mofux]

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.

[mmis1000]

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

• user change them with control code
• a reset control code is encountered
• a line break cause the following cell to be null cells.

So they either

1. don't change (fully match)
2. partial change (user issued a control code)
3. reset back to default (which can be checked by whether the following cell is default and thus don't really matter)

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).

[jerch]

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

[mmis1000]

@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