fix trimming of right side

src/Buffer.test.ts

@@ -508,11 +508,10 @@ describe('Buffer', () => {
       // the dangling last cell is wrongly added in the string
       // --> fixable after resolving #1685
       terminal.writeSync(input);
-      // TODO: reenable after fix
-      // const s = terminal.buffer.contents(true).toArray()[0];
-      // assert.equal(input, s);
+      const s = terminal.buffer.iterator(true).next().content;
+      assert.equal(input, s);
       for (let i = 10; i < input.length; ++i) {
-        const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i + 1); // TODO: remove +1 after fix
+        const bufferIndex = terminal.buffer.stringIndexToBufferIndex(0, i);
         const j = (i - 0) << 1;
         assert.deepEqual([(j / terminal.cols) | 0, j % terminal.cols], bufferIndex);
       }

src/Buffer.ts

@@ -17,9 +17,13 @@ export const CHAR_DATA_WIDTH_INDEX = 2;
 export const CHAR_DATA_CODE_INDEX = 3;
 export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1
 
-export const NULL_CELL_CHAR = ' ';
+export const NULL_CELL_CHAR = '';
 export const NULL_CELL_WIDTH = 1;
-export const NULL_CELL_CODE = 32;
+export const NULL_CELL_CODE = 0;
+
+export const WHITESPACE_CELL_CHAR = ' ';
+export const WHITESPACE_CELL_WIDTH = 1;
+export const WHITESPACE_CELL_CODE = 32;
 
 /**
  * This class represents a terminal buffer (an internal state of the terminal), where the
@@ -272,64 +276,11 @@ export class Buffer implements IBuffer {
    * @param endCol The column to end at.
    */
   public translateBufferLineToString(lineIndex: number, trimRight: boolean, startCol: number = 0, endCol: number = null): string {
-    // Get full line
-    let lineString = '';
     const line = this.lines.get(lineIndex);
     if (!line) {
       return '';
     }
-
-    // Initialize column and index values. Column values represent the actual
-    // cell column, indexes represent the index in the string. Indexes are
-    // needed here because some chars are 0 characters long (eg. after wide
-    // chars) and some chars are longer than 1 characters long (eg. emojis).
-    let startIndex = startCol;
-    // Only set endCol to the line length when it is null. 0 is a valid column.
-    if (endCol === null) {
-      endCol = line.length;
-    }
-    let endIndex = endCol;
-
-    for (let i = 0; i < line.length; i++) {
-      const char = line.get(i);
-      lineString += char[CHAR_DATA_CHAR_INDEX];
-      // Adjust start and end cols for wide characters if they affect their
-      // column indexes
-      if (char[CHAR_DATA_WIDTH_INDEX] === 0) {
-        if (startCol >= i) {
-          startIndex--;
-        }
-        if (endCol > i) {
-          endIndex--;
-        }
-      } else {
-        // Adjust the columns to take glyphs that are represented by multiple
-        // code points into account.
-        if (char[CHAR_DATA_CHAR_INDEX].length > 1) {
-          if (startCol > i) {
-            startIndex += char[CHAR_DATA_CHAR_INDEX].length - 1;
-          }
-          if (endCol > i) {
-            endIndex += char[CHAR_DATA_CHAR_INDEX].length - 1;
-          }
-        }
-      }
-    }
-
-    // Calculate the final end col by trimming whitespace on the right of the
-    // line if needed.
-    if (trimRight) {
-      const rightWhitespaceIndex = lineString.search(/\s+$/);
-      if (rightWhitespaceIndex !== -1) {
-        endIndex = Math.min(endIndex, rightWhitespaceIndex);
-      }
-      // Return the empty string if only trimmed whitespace is selected
-      if (endIndex <= startIndex) {
-        return '';
-      }
-    }
-
-    return lineString.substring(startIndex, endIndex);
+    return line.translateToString(trimRight, startCol, endCol);
   }
 
   public getWrappedRangeForLine(y: number): { first: number, last: number } {
@@ -488,8 +439,7 @@ export class BufferStringIterator implements IBufferStringIterator {
     range.last = Math.min(range.last, this._buffer.lines.length);
     let result = '';
     for (let i = range.first; i <= range.last; ++i) {
-      // TODO: always apply trimRight after fixing #1685
-      result += this._buffer.translateBufferLineToString(i, (this._trimRight) ? i === range.last : false);
+      result += this._buffer.translateBufferLineToString(i, this._trimRight);
     }
     this._current = range.last + 1;
     return {range: range, content: result};

src/BufferLine.test.ts

@@ -5,7 +5,7 @@
 import * as chai from 'chai';
 import { BufferLine } from './BufferLine';
 import { CharData, IBufferLine } from './Types';
-import { NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE } from './Buffer';
+import { NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE, DEFAULT_ATTR } from './Buffer';
 
 
 class TestBufferLine extends BufferLine {
@@ -200,4 +200,130 @@ describe('BufferLine', function(): void {
       chai.expect(line.toArray()).eql(Array(7).fill([1, 'a', 0, 'a'.charCodeAt(0)]));
     });
   });
+  describe('getTrimLength', function(): void {
+    it('empty line', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      chai.expect(line.getTrimmedLength()).equal(0);
+    });
+    it('ASCII', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      chai.expect(line.getTrimmedLength()).equal(3);
+    });
+    it('surrogate', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, '𝄞', 1, '𝄞'.charCodeAt(0)]);
+      chai.expect(line.getTrimmedLength()).equal(3);
+    });
+    it('combining', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, 'e\u0301', 1, '\u0301'.charCodeAt(0)]);
+      chai.expect(line.getTrimmedLength()).equal(3);
+    });
+    it('fullwidth', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, '１', 2, '１'.charCodeAt(0)]);
+      line.set(3, [0, '', 0, undefined]);
+      chai.expect(line.getTrimmedLength()).equal(4); // also counts null cell after fullwidth
+    });
+  });
+  describe('translateToString with and w\'o trimming', function(): void {
+    it('empty line', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      chai.expect(line.translateToString(false)).equal('          ');
+      chai.expect(line.translateToString(true)).equal('');
+    });
+    it('ASCII', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(4, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(5, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      chai.expect(line.translateToString(false)).equal('a a aa    ');
+      chai.expect(line.translateToString(true)).equal('a a aa');
+      chai.expect(line.translateToString(false, 0, 5)).equal('a a a');
+      chai.expect(line.translateToString(false, 0, 4)).equal('a a ');
+      chai.expect(line.translateToString(false, 0, 3)).equal('a a');
+      chai.expect(line.translateToString(true, 0, 5)).equal('a a a');
+      chai.expect(line.translateToString(true, 0, 4)).equal('a a ');
+      chai.expect(line.translateToString(true, 0, 3)).equal('a a');
+
+    });
+    it('surrogate', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, '𝄞', 1, '𝄞'.charCodeAt(0)]);
+      line.set(4, [1, '𝄞', 1, '𝄞'.charCodeAt(0)]);
+      line.set(5, [1, '𝄞', 1, '𝄞'.charCodeAt(0)]);
+      chai.expect(line.translateToString(false)).equal('a 𝄞 𝄞𝄞    ');
+      chai.expect(line.translateToString(true)).equal('a 𝄞 𝄞𝄞');
+      chai.expect(line.translateToString(false, 0, 5)).equal('a 𝄞 𝄞');
+      chai.expect(line.translateToString(false, 0, 4)).equal('a 𝄞 ');
+      chai.expect(line.translateToString(false, 0, 3)).equal('a 𝄞');
+      chai.expect(line.translateToString(true, 0, 5)).equal('a 𝄞 𝄞');
+      chai.expect(line.translateToString(true, 0, 4)).equal('a 𝄞 ');
+      chai.expect(line.translateToString(true, 0, 3)).equal('a 𝄞');
+    });
+    it('combining', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, 'e\u0301', 1, '\u0301'.charCodeAt(0)]);
+      line.set(4, [1, 'e\u0301', 1, '\u0301'.charCodeAt(0)]);
+      line.set(5, [1, 'e\u0301', 1, '\u0301'.charCodeAt(0)]);
+      chai.expect(line.translateToString(false)).equal('a e\u0301 e\u0301e\u0301    ');
+      chai.expect(line.translateToString(true)).equal('a e\u0301 e\u0301e\u0301');
+      chai.expect(line.translateToString(false, 0, 5)).equal('a e\u0301 e\u0301');
+      chai.expect(line.translateToString(false, 0, 4)).equal('a e\u0301 ');
+      chai.expect(line.translateToString(false, 0, 3)).equal('a e\u0301');
+      chai.expect(line.translateToString(true, 0, 5)).equal('a e\u0301 e\u0301');
+      chai.expect(line.translateToString(true, 0, 4)).equal('a e\u0301 ');
+      chai.expect(line.translateToString(true, 0, 3)).equal('a e\u0301');
+    });
+    it('fullwidth', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, '１', 2, '１'.charCodeAt(0)]);
+      line.set(3, [0, '', 0, undefined]);
+      line.set(5, [1, '１', 2, '１'.charCodeAt(0)]);
+      line.set(6, [0, '', 0, undefined]);
+      line.set(7, [1, '１', 2, '１'.charCodeAt(0)]);
+      line.set(8, [0, '', 0, undefined]);
+      chai.expect(line.translateToString(false)).equal('a １ １１ ');
+      chai.expect(line.translateToString(true)).equal('a １ １１');
+      chai.expect(line.translateToString(false, 0, 7)).equal('a １ １');
+      chai.expect(line.translateToString(false, 0, 6)).equal('a １ １');
+      chai.expect(line.translateToString(false, 0, 5)).equal('a １ ');
+      chai.expect(line.translateToString(false, 0, 4)).equal('a １');
+      chai.expect(line.translateToString(false, 0, 3)).equal('a １');
+      chai.expect(line.translateToString(false, 0, 2)).equal('a ');
+      chai.expect(line.translateToString(true, 0, 7)).equal('a １ １');
+      chai.expect(line.translateToString(true, 0, 6)).equal('a １ １');
+      chai.expect(line.translateToString(true, 0, 5)).equal('a １ ');
+      chai.expect(line.translateToString(true, 0, 4)).equal('a １');
+      chai.expect(line.translateToString(true, 0, 3)).equal('a １');
+      chai.expect(line.translateToString(true, 0, 2)).equal('a ');
+    });
+    it('space at end', function(): void {
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE], false);
+      line.set(0, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(2, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(4, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(5, [1, 'a', 1, 'a'.charCodeAt(0)]);
+      line.set(6, [1, ' ', 1, ' '.charCodeAt(0)]);
+      chai.expect(line.translateToString(false)).equal('a a aa    ');
+      chai.expect(line.translateToString(true)).equal('a a aa ');
+    });
+    it('should always return some sane value', function(): void {
+      // sanity check - broken line with invalid out of bound null width cells
+      // this can atm happen with deleting/inserting chars in inputhandler by "breaking"
+      // fullwidth pairs --> needs to be fixed after settling BufferLine impl
+      const line = new TestBufferLine(10, [DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE], false);
+      chai.expect(line.translateToString(false)).equal('          ');
+      chai.expect(line.translateToString(true)).equal('');
+    });
+  });
 });

src/BufferLine.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 import { CharData, IBufferLine } from './Types';
-import { NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR } from './Buffer';
+import { NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR, CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX, WHITESPACE_CELL_CHAR } from './Buffer';
 
 /**
  * Class representing a terminal line.
@@ -107,6 +107,29 @@ export class BufferLineJSArray implements IBufferLine {
     newLine.copyFrom(this);
     return newLine;
   }
+
+  public getTrimmedLength(): number {
+    for (let i = this.length - 1; i >= 0; --i) {
+      const ch = this.get(i);
+      if (ch[CHAR_DATA_CHAR_INDEX] !== '') {
+        return i + ch[CHAR_DATA_WIDTH_INDEX];
+      }
+    }
+    return 0;
+  }
+
+  public translateToString(trimRight: boolean = false, startCol: number = 0, endCol: number = null): string {
+    let length = endCol || this.length;
+    if (trimRight) {
+      length = Math.min(length, this.getTrimmedLength());
+    }
+    let result = '';
+    while (startCol < length) {
+      result += this.get(startCol)[CHAR_DATA_CHAR_INDEX] || WHITESPACE_CELL_CHAR;
+      startCol += this.get(startCol)[CHAR_DATA_WIDTH_INDEX] || 1;
+    }
+    return result;
+  }
 }
 
 /** typed array slots taken by one cell */
@@ -119,6 +142,9 @@ const enum Cell {
   WIDTH = 2
 }
 
+/** single vs. combined char distinction */
+const IS_COMBINED_BIT_MASK = 0x80000000;
+
 /**
  * Typed array based bufferline implementation.
  */
@@ -144,11 +170,11 @@ export class BufferLine implements IBufferLine {
     const stringData = this._data[index * CELL_SIZE + Cell.STRING];
     return [
       this._data[index * CELL_SIZE + Cell.FLAGS],
-      (stringData & 0x80000000)
+      (stringData & IS_COMBINED_BIT_MASK)
         ? this._combined[index]
         : (stringData) ? String.fromCharCode(stringData) : '',
       this._data[index * CELL_SIZE + Cell.WIDTH],
-      (stringData & 0x80000000)
+      (stringData & IS_COMBINED_BIT_MASK)
         ? this._combined[index].charCodeAt(this._combined[index].length - 1)
         : stringData
     ];
@@ -158,7 +184,7 @@ export class BufferLine implements IBufferLine {
     this._data[index * CELL_SIZE + Cell.FLAGS] = value[0];
     if (value[1].length > 1) {
       this._combined[index] = value[1];
-      this._data[index * CELL_SIZE + Cell.STRING] = index | 0x80000000;
+      this._data[index * CELL_SIZE + Cell.STRING] = index | IS_COMBINED_BIT_MASK;
     } else {
       this._data[index * CELL_SIZE + Cell.STRING] = value[1].charCodeAt(0);
     }
@@ -269,4 +295,27 @@ export class BufferLine implements IBufferLine {
     newLine.isWrapped = this.isWrapped;
     return newLine;
   }
+
+  public getTrimmedLength(): number {
+    for (let i = this.length - 1; i >= 0; --i) {
+      if (this._data[i * CELL_SIZE + Cell.STRING] !== 0) {  // 0 ==> ''.charCodeAt(0) ==> NaN ==> 0
+        return i + this._data[i * CELL_SIZE + Cell.WIDTH];
+      }
+    }
+    return 0;
+  }
+
+  public translateToString(trimRight: boolean = false, startCol: number = 0, endCol: number = null): string {
+    let length = endCol || this.length;
+    if (trimRight) {
+      length = Math.min(length, this.getTrimmedLength());
+    }
+    let result = '';
+    while (startCol < length) {
+      const stringData = this._data[startCol * CELL_SIZE + Cell.STRING];
+      result += (stringData & IS_COMBINED_BIT_MASK) ? this._combined[startCol] : (stringData) ? String.fromCharCode(stringData) : WHITESPACE_CELL_CHAR;
+      startCol += this._data[startCol * CELL_SIZE + Cell.WIDTH] || 1;
+    }
+    return result;
+  }
 }