From fa2a32e43164c8a9cf69df18201220ce463ad89e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 7 Apr 2019 13:16:19 -0400 Subject: [PATCH 1/2] Fix reflow smaller case related to ended \t chars Part of #1932 --- src/Buffer.test.ts | 49 +++++++++++++++++++++++++++++++++++++++++++++ src/Buffer.ts | 8 ++++---- src/BufferReflow.ts | 29 +++++++++++++++++++++------ 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 53de19b7cc..29313bfd42 100644 --- a/src/Buffer.test.ts +++ b/src/Buffer.test.ts @@ -518,6 +518,29 @@ describe('Buffer', () => { assert.equal(secondMarker.line, 1, 'second marker should be restored'); assert.equal(thirdMarker.line, 2, 'third marker should be restored'); }); + it('should correctly reflow wrapped lines that end in null space (via tab char)', () => { + buffer.fillViewportRows(); + buffer.resize(4, 10); + buffer.y = 2; + buffer.lines.get(0).set(0, [null, 'a', 1, 'a'.charCodeAt(0)]); + buffer.lines.get(0).set(1, [null, 'b', 1, 'b'.charCodeAt(0)]); + buffer.lines.get(1).set(0, [null, 'c', 1, 'c'.charCodeAt(0)]); + buffer.lines.get(1).set(1, [null, 'd', 1, 'd'.charCodeAt(0)]); + buffer.lines.get(1).isWrapped = true; + // Buffer: + // "ab " (wrapped) + // "cd" + buffer.resize(5, 10); + assert.equal(buffer.ybase, 0); + assert.equal(buffer.lines.length, 10); + assert.equal(buffer.lines.get(0).translateToString(true), 'ab c'); + assert.equal(buffer.lines.get(1).translateToString(false), 'd '); + buffer.resize(6, 10); + assert.equal(buffer.ybase, 0); + assert.equal(buffer.lines.length, 10); + assert.equal(buffer.lines.get(0).translateToString(true), 'ab cd'); + assert.equal(buffer.lines.get(1).translateToString(false), ' '); + }); it('should wrap wide characters correctly when reflowing larger', () => { buffer.fillViewportRows(); buffer.resize(12, 10); @@ -553,6 +576,32 @@ describe('Buffer', () => { assert.equal(buffer.lines.get(1).translateToString(true), '语汉语汉语'); assert.equal(buffer.lines.get(1).translateToString(false), '语汉语汉语 '); }); + it('should correctly reflow wrapped lines that end in null space (via tab char)', () => { + buffer.fillViewportRows(); + buffer.resize(4, 10); + buffer.y = 2; + buffer.lines.get(0).set(0, [null, 'a', 1, 'a'.charCodeAt(0)]); + buffer.lines.get(0).set(1, [null, 'b', 1, 'b'.charCodeAt(0)]); + buffer.lines.get(1).set(0, [null, 'c', 1, 'c'.charCodeAt(0)]); + buffer.lines.get(1).set(1, [null, 'd', 1, 'd'.charCodeAt(0)]); + buffer.lines.get(1).isWrapped = true; + // Buffer: + // "ab " (wrapped) + // "cd" + buffer.resize(3, 10); + assert.equal(buffer.y, 2); + assert.equal(buffer.ybase, 0); + assert.equal(buffer.lines.length, 10); + assert.equal(buffer.lines.get(0).translateToString(false), 'ab '); + assert.equal(buffer.lines.get(1).translateToString(false), ' cd'); + buffer.resize(2, 10); + assert.equal(buffer.y, 3); + assert.equal(buffer.ybase, 0); + assert.equal(buffer.lines.length, 10); + assert.equal(buffer.lines.get(0).translateToString(false), 'ab'); + assert.equal(buffer.lines.get(1).translateToString(false), ' '); + assert.equal(buffer.lines.get(2).translateToString(false), 'cd'); + }); it('should wrap wide characters correctly when reflowing smaller', () => { buffer.fillViewportRows(); buffer.resize(12, 10); diff --git a/src/Buffer.ts b/src/Buffer.ts index 5eea5d9a41..8c61c83cbc 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -5,7 +5,7 @@ import { IMarker } from 'xterm'; import { BufferLine } from './BufferLine'; -import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths } from './BufferReflow'; +import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths, getWrappedLineTrimmedLength } from './BufferReflow'; import { CircularList, IDeleteEvent, IInsertEvent } from './common/CircularList'; import { EventEmitter } from './common/EventEmitter'; import { DEFAULT_COLOR } from './renderer/atlas/Types'; @@ -244,7 +244,7 @@ export class Buffer implements IBuffer { } private _reflowLarger(newCols: number, newRows: number): void { - const toRemove: number[] = reflowLargerGetLinesToRemove(this.lines, newCols, this.ybase + this.y); + const toRemove: number[] = reflowLargerGetLinesToRemove(this.lines, this._cols, newCols, this.ybase + this.y); if (toRemove.length > 0) { const newLayoutResult = reflowLargerCreateNewLayout(this.lines, toRemove); reflowLargerApplyNewLayout(this.lines, newLayoutResult.layout); @@ -348,8 +348,8 @@ export class Buffer implements IBuffer { srcCol -= cellsToCopy; if (srcCol === 0) { srcLineIndex--; - // TODO: srcCol shoudl take trimmed length into account - srcCol = wrappedLines[Math.max(srcLineIndex, 0)].getTrimmedLength(); // this._cols; + const wrappedLinesIndex = Math.max(srcLineIndex, 0); + srcCol = getWrappedLineTrimmedLength(wrappedLines, wrappedLinesIndex, this._cols); } } diff --git a/src/BufferReflow.ts b/src/BufferReflow.ts index 24ab69e6bc..d37917b59d 100644 --- a/src/BufferReflow.ts +++ b/src/BufferReflow.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { FILL_CHAR_DATA } from './Buffer'; +import { FILL_CHAR_DATA, CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX } from './Buffer'; import { BufferLine } from './BufferLine'; import { CircularList, IDeleteEvent } from './common/CircularList'; import { IBufferLine } from './Types'; @@ -19,7 +19,7 @@ export interface INewLayoutResult { * @param lines The buffer lines. * @param newCols The columns after resize. */ -export function reflowLargerGetLinesToRemove(lines: CircularList, newCols: number, bufferAbsoluteY: number): number[] { +export function reflowLargerGetLinesToRemove(lines: CircularList, oldCols: number, newCols: number, bufferAbsoluteY: number): number[] { // Gather all BufferLines that need to be removed from the Buffer here so that they can be // batched up and only committed once const toRemove: number[] = []; @@ -48,11 +48,11 @@ export function reflowLargerGetLinesToRemove(lines: CircularList, n // Copy buffer data to new locations let destLineIndex = 0; - let destCol = wrappedLines[destLineIndex].getTrimmedLength(); + let destCol = wrappedLines.length === 1 ? wrappedLines[destLineIndex].getTrimmedLength() : oldCols; let srcLineIndex = 1; let srcCol = 0; while (srcLineIndex < wrappedLines.length) { - const srcTrimmedTineLength = wrappedLines[srcLineIndex].getTrimmedLength(); + const srcTrimmedTineLength = srcLineIndex === wrappedLines.length - 1 ? wrappedLines[srcLineIndex].getTrimmedLength() : oldCols; const srcRemainingCells = srcTrimmedTineLength - srcCol; const destRemainingCells = newCols - destCol; const cellsToCopy = Math.min(srcRemainingCells, destRemainingCells); @@ -173,7 +173,7 @@ export function reflowLargerApplyNewLayout(lines: CircularList, new */ export function reflowSmallerGetNewLineLengths(wrappedLines: BufferLine[], oldCols: number, newCols: number): number[] { const newLineLengths: number[] = []; - const cellsNeeded = wrappedLines.map(l => l.getTrimmedLength()).reduce((p, c) => p + c); + const cellsNeeded = wrappedLines.map((l, i) => getWrappedLineTrimmedLength(wrappedLines, i, oldCols)).reduce((p, c) => p + c); // Use srcCol and srcLine to find the new wrapping point, use that to get the cellsAvailable and // linesNeeded @@ -187,7 +187,7 @@ export function reflowSmallerGetNewLineLengths(wrappedLines: BufferLine[], oldCo break; } srcCol += newCols; - const oldTrimmedLength = wrappedLines[srcLine].getTrimmedLength(); + const oldTrimmedLength = getWrappedLineTrimmedLength(wrappedLines, srcLine, oldCols); if (srcCol > oldTrimmedLength) { srcCol -= oldTrimmedLength; srcLine++; @@ -203,3 +203,20 @@ export function reflowSmallerGetNewLineLengths(wrappedLines: BufferLine[], oldCo return newLineLengths; } + +export function getWrappedLineTrimmedLength(lines: BufferLine[], i: number, cols: number): number { + // If this is the last row in the wrapped line, get the actual trimmed length + if (i === lines.length - 1) { + return lines[i].getTrimmedLength(); + } + // Detect whether the following line starts with a wide character and the end of the current line + // is null, if so then we can be pretty sure the null character should be excluded from the line + // length] + const cell = lines[i].get(cols - 1); + const endsInNull = cell[CHAR_DATA_CHAR_INDEX] === '' && cell[CHAR_DATA_WIDTH_INDEX] === 1; + const followingLineStartsWithWide = lines[i + 1].getWidth(0) === 2; + if (endsInNull && followingLineStartsWithWide) { + return cols - 1; + } + return cols; +} From 925cdaef6751dec635935e03b6309d6049e7728e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 7 Apr 2019 13:18:05 -0400 Subject: [PATCH 2/2] Fix reflow larger case Fixes #1932 --- src/BufferReflow.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/BufferReflow.ts b/src/BufferReflow.ts index d37917b59d..bdc9768f75 100644 --- a/src/BufferReflow.ts +++ b/src/BufferReflow.ts @@ -48,11 +48,11 @@ export function reflowLargerGetLinesToRemove(lines: CircularList, o // Copy buffer data to new locations let destLineIndex = 0; - let destCol = wrappedLines.length === 1 ? wrappedLines[destLineIndex].getTrimmedLength() : oldCols; + let destCol = getWrappedLineTrimmedLength(wrappedLines, destLineIndex, oldCols); let srcLineIndex = 1; let srcCol = 0; while (srcLineIndex < wrappedLines.length) { - const srcTrimmedTineLength = srcLineIndex === wrappedLines.length - 1 ? wrappedLines[srcLineIndex].getTrimmedLength() : oldCols; + const srcTrimmedTineLength = getWrappedLineTrimmedLength(wrappedLines, srcLineIndex, oldCols); const srcRemainingCells = srcTrimmedTineLength - srcCol; const destRemainingCells = newCols - destCol; const cellsToCopy = Math.min(srcRemainingCells, destRemainingCells);