From ab44650d87a4d701171d891feeee60796dc56f48 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/BufferLine.ts | 2 +- src/BufferReflow.ts | 26 +++++++++++++++++++----- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/Buffer.test.ts b/src/Buffer.test.ts index 59475adb..e2ef35fe 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 4d844ea1..770b027c 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -8,7 +8,7 @@ import { ITerminal, IBuffer, IBufferLine, BufferIndex, IBufferStringIterator, IB import { EventEmitter } from './common/EventEmitter'; import { IMarker } from 'xterm'; import { BufferLine, CellData } from './BufferLine'; -import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths } from './BufferReflow'; +import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths, getWrappedLineTrimmedLength } from './BufferReflow'; import { DEFAULT_COLOR } from './renderer/atlas/Types'; @@ -269,7 +269,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); @@ -375,8 +375,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/BufferLine.ts b/src/BufferLine.ts index c4aa55be..07207248 100644 --- a/src/BufferLine.ts +++ b/src/BufferLine.ts @@ -58,7 +58,7 @@ export const enum ContentMasks { * bit 1..22 mask to check whether a cell contains any string data * we need to check for codepoint and isCombined bits to see * whether a cell contains anything - * read: `isEmtpy = !(content & Content.hasContent)` + * read: `isEmpty = !(content & Content.hasContent)` */ HAS_CONTENT = 0x3FFFFF, diff --git a/src/BufferReflow.ts b/src/BufferReflow.ts index d27d7c48..d3adfc6d 100644 --- a/src/BufferReflow.ts +++ b/src/BufferReflow.ts @@ -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[] { const nullCell = CellData.fromCharData([DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE]); // Gather all BufferLines that need to be removed from the Buffer here so that they can be // batched up and only committed once @@ -49,11 +49,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); @@ -174,7 +174,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 @@ -188,7 +188,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++; @@ -204,3 +204,19 @@ 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 endsInNull = !(lines[i].hasContent(cols - 1)) && lines[i].getWidth(cols - 1) === 1; + const followingLineStartsWithWide = lines[i + 1].getWidth(0) === 2; + if (endsInNull && followingLineStartsWithWide) { + return cols - 1; + } + return cols; +} From 3ac313f8e12ef7ded12e60e60ceb41d1f6105f37 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 d3adfc6d..c090b0d7 100644 --- a/src/BufferReflow.ts +++ b/src/BufferReflow.ts @@ -49,11 +49,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);