Merge pull request #2002 from Tyriar/1932_broken_reflow

Fix reflow case where wrapped line portions ended in \t

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);

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, AttributeData } from './BufferLine';
-import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths } from './BufferReflow';
+import { reflowLargerApplyNewLayout, reflowLargerCreateNewLayout, reflowLargerGetLinesToRemove, reflowSmallerGetNewLineLengths, getWrappedLineTrimmedLength } from './BufferReflow';
 import { DEFAULT_COLOR } from './renderer/atlas/Types';
 
 
@@ -282,7 +282,7 @@ export class Buffer implements IBuffer {
   }
 
   private _reflowLarger(newCols: number, newRows: number): void {
-    const toRemove: number[] = reflowLargerGetLinesToRemove(this.lines, newCols, this.ybase + this.y, this.getNullCell(DEFAULT_ATTR_DATA));
+    const toRemove: number[] = reflowLargerGetLinesToRemove(this.lines, this._cols, newCols, this.ybase + this.y, this.getNullCell(DEFAULT_ATTR_DATA));
     if (toRemove.length > 0) {
       const newLayoutResult = reflowLargerCreateNewLayout(this.lines, toRemove);
       reflowLargerApplyNewLayout(this.lines, newLayoutResult.layout);
@@ -388,8 +388,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);
         }
       }
 

src/BufferLine.ts

@@ -58,7 +58,7 @@ export const enum Content {
    * 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_MASK = 0x3FFFFF,
 

src/BufferReflow.ts

@@ -18,7 +18,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, nullCell: ICellData): number[] {
+export function reflowLargerGetLinesToRemove(lines: CircularList<IBufferLine>, oldCols: number, newCols: number, bufferAbsoluteY: number, nullCell: ICellData): 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[] = [];
@@ -47,11 +47,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);
@@ -172,7 +172,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
@@ -186,7 +186,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++;
@@ -202,3 +202,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;
+}