Fix final-space handling of translateToString cache

[PerBothner]

This trimEnd call in in translateToString:

  if (isCanonical && this._cacheValid) {
    if (trimRight) {
      return this._cacheTrimmed ? this._cache : this._cache.trimEnd();

incorrectly trims off significant (explicitly witten) whitespace at the end of a line.

It fails if you add these lines to the end of the 'should cache canonical string translations' test in BufferLine.test.ts:

      line.setCell(3, createCellData(1, ' ', 1));
      assert.equal(line.translateToString(false, undefined, undefined, undefined), 'abc  ');
      assert.equal(line.translateToString(true, undefined, undefined, undefined), 'abc ');

My proposed fix is to simplify the test in translateToString to just:

    if (isCanonical && this._cacheValid && trimRight === this._cacheTrimmed) {
      return this._cache;
    }

Relatedly: I believe only the trimRight == true case is useful. The only examples of trimRight == false I could find were in test case except for one place in the addon-search which is trying to get the translated string for the "logical line"; that should be redone if string caching is done at the LogicalIne level, as I believe it should.

[jerch]

I saw that when I rewrote the line caching, but did not think that deliberately appending SP is a thing.

I agree that the trimmed version is the more useful here. If we need to restore full versions from cache values at some point, we could save the trimmedLength along and just return _cache + ' '.repeat(cols - trimmedLength).

Edit: Could you change that in only caching the trimmed version?

[PerBothner]

did not think that deliberately appending SP is a thing.

Perhaps not - but xterm.js implements a lot of corner cases that no-one actually uses. And this can be user-visible even: Type an input line ending it with some spaces, and then try selecting and copying it. (How it works depends on the terminal, and whether the input is raw or canonical; the point is that it is user-visible, so we should probably be consistent.)

Could you change that in only caching the trimmed version?

That might be reasonable but I think the _chars proposal is more interesting. Regardless, any caching should be done at the LogicalLine level, so I suggest the current PR is good enough for now.

[PerBothner]

did not think that deliberately appending SP is a thing.

More critically: When merging master into PR #5797 I ran into some testsuite failures that were related to this. Didn't gig into exactly why my branch failed while master didn't, but it was related to the string cache, and this seemed a good fix in both master and my branch.