Skip to content

Commit

Permalink
Merge pull request #2644 from jerch/fullwidth_erase
Browse files Browse the repository at this point in the history
Correctly reset cells of previous fullwidth chars during buffer writes
  • Loading branch information
jerch authored Dec 26, 2019
2 parents b32bfed + 9f8ea28 commit e52eafa
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 17 deletions.
59 changes: 59 additions & 0 deletions src/InputHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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), ['¥¥ ¥¥', '¥¥ ¥', '¥¥ ¥', '¥¥¥¥¥', '']);
});
});
});
36 changes: 26 additions & 10 deletions src/InputHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions src/common/Types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
90 changes: 90 additions & 0 deletions src/common/buffer/BufferLine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
38 changes: 34 additions & 4 deletions src/common/buffer/BufferLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
Expand All @@ -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);
}
Expand Down

0 comments on commit e52eafa

Please sign in to comment.