Merge pull request #2644 from jerch/fullwidth_erase

Correctly reset cells of previous fullwidth chars during buffer writes

src/InputHandler.test.ts

@@ -1270,4 +1270,63 @@ describe('InputHandler', () => {
       [131072, 131072], [131072, 131072], [131072, 300000 - 131072 - 131072]
     ]);
   });
+  describe('should correctly reset cells taken by wide chars', () => {
+    let term: TestTerminal;
+    beforeEach(() => {
+      term = new TestTerminal({cols: 10, rows: 5, scrollback: 1});
+      term.writeSync('￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥￥');
+    });
+    it('print', () => {
+      term.writeSync('\x1b[H#');
+      assert.deepEqual(getLines(term), ['# ￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[1;6H######');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '# ￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '##￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '### ￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[3;9H#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '### ￥￥￥', '￥￥￥￥#', '￥￥￥￥￥', '']);
+      term.writeSync('#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '### ￥￥￥', '￥￥￥￥##', '￥￥￥￥￥', '']);
+      term.writeSync('#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '### ￥￥￥', '￥￥￥￥##', '# ￥￥￥￥', '']);
+      term.writeSync('\x1b[4;10H#');
+      assert.deepEqual(getLines(term), ['# ￥ #####', '### ￥￥￥', '￥￥￥￥##', '# ￥￥￥ #', '']);
+    });
+    it('EL', () => {
+      term.writeSync('\x1b[1;6H\x1b[K#');
+      assert.deepEqual(getLines(term), ['￥￥ #', '￥￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[2;5H\x1b[1K');
+      assert.deepEqual(getLines(term), ['￥￥ #', '      ￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[3;6H\x1b[1K');
+      assert.deepEqual(getLines(term), ['￥￥ #', '      ￥￥', '      ￥￥', '￥￥￥￥￥', '']);
+    });
+    it('ICH', () => {
+      term.writeSync('\x1b[1;6H\x1b[@');
+      assert.deepEqual(getLines(term), ['￥￥   ￥', '￥￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[2;4H\x1b[2@');
+      assert.deepEqual(getLines(term), ['￥￥   ￥', '￥    ￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[3;4H\x1b[3@');
+      assert.deepEqual(getLines(term), ['￥￥   ￥', '￥    ￥￥', '￥     ￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[4;4H\x1b[4@');
+      assert.deepEqual(getLines(term), ['￥￥   ￥', '￥    ￥￥', '￥     ￥', '￥      ￥', '']);
+    });
+    it('DCH', () => {
+      term.writeSync('\x1b[1;6H\x1b[P');
+      assert.deepEqual(getLines(term), ['￥￥ ￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[2;6H\x1b[2P');
+      assert.deepEqual(getLines(term), ['￥￥ ￥￥', '￥￥  ￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[3;6H\x1b[3P');
+      assert.deepEqual(getLines(term), ['￥￥ ￥￥', '￥￥  ￥', '￥￥ ￥', '￥￥￥￥￥', '']);
+    });
+    it('ECH', () => {
+      term.writeSync('\x1b[1;6H\x1b[X');
+      assert.deepEqual(getLines(term), ['￥￥  ￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[2;6H\x1b[2X');
+      assert.deepEqual(getLines(term), ['￥￥  ￥￥', '￥￥    ￥', '￥￥￥￥￥', '￥￥￥￥￥', '']);
+      term.writeSync('\x1b[3;6H\x1b[3X');
+      assert.deepEqual(getLines(term), ['￥￥  ￥￥', '￥￥    ￥', '￥￥    ￥', '￥￥￥￥￥', '']);
+    });
+  });
 });

src/InputHandler.ts

@@ -398,6 +398,12 @@ export class InputHandler extends Disposable implements IInputHandler {
     let bufferRow = buffer.lines.get(buffer.y + buffer.ybase);
 
     this._dirtyRowService.markDirty(buffer.y);
+
+    // handle wide chars: reset start_cell-1 if we would overwrite the second cell of a wide char
+    if (buffer.x && end - start > 0 && bufferRow.getWidth(buffer.x - 1) === 2) {
+      bufferRow.setCellFromCodePoint(buffer.x - 1, 0, 1, curAttr.fg, curAttr.bg);
+    }
+
     for (let pos = start; pos < end; ++pos) {
       code = data[pos];
 
@@ -473,7 +479,7 @@ export class InputHandler extends Disposable implements IInputHandler {
       // insert mode: move characters to right
       if (insertMode) {
         // right shift cells according to the width
-        bufferRow.insertCells(buffer.x, chWidth, buffer.getNullCell(curAttr));
+        bufferRow.insertCells(buffer.x, chWidth, buffer.getNullCell(curAttr), curAttr);
         // test last cell - since the last cell has only room for
         // a halfwidth char any fullwidth shifted there is lost
         // and will be set to empty cell
@@ -499,7 +505,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     // This needs to check whether:
     //  - fullwidth + surrogates: reset
     //  - combining: only base char gets carried on (bug in xterm?)
-    if (end) {
+    if (end - start > 0) {
       bufferRow.loadCell(buffer.x - 1, this._workCell);
       if (this._workCell.getWidth() === 2 || this._workCell.getCode() > 0xFFFF) {
         this._parser.precedingCodepoint = 0;
@@ -509,6 +515,12 @@ export class InputHandler extends Disposable implements IInputHandler {
         this._parser.precedingCodepoint = this._workCell.content;
       }
     }
+
+    // handle wide chars: reset cell to the right if it is second cell of a wide char
+    if (buffer.x < cols && end - start > 0 && bufferRow.getWidth(buffer.x) === 0 && !bufferRow.hasContent(buffer.x)) {
+      bufferRow.setCellFromCodePoint(buffer.x, 0, 1, curAttr.fg, curAttr.bg);
+    }
+
     this._dirtyRowService.markDirty(buffer.y);
   }
 
@@ -855,7 +867,8 @@ export class InputHandler extends Disposable implements IInputHandler {
     line.replaceCells(
       start,
       end,
-      this._bufferService.buffer.getNullCell(this._eraseAttrData())
+      this._bufferService.buffer.getNullCell(this._eraseAttrData()),
+      this._eraseAttrData()
     );
     if (clearWrap) {
       line.isWrapped = false;
@@ -1033,7 +1046,8 @@ export class InputHandler extends Disposable implements IInputHandler {
       line.insertCells(
         this._bufferService.buffer.x,
         params.params[0] || 1,
-        this._bufferService.buffer.getNullCell(this._eraseAttrData())
+        this._bufferService.buffer.getNullCell(this._eraseAttrData()),
+        this._eraseAttrData()
       );
       this._dirtyRowService.markDirty(this._bufferService.buffer.y);
     }
@@ -1050,7 +1064,8 @@ export class InputHandler extends Disposable implements IInputHandler {
       line.deleteCells(
         this._bufferService.buffer.x,
         params.params[0] || 1,
-        this._bufferService.buffer.getNullCell(this._eraseAttrData())
+        this._bufferService.buffer.getNullCell(this._eraseAttrData()),
+        this._eraseAttrData()
       );
       this._dirtyRowService.markDirty(this._bufferService.buffer.y);
     }
@@ -1110,7 +1125,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     const param = params.params[0] || 1;
     for (let y = buffer.scrollTop; y <= buffer.scrollBottom; ++y) {
       const line = buffer.lines.get(buffer.ybase + y);
-      line.deleteCells(0, param, buffer.getNullCell(this._eraseAttrData()));
+      line.deleteCells(0, param, buffer.getNullCell(this._eraseAttrData()), this._eraseAttrData());
       line.isWrapped = false;
     }
     this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom);
@@ -1138,7 +1153,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     const param = params.params[0] || 1;
     for (let y = buffer.scrollTop; y <= buffer.scrollBottom; ++y) {
       const line = buffer.lines.get(buffer.ybase + y);
-      line.insertCells(0, param, buffer.getNullCell(this._eraseAttrData()));
+      line.insertCells(0, param, buffer.getNullCell(this._eraseAttrData()), this._eraseAttrData());
       line.isWrapped = false;
     }
     this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom);
@@ -1156,7 +1171,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     const param = params.params[0] || 1;
     for (let y = buffer.scrollTop; y <= buffer.scrollBottom; ++y) {
       const line = this._bufferService.buffer.lines.get(buffer.ybase + y);
-      line.insertCells(buffer.x, param, buffer.getNullCell(this._eraseAttrData()));
+      line.insertCells(buffer.x, param, buffer.getNullCell(this._eraseAttrData()), this._eraseAttrData());
       line.isWrapped = false;
     }
     this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom);
@@ -1174,7 +1189,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     const param = params.params[0] || 1;
     for (let y = buffer.scrollTop; y <= buffer.scrollBottom; ++y) {
       const line = buffer.lines.get(buffer.ybase + y);
-      line.deleteCells(buffer.x, param, buffer.getNullCell(this._eraseAttrData()));
+      line.deleteCells(buffer.x, param, buffer.getNullCell(this._eraseAttrData()), this._eraseAttrData());
       line.isWrapped = false;
     }
     this._dirtyRowService.markRangeDirty(buffer.scrollTop, buffer.scrollBottom);
@@ -1191,7 +1206,8 @@ export class InputHandler extends Disposable implements IInputHandler {
       line.replaceCells(
         this._bufferService.buffer.x,
         this._bufferService.buffer.x + (params.params[0] || 1),
-        this._bufferService.buffer.getNullCell(this._eraseAttrData())
+        this._bufferService.buffer.getNullCell(this._eraseAttrData()),
+        this._eraseAttrData()
       );
       this._dirtyRowService.markDirty(this._bufferService.buffer.y);
     }

src/common/Types.d.ts

@@ -123,9 +123,9 @@ export interface IBufferLine {
   setCell(index: number, cell: ICellData): void;
   setCellFromCodePoint(index: number, codePoint: number, width: number, fg: number, bg: number): void;
   addCodepointToCell(index: number, codePoint: number): void;
-  insertCells(pos: number, n: number, ch: ICellData): void;
-  deleteCells(pos: number, n: number, fill: ICellData): void;
-  replaceCells(start: number, end: number, fill: ICellData): void;
+  insertCells(pos: number, n: number, ch: ICellData, eraseAttr?: IAttributeData): void;
+  deleteCells(pos: number, n: number, fill: ICellData, eraseAttr?: IAttributeData): void;
+  replaceCells(start: number, end: number, fill: ICellData, eraseAttr?: IAttributeData): void;
   resize(cols: number, fill: ICellData): void;
   fill(fillCellData: ICellData): void;
   copyFrom(line: IBufferLine): void;

src/common/buffer/BufferLine.test.ts

@@ -366,4 +366,94 @@ describe('BufferLine', function(): void {
       chai.assert.equal(cell.isCombined(), Content.IS_COMBINED_MASK);
     });
   });
+  describe('correct fullwidth handling', () => {
+    function populate(line: BufferLine): void {
+      const cell = CellData.fromCharData([1, '￥', 2, '￥'.charCodeAt(0)]);
+      for (let i = 0; i < line.length; i += 2) {
+        line.setCell(i, cell);
+      }
+    }
+    it('insert - wide char at pos', () => {
+      const line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.insertCells(9, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), '￥￥￥￥ a');
+      line.insertCells(8, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), '￥￥￥￥a ');
+      line.insertCells(1, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' a ￥￥￥a');
+    });
+    it('insert - wide char at end', () => {
+      const line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.insertCells(0, 3, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaa￥￥￥ ');
+      line.insertCells(4, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaa a ￥￥');
+      line.insertCells(4, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaa aa ￥ ');
+    });
+    it('delete', () => {
+      const line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.deleteCells(0, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' ￥￥￥￥a');
+      line.deleteCells(5, 2, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' ￥￥￥aaa');
+      line.deleteCells(0, 2, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' ￥￥aaaaa');
+    });
+    it('replace - start at 0', () => {
+      let line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 1, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'a ￥￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 2, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aa￥￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 3, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaa ￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 8, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaaaaaaa￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 9, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaaaaaaaa ');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(0, 10, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), 'aaaaaaaaaa');
+    });
+    it('replace - start at 1', () => {
+      let line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 2, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' a￥￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 3, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' aa ￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 4, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' aaa￥￥￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 8, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' aaaaaaa￥');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 9, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' aaaaaaaa ');
+      line = new TestBufferLine(10, CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, 0, NULL_CELL_CODE]), false);
+      populate(line);
+      line.replaceCells(1, 10, CellData.fromCharData([1, 'a', 1, 'a'.charCodeAt(0)]));
+      chai.assert.equal(line.translateToString(), ' aaaaaaaaa');
+    });
+  });
 });

src/common/buffer/BufferLine.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { CharData, IBufferLine, ICellData } from 'common/Types';
+import { CharData, IBufferLine, ICellData, IAttributeData } from 'common/Types';
 import { stringFromCodePoint } from 'common/input/TextDecoder';
 import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX, CHAR_DATA_ATTR_INDEX, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE, WHITESPACE_CELL_CHAR, Content } from 'common/buffer/Constants';
 import { CellData } from 'common/buffer/CellData';
@@ -228,8 +228,14 @@ export class BufferLine implements IBufferLine {
     }
   }
 
-  public insertCells(pos: number, n: number, fillCellData: ICellData): void {
+  public insertCells(pos: number, n: number, fillCellData: ICellData, eraseAttr?: IAttributeData): void {
     pos %= this.length;
+
+    // handle fullwidth at pos: reset cell one to the left if pos is second cell of a wide char
+    if (pos && this.getWidth(pos - 1) === 2) {
+      this.setCellFromCodePoint(pos - 1, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
+
     if (n < this.length - pos) {
       const cell = new CellData();
       for (let i = this.length - pos - n - 1; i >= 0; --i) {
@@ -243,9 +249,14 @@ export class BufferLine implements IBufferLine {
         this.setCell(i, fillCellData);
       }
     }
+
+    // handle fullwidth at line end: reset last cell if it is first cell of a wide char
+    if (this.getWidth(this.length - 1) === 2) {
+      this.setCellFromCodePoint(this.length - 1, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
   }
 
-  public deleteCells(pos: number, n: number, fillCellData: ICellData): void {
+  public deleteCells(pos: number, n: number, fillCellData: ICellData, eraseAttr?: IAttributeData): void {
     pos %= this.length;
     if (n < this.length - pos) {
       const cell = new CellData();
@@ -260,9 +271,28 @@ export class BufferLine implements IBufferLine {
         this.setCell(i, fillCellData);
       }
     }
+
+    // handle fullwidth at pos:
+    // - reset pos-1 if wide char
+    // - reset pos if width==0 (previous second cell of a wide char)
+    if (pos && this.getWidth(pos - 1) === 2) {
+      this.setCellFromCodePoint(pos - 1, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
+    if (this.getWidth(pos) === 0 && !this.hasContent(pos)) {
+      this.setCellFromCodePoint(pos, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
   }
 
-  public replaceCells(start: number, end: number, fillCellData: ICellData): void {
+  public replaceCells(start: number, end: number, fillCellData: ICellData, eraseAttr?: IAttributeData): void {
+    // handle fullwidth at start: reset cell one to the left if start is second cell of a wide char
+    if (start && this.getWidth(start - 1) === 2) {
+      this.setCellFromCodePoint(start - 1, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
+    // handle fullwidth at last cell + 1: reset to empty cell if it is second part of a wide char
+    if (end < this.length && this.getWidth(end - 1) === 2) {
+      this.setCellFromCodePoint(end, 0, 1, eraseAttr?.fg || 0, eraseAttr?.bg || 0);
+    }
+
     while (start < end  && start < this.length) {
       this.setCell(start++, fillCellData);
     }