From 9a8b7a4d8de143ce81c2211fa32772f65c1299fb Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 25 Aug 2017 11:26:46 -0700 Subject: [PATCH 1/5] Fix scroll region scrolling with 0 scrollback --- src/Buffer.ts | 3 +- src/Terminal.test.ts | 140 ++++++++++++++++++++++++++++++++++++++++++- src/Terminal.ts | 118 ++++++++++++++++++++++++++---------- 3 files changed, 228 insertions(+), 33 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index fd6f8aef..7af090ac 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -78,10 +78,9 @@ export class Buffer implements IBuffer { this.ybase = 0; this.y = 0; this.x = 0; - this.scrollBottom = 0; - this.scrollTop = 0; this.tabs = {}; this._lines = new CircularList(this._getCorrectBufferLength(this._terminal.rows)); + this.scrollTop = 0; this.scrollBottom = this._terminal.rows - 1; } diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index 771835bb..0c333741 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -3,6 +3,9 @@ import { Terminal } from './Terminal'; import { MockViewport, MockCompositionHelper } from './utils/TestUtils.test'; import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX } from './Buffer'; +const INIT_COLS = 80; +const INIT_ROWS = 24; + class TestTerminal extends Terminal { public evaluateKeyEscapeSequence(ev: any): {cancel: boolean, key: string, scrollDisp: number} { return this._evaluateKeyEscapeSequence(ev); } public keyDown(ev: any): boolean { return this._keyDown(ev); } @@ -13,7 +16,10 @@ describe('term.js addons', () => { let term: TestTerminal; beforeEach(() => { - term = new TestTerminal(); + term = new TestTerminal({ + cols: INIT_COLS, + rows: INIT_ROWS + }); term.refresh = () => {}; term.viewport = new MockViewport(); (term).compositionHelper = new MockCompositionHelper(); @@ -283,6 +289,138 @@ describe('term.js addons', () => { assert.equal(term.buffer.ydisp, startYDisp - 1); }); }); + + describe('scroll() function', () => { + describe('when scrollback > 0', () => { + it('should create a new line and scroll', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(INIT_ROWS - 1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS + 1); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a'); + assert.equal(term.buffer.lines.get(INIT_ROWS - 1)[0][CHAR_DATA_CHAR_INDEX], 'b'); + assert.equal(term.buffer.lines.get(INIT_ROWS)[0][CHAR_DATA_CHAR_INDEX], ' '); + }); + + it('should properly scroll inside a scroll region (scrollTop set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + term.buffer.scrollTop = 1; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'c'); + }); + + it('should properly scroll inside a scroll region (scrollBottom set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX] = 'd'; + term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX] = 'e'; + term.buffer.y = 3; + term.buffer.scrollBottom = 3; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS + 1); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a', '\'a\' should be pushed to the scrollback'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'b'); + assert.equal(term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX], 'c'); + assert.equal(term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX], 'd'); + assert.equal(term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX], ' ', 'a blank line should be added at scrollBottom\'s index'); + assert.equal(term.buffer.lines.get(5)[0][CHAR_DATA_CHAR_INDEX], 'e'); + }); + + it('should properly scroll inside a scroll region (scrollTop and scrollBottom set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX] = 'd'; + term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX] = 'e'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + term.buffer.scrollTop = 1; + term.buffer.scrollBottom = 3; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'c', '\'b\' should be removed from the buffer'); + assert.equal(term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX], 'd'); + assert.equal(term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX], ' ', 'a blank line should be added at scrollBottom\'s index'); + assert.equal(term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX], 'e'); + }); + }); + + describe('when scrollback === 0', () => { + beforeEach(() => { + term.setOption('scrollback', 0); + assert.equal(term.buffer.lines.maxLength, INIT_ROWS); + }); + + it('should create a new line and shift everything up', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(INIT_ROWS - 1)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + assert.equal(term.buffer.lines.length, INIT_ROWS); + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + // 'a' gets pushed out of buffer + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'b'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], ' '); + assert.equal(term.buffer.lines.get(INIT_ROWS - 2)[0][CHAR_DATA_CHAR_INDEX], 'c'); + assert.equal(term.buffer.lines.get(INIT_ROWS - 1)[0][CHAR_DATA_CHAR_INDEX], ' '); + }); + + it('should properly scroll inside a scroll region (scrollTop set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + term.buffer.scrollTop = 1; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'c'); + }); + + it('should properly scroll inside a scroll region (scrollBottom set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX] = 'd'; + term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX] = 'e'; + term.buffer.y = 3; + term.buffer.scrollBottom = 3; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'b'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'c'); + assert.equal(term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX], 'd'); + assert.equal(term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX], ' ', 'a blank line should be added at scrollBottom\'s index'); + assert.equal(term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX], 'e'); + }); + + it('should properly scroll inside a scroll region (scrollTop and scrollBottom set)', () => { + term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX] = 'a'; + term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX] = 'b'; + term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX] = 'c'; + term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX] = 'd'; + term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX] = 'e'; + term.buffer.y = INIT_ROWS - 1; // Move cursor to last line + term.buffer.scrollTop = 1; + term.buffer.scrollBottom = 3; + term.scroll(); + assert.equal(term.buffer.lines.length, INIT_ROWS); + assert.equal(term.buffer.lines.get(0)[0][CHAR_DATA_CHAR_INDEX], 'a'); + assert.equal(term.buffer.lines.get(1)[0][CHAR_DATA_CHAR_INDEX], 'c', '\'b\' should be removed from the buffer'); + assert.equal(term.buffer.lines.get(2)[0][CHAR_DATA_CHAR_INDEX], 'd'); + assert.equal(term.buffer.lines.get(3)[0][CHAR_DATA_CHAR_INDEX], ' ', 'a blank line should be added at scrollBottom\'s index'); + assert.equal(term.buffer.lines.get(4)[0][CHAR_DATA_CHAR_INDEX], 'e'); + }); + }); + }); }); describe('evaluateKeyEscapeSequence', () => { diff --git a/src/Terminal.ts b/src/Terminal.ts index 03babac5..a083d9d4 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1167,46 +1167,104 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT * line. */ public scroll(isWrapped?: boolean): void { - let row; + // The problem is caused when scrollback = 0. This section was written + // around the assumption that there would be buffer to trim when necessary. - // Make room for the new row in lines - const bufferNeedsTrimming = this.buffer.lines.length === this.buffer.lines.maxLength; - if (bufferNeedsTrimming) { - this.buffer.lines.trimStart(1); - this.buffer.ybase--; - this.buffer.ydisp = Math.max(this.buffer.ydisp - 1, 0); + + // shiftElements should be used here + + // If scrollTop is set (non-zero), we should shiftElements + // scrollBottom defines the position at which the new element is inserted + + // Note that both scrollBottom and scrollTop need to be handled individually + // because they can be set to terminal.rows or 0 respectively + + // Don't mess with ybase temporarily, asking for trouble + + // This should work with minimal effort utilizing the CiruclarList + + // TODO: Need to hold onto top given the if? + const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; + console.log('bottomRow', bottomRow); + console.log('this.buffer.scrollTop: ' + this.buffer.scrollTop); + if (this.buffer.scrollTop === 0) { + const willBufferBeTrimmed = this.buffer.lines.length === this.buffer.lines.maxLength; + + if (bottomRow === this.buffer.lines.length - 1) { + // Pushing when possible is faster than splicing + this.buffer.lines.push(this.blankLine(undefined, isWrapped)); + console.log('push'); + } else { + // Insert a row *below* the bottomRow, pushing the top row into the scrollback + this.buffer.lines.splice(bottomRow + 1, 0, this.blankLine(undefined, isWrapped)); + console.log('splice'); + } + + // Adjust ydisp and ybase to accommodate the changes after the buffer was + // trimmed. + if (!willBufferBeTrimmed) { + console.log('increment ydisp/ybase'); + this.buffer.ybase++; + this.buffer.ydisp++; + } + console.log('this.buffer.ybase: ' + this.buffer.ybase); + console.log('this.buffer.ybase: ' + this.buffer.ydisp); + } else { + const topRow = this.buffer.ybase + this.buffer.scrollTop; + const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; +console.log('shiftElements'); +console.log('topRow: ' + topRow + '(' + this.buffer.lines.get(topRow)[0][1] + ')'); +console.log('scrollRegionHeight: ' + scrollRegionHeight); + this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); + this.buffer.lines.set(bottomRow, this.blankLine(undefined, isWrapped)); } - this.buffer.ybase++; - - // Scroll the viewport down to the bottom if the user is not scrolling if (!this.userScrolling) { this.buffer.ydisp = this.buffer.ybase; } - // last line - row = this.buffer.ybase + this.rows - 1; - // subtract the bottom scroll region - row -= this.rows - 1 - this.buffer.scrollBottom; + // Make room for the new row in lines + // const bufferNeedsTrimming = this.buffer.lines.length === this.buffer.lines.maxLength; + // if (bufferNeedsTrimming) { + // this.buffer.lines.trimStart(1); + // this.buffer.ybase--; + // this.buffer.ydisp = Math.max(this.buffer.ydisp - 1, 0); + // } - if (row === this.buffer.lines.length) { - // Optimization: pushing is faster than splicing when they amount to the same behavior - this.buffer.lines.push(this.blankLine(undefined, isWrapped)); - } else { - // add our new line - this.buffer.lines.splice(row, 0, this.blankLine(undefined, isWrapped)); - } + // this.buffer.ybase++; - if (this.buffer.scrollTop !== 0) { - if (this.buffer.ybase !== 0) { - this.buffer.ybase--; - if (!this.userScrolling) { - this.buffer.ydisp = this.buffer.ybase; - } - } - this.buffer.lines.splice(this.buffer.ybase + this.buffer.scrollTop, 1); - } + // // Scroll the viewport down to the bottom if the user is not scrolling + // if (!this.userScrolling) { + // this.buffer.ydisp = this.buffer.ybase; + // } + + // // last line + // let row = this.buffer.ybase + this.rows - 1; + + // // subtract the bottom scroll region + // row -= this.rows - 1 - this.buffer.scrollBottom; + + // // Same as this? + // // row = this.buffer.ybase + this.buffer.scrollBottom; + + // if (row === this.buffer.lines.length) { + // // Optimization: pushing is faster than splicing when they amount to the same behavior + // this.buffer.lines.push(this.blankLine(undefined, isWrapped)); + // } else { + // // add our new line + // this.buffer.lines.splice(row, 0, this.blankLine(undefined, isWrapped)); + // } + + // if (this.buffer.scrollTop !== 0) { + // if (this.buffer.ybase !== 0) { + // this.buffer.ybase--; + // if (!this.userScrolling) { + // this.buffer.ydisp = this.buffer.ybase; + // } + // } + // this.buffer.lines.splice(this.buffer.ybase + this.buffer.scrollTop, 1); + // } // this.maxRange(); this.updateRange(this.buffer.scrollTop); From a2161532db2deb3b05da3f1238790bc929797552 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 25 Aug 2017 12:11:20 -0700 Subject: [PATCH 2/5] Fix reverseIndex to work with scroll regions and 0 scrollback --- src/Buffer.ts | 4 ++++ src/InputHandler.ts | 3 +++ src/Terminal.ts | 48 +++++++++++++++++++++++++++++++++------------ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 7af090ac..53645837 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -46,6 +46,10 @@ export class Buffer implements IBuffer { return this._lines; } + public get hasScrollback(): boolean { + return this._hasScrollback && this.lines.maxLength > this._terminal.rows; + } + /** * Gets the correct buffer length based on the rows provided, the terminal's * scrollback and whether this buffer is flagged to have scrollback or not. diff --git a/src/InputHandler.ts b/src/InputHandler.ts index f2d97751..e6ab5540 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -464,10 +464,13 @@ export class InputHandler implements IInputHandler { let j: number; j = this._terminal.rows - 1 - this._terminal.buffer.scrollBottom; j = this._terminal.rows - 1 + this._terminal.buffer.ybase - j; + console.log('deleteLines', params); while (param--) { // test: echo -e '\e[44m\e[1M\e[0m' // blankLine(true) - xterm/linux behavior + console.log('splice delete row: ' + (row - 1) + '(' + this._terminal.buffer.lines.get(row - 1)[0][1] + ')'); this._terminal.buffer.lines.splice(row - 1, 1); + console.log('splice add blank row: ' + (j) + '(' + this._terminal.buffer.lines.get(j)[0][1] + ')'); this._terminal.buffer.lines.splice(j, 0, this._terminal.blankLine(true)); } diff --git a/src/Terminal.ts b/src/Terminal.ts index a083d9d4..fa3161f0 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1184,39 +1184,62 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT // This should work with minimal effort utilizing the CiruclarList // TODO: Need to hold onto top given the if? - const bottomRow = this.buffer.ybase + this.buffer.scrollBottom; + const newLine = this.blankLine(undefined, isWrapped); + const topRow = this.buffer.ybase + this.buffer.scrollTop; + let bottomRow = this.buffer.ybase + this.buffer.scrollBottom; + + + +// TODO: There's a problem with ybase going beyond scrollback? + + console.log('bottomRow', bottomRow); console.log('this.buffer.scrollTop: ' + this.buffer.scrollTop); if (this.buffer.scrollTop === 0) { const willBufferBeTrimmed = this.buffer.lines.length === this.buffer.lines.maxLength; + // Remove the line first if there is no scrollback so the list is not + // trimmed + if (!this.buffer.hasScrollback) { + this.buffer.lines.splice(topRow, 1); + // Adjust bottomRow to make up for the deleted row + // bottomRow--; + } + if (bottomRow === this.buffer.lines.length - 1) { // Pushing when possible is faster than splicing - this.buffer.lines.push(this.blankLine(undefined, isWrapped)); + this.buffer.lines.push(newLine); console.log('push'); } else { // Insert a row *below* the bottomRow, pushing the top row into the scrollback - this.buffer.lines.splice(bottomRow + 1, 0, this.blankLine(undefined, isWrapped)); + if (!this.buffer.hasScrollback) { + // A line is deleted in this case so bottomRow is pushed up + this.buffer.lines.splice(bottomRow, 0, newLine); + } else { + this.buffer.lines.splice(bottomRow + 1, 0, newLine); + } console.log('splice'); } - // Adjust ydisp and ybase to accommodate the changes after the buffer was - // trimmed. + // Only adjust ybase and ydisp when the buffer is not trimmed if (!willBufferBeTrimmed) { console.log('increment ydisp/ybase'); this.buffer.ybase++; this.buffer.ydisp++; } + console.log('this.buffer.ybase: ' + this.buffer.ybase); - console.log('this.buffer.ybase: ' + this.buffer.ydisp); + console.log('this.buffer.ydisp: ' + this.buffer.ydisp); + } else { - const topRow = this.buffer.ybase + this.buffer.scrollTop; const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; -console.log('shiftElements'); -console.log('topRow: ' + topRow + '(' + this.buffer.lines.get(topRow)[0][1] + ')'); -console.log('scrollRegionHeight: ' + scrollRegionHeight); + + console.log('shiftElements'); + console.log('topRow: ' + topRow + '(' + this.buffer.lines.get(topRow)[0][1] + ')'); + console.log('scrollRegionHeight: ' + scrollRegionHeight); + this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); - this.buffer.lines.set(bottomRow, this.blankLine(undefined, isWrapped)); + this.buffer.lines.set(bottomRow, newLine); } if (!this.userScrolling) { @@ -2247,7 +2270,8 @@ console.log('scrollRegionHeight: ' + scrollRegionHeight); // possibly move the code below to term.reverseScroll(); // test: echo -ne '\e[1;1H\e[44m\eM\e[0m' // blankLine(true) is xterm/linux behavior - this.buffer.lines.shiftElements(this.buffer.y + this.buffer.ybase, this.rows - 1, 1); + const scrollRegionHeight = this.buffer.scrollBottom - this.buffer.scrollTop; + this.buffer.lines.shiftElements(this.buffer.y + this.buffer.ybase, scrollRegionHeight, 1); this.buffer.lines.set(this.buffer.y + this.buffer.ybase, this.blankLine(true)); this.updateRange(this.buffer.scrollTop); this.updateRange(this.buffer.scrollBottom); From 41463d8eb5ae6e604eb524c9be7787935f5a75d7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 25 Aug 2017 12:41:53 -0700 Subject: [PATCH 3/5] Make reverseIndex scroll region-aware This was causing a issue when scrollback was 0 as the start of the buffer was getting copied --- src/InputHandler.ts | 3 -- src/Terminal.ts | 107 ++++---------------------------------------- test/bar | 0 test/foo | 0 4 files changed, 9 insertions(+), 101 deletions(-) create mode 100644 test/bar create mode 100644 test/foo diff --git a/src/InputHandler.ts b/src/InputHandler.ts index e6ab5540..f2d97751 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -464,13 +464,10 @@ export class InputHandler implements IInputHandler { let j: number; j = this._terminal.rows - 1 - this._terminal.buffer.scrollBottom; j = this._terminal.rows - 1 + this._terminal.buffer.ybase - j; - console.log('deleteLines', params); while (param--) { // test: echo -e '\e[44m\e[1M\e[0m' // blankLine(true) - xterm/linux behavior - console.log('splice delete row: ' + (row - 1) + '(' + this._terminal.buffer.lines.get(row - 1)[0][1] + ')'); this._terminal.buffer.lines.splice(row - 1, 1); - console.log('splice add blank row: ' + (j) + '(' + this._terminal.buffer.lines.get(j)[0][1] + ')'); this._terminal.buffer.lines.splice(j, 0, this._terminal.blankLine(true)); } diff --git a/src/Terminal.ts b/src/Terminal.ts index fa3161f0..ea3800d0 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1163,133 +1163,44 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT /** * Scroll the terminal down 1 row, creating a blank line. - * @param {boolean} isWrapped Whether the new line is wrapped from the previous - * line. + * @param isWrapped Whether the new line is wrapped from the previous line. */ public scroll(isWrapped?: boolean): void { - // The problem is caused when scrollback = 0. This section was written - // around the assumption that there would be buffer to trim when necessary. - - - // shiftElements should be used here - - // If scrollTop is set (non-zero), we should shiftElements - // scrollBottom defines the position at which the new element is inserted - - // Note that both scrollBottom and scrollTop need to be handled individually - // because they can be set to terminal.rows or 0 respectively - - // Don't mess with ybase temporarily, asking for trouble - - // This should work with minimal effort utilizing the CiruclarList - - // TODO: Need to hold onto top given the if? const newLine = this.blankLine(undefined, isWrapped); const topRow = this.buffer.ybase + this.buffer.scrollTop; let bottomRow = this.buffer.ybase + this.buffer.scrollBottom; - - -// TODO: There's a problem with ybase going beyond scrollback? - - - console.log('bottomRow', bottomRow); - console.log('this.buffer.scrollTop: ' + this.buffer.scrollTop); if (this.buffer.scrollTop === 0) { + // Determine whether the buffer is going to be trimmed after insertion. const willBufferBeTrimmed = this.buffer.lines.length === this.buffer.lines.maxLength; - // Remove the line first if there is no scrollback so the list is not - // trimmed - if (!this.buffer.hasScrollback) { - this.buffer.lines.splice(topRow, 1); - // Adjust bottomRow to make up for the deleted row - // bottomRow--; - } - + // Insert the line using the fastest method if (bottomRow === this.buffer.lines.length - 1) { - // Pushing when possible is faster than splicing this.buffer.lines.push(newLine); - console.log('push'); } else { - // Insert a row *below* the bottomRow, pushing the top row into the scrollback - if (!this.buffer.hasScrollback) { - // A line is deleted in this case so bottomRow is pushed up - this.buffer.lines.splice(bottomRow, 0, newLine); - } else { - this.buffer.lines.splice(bottomRow + 1, 0, newLine); - } - console.log('splice'); + this.buffer.lines.splice(bottomRow + 1, 0, newLine); } // Only adjust ybase and ydisp when the buffer is not trimmed if (!willBufferBeTrimmed) { - console.log('increment ydisp/ybase'); this.buffer.ybase++; this.buffer.ydisp++; } - - console.log('this.buffer.ybase: ' + this.buffer.ybase); - console.log('this.buffer.ydisp: ' + this.buffer.ydisp); - } else { + // scrollTop is non-zero which means no line will be going to the + // scrollback, instead we can just shift them in-place. const scrollRegionHeight = bottomRow - topRow + 1/*as it's zero-based*/; - - console.log('shiftElements'); - console.log('topRow: ' + topRow + '(' + this.buffer.lines.get(topRow)[0][1] + ')'); - console.log('scrollRegionHeight: ' + scrollRegionHeight); - this.buffer.lines.shiftElements(topRow + 1, scrollRegionHeight - 1, -1); this.buffer.lines.set(bottomRow, newLine); } + // Move the viewport to the bottom of the buffer unless the user is + // scrolling. if (!this.userScrolling) { this.buffer.ydisp = this.buffer.ybase; } - - // Make room for the new row in lines - // const bufferNeedsTrimming = this.buffer.lines.length === this.buffer.lines.maxLength; - // if (bufferNeedsTrimming) { - // this.buffer.lines.trimStart(1); - // this.buffer.ybase--; - // this.buffer.ydisp = Math.max(this.buffer.ydisp - 1, 0); - // } - - // this.buffer.ybase++; - - // // Scroll the viewport down to the bottom if the user is not scrolling - // if (!this.userScrolling) { - // this.buffer.ydisp = this.buffer.ybase; - // } - - // // last line - // let row = this.buffer.ybase + this.rows - 1; - - // // subtract the bottom scroll region - // row -= this.rows - 1 - this.buffer.scrollBottom; - - // // Same as this? - // // row = this.buffer.ybase + this.buffer.scrollBottom; - - // if (row === this.buffer.lines.length) { - // // Optimization: pushing is faster than splicing when they amount to the same behavior - // this.buffer.lines.push(this.blankLine(undefined, isWrapped)); - // } else { - // // add our new line - // this.buffer.lines.splice(row, 0, this.blankLine(undefined, isWrapped)); - // } - - // if (this.buffer.scrollTop !== 0) { - // if (this.buffer.ybase !== 0) { - // this.buffer.ybase--; - // if (!this.userScrolling) { - // this.buffer.ydisp = this.buffer.ybase; - // } - // } - // this.buffer.lines.splice(this.buffer.ybase + this.buffer.scrollTop, 1); - // } - - // this.maxRange(); + // Flag rows that need updating this.updateRange(this.buffer.scrollTop); this.updateRange(this.buffer.scrollBottom); diff --git a/test/bar b/test/bar new file mode 100644 index 00000000..e69de29b diff --git a/test/foo b/test/foo new file mode 100644 index 00000000..e69de29b From 54cf30ae68f8b80b183f346241752dc2b99e2508 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 25 Aug 2017 12:47:45 -0700 Subject: [PATCH 4/5] Remove test files --- test/bar | 0 test/foo | 0 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test/bar delete mode 100644 test/foo diff --git a/test/bar b/test/bar deleted file mode 100644 index e69de29b..00000000 diff --git a/test/foo b/test/foo deleted file mode 100644 index e69de29b..00000000 From bc86664726bf2f1f0ca2e700f70545a125f3b369 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 25 Aug 2017 13:14:30 -0700 Subject: [PATCH 5/5] Improve Buffer.resize to work with uninitialized buffers Fixes #924 --- src/Buffer.ts | 138 +++++++++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 53645837..4f81025f 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -94,11 +94,6 @@ export class Buffer implements IBuffer { * @param newRows The new number of rows. */ public resize(newCols: number, newRows: number): void { - // Don't resize the buffer if it's empty and hasn't been used yet. - if (this._lines.length === 0) { - return; - } - // Increase max length if needed before adjustments to allow space to fill // as required. const newMaxLength = this._getCorrectBufferLength(newRows); @@ -106,83 +101,88 @@ export class Buffer implements IBuffer { this._lines.maxLength = newMaxLength; } - // Deal with columns increasing (we don't do anything when columns reduce) - if (this._terminal.cols < newCols) { - const ch: CharData = [this._terminal.defAttr, ' ', 1]; // does xterm use the default attr? - for (let i = 0; i < this._lines.length; i++) { - // TODO: This should be removed, with tests setup for the case that was - // causing the underlying bug, see https://github.com/sourcelair/xterm.js/issues/824 - if (this._lines.get(i) === undefined) { - this._lines.set(i, this._terminal.blankLine(undefined, undefined, newCols)); - } - while (this._lines.get(i).length < newCols) { - this._lines.get(i).push(ch); + // The following adjustments should only happen if the buffer has been + // initialized/filled. + if (this._lines.length > 0) { + // Deal with columns increasing (we don't do anything when columns reduce) + if (this._terminal.cols < newCols) { + const ch: CharData = [this._terminal.defAttr, ' ', 1]; // does xterm use the default attr? + for (let i = 0; i < this._lines.length; i++) { + // TODO: This should be removed, with tests setup for the case that was + // causing the underlying bug, see https://github.com/sourcelair/xterm.js/issues/824 + if (this._lines.get(i) === undefined) { + this._lines.set(i, this._terminal.blankLine(undefined, undefined, newCols)); + } + while (this._lines.get(i).length < newCols) { + this._lines.get(i).push(ch); + } } } - } - // Resize rows in both directions as needed - let addToY = 0; - if (this._terminal.rows < newRows) { - for (let y = this._terminal.rows; y < newRows; y++) { - if (this._lines.length < newRows + this.ybase) { - if (this.ybase > 0 && this._lines.length <= this.ybase + this.y + addToY + 1) { - // There is room above the buffer and there are no empty elements below the line, - // scroll up - this.ybase--; - addToY++; - if (this.ydisp > 0) { - // Viewport is at the top of the buffer, must increase downwards - this.ydisp--; + // Resize rows in both directions as needed + let addToY = 0; + if (this._terminal.rows < newRows) { + for (let y = this._terminal.rows; y < newRows; y++) { + if (this._lines.length < newRows + this.ybase) { + if (this.ybase > 0 && this._lines.length <= this.ybase + this.y + addToY + 1) { + // There is room above the buffer and there are no empty elements below the line, + // scroll up + this.ybase--; + addToY++; + if (this.ydisp > 0) { + // Viewport is at the top of the buffer, must increase downwards + this.ydisp--; + } + } else { + // Add a blank line if there is no buffer left at the top to scroll to, or if there + // are blank lines after the cursor + this._lines.push(this._terminal.blankLine(undefined, undefined, newCols)); + } + } + } + } else { // (this._terminal.rows >= newRows) + for (let y = this._terminal.rows; y > newRows; y--) { + if (this._lines.length > newRows + this.ybase) { + if (this._lines.length > this.ybase + this.y + 1) { + // The line is a blank line below the cursor, remove it + this._lines.pop(); + } else { + // The line is the cursor, scroll down + this.ybase++; + this.ydisp++; } - } else { - // Add a blank line if there is no buffer left at the top to scroll to, or if there - // are blank lines after the cursor - this._lines.push(this._terminal.blankLine(undefined, undefined, newCols)); } } } - } else { // (this._terminal.rows >= newRows) - for (let y = this._terminal.rows; y > newRows; y--) { - if (this._lines.length > newRows + this.ybase) { - if (this._lines.length > this.ybase + this.y + 1) { - // The line is a blank line below the cursor, remove it - this._lines.pop(); - } else { - // The line is the cursor, scroll down - this.ybase++; - this.ydisp++; - } + + // Reduce max length if needed after adjustments, this is done after as it + // would otherwise cut data from the bottom of the buffer. + if (newMaxLength < this._lines.maxLength) { + // Trim from the top of the buffer and adjust ybase and ydisp. + const amountToTrim = this._lines.length - newMaxLength; + if (amountToTrim > 0) { + this._lines.trimStart(amountToTrim); + this.ybase = Math.max(this.ybase - amountToTrim, 0); + this.ydisp = Math.max(this.ydisp - amountToTrim, 0); } + this._lines.maxLength = newMaxLength; } - } - // Reduce max length if needed after adjustments, this is done after as it - // would otherwise cut data from the bottom of the buffer. - if (newMaxLength < this._lines.maxLength) { - // Trim from the top of the buffer and adjust ybase and ydisp. - const amountToTrim = this._lines.length - newMaxLength; - if (amountToTrim > 0) { - this._lines.trimStart(amountToTrim); - this.ybase = Math.max(this.ybase - amountToTrim, 0); - this.ydisp = Math.max(this.ydisp - amountToTrim, 0); + // Make sure that the cursor stays on screen + if (this.y >= newRows) { + this.y = newRows - 1; } - this._lines.maxLength = newMaxLength; + if (addToY) { + this.y += addToY; + } + + if (this.x >= newCols) { + this.x = newCols - 1; + } + + this.scrollTop = 0; } - // Make sure that the cursor stays on screen - if (this.y >= newRows) { - this.y = newRows - 1; - } - if (addToY) { - this.y += addToY; - } - - if (this.x >= newCols) { - this.x = newCols - 1; - } - - this.scrollTop = 0; this.scrollBottom = newRows - 1; }