Skip to content

Commit

Permalink
Merge pull request #2010 from Tyriar/3.12.1_1932_reflow
Browse files Browse the repository at this point in the history
Fix #1932 reflow issue in 3.12.1
  • Loading branch information
Tyriar committed Apr 12, 2019
2 parents 4046d68 + 925cdae commit 3ad45b7
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 10 deletions.
49 changes: 49 additions & 0 deletions src/Buffer.test.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/Buffer.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down
29 changes: 23 additions & 6 deletions src/BufferReflow.ts
Expand Up @@ -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';
Expand All @@ -19,7 +19,7 @@ export interface INewLayoutResult {
* @param lines The buffer lines.
* @param newCols The columns after resize.
*/
export function reflowLargerGetLinesToRemove(lines: CircularList<IBufferLine>, newCols: number, bufferAbsoluteY: number): number[] {
export function reflowLargerGetLinesToRemove(lines: CircularList<IBufferLine>, 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[] = [];
Expand Down Expand Up @@ -48,11 +48,11 @@ export function reflowLargerGetLinesToRemove(lines: CircularList<IBufferLine>, n

// Copy buffer data to new locations
let destLineIndex = 0;
let destCol = wrappedLines[destLineIndex].getTrimmedLength();
let destCol = getWrappedLineTrimmedLength(wrappedLines, destLineIndex, oldCols);
let srcLineIndex = 1;
let srcCol = 0;
while (srcLineIndex < wrappedLines.length) {
const srcTrimmedTineLength = wrappedLines[srcLineIndex].getTrimmedLength();
const srcTrimmedTineLength = getWrappedLineTrimmedLength(wrappedLines, srcLineIndex, oldCols);
const srcRemainingCells = srcTrimmedTineLength - srcCol;
const destRemainingCells = newCols - destCol;
const cellsToCopy = Math.min(srcRemainingCells, destRemainingCells);
Expand Down Expand Up @@ -173,7 +173,7 @@ export function reflowLargerApplyNewLayout(lines: CircularList<IBufferLine>, 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
Expand All @@ -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++;
Expand All @@ -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;
}

0 comments on commit 3ad45b7

Please sign in to comment.