Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make buffer length equal rows + scrollback, make alt buffer always 0 scrollback #855

Merged
merged 7 commits into from
Aug 16, 2017
37 changes: 34 additions & 3 deletions src/Buffer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ describe('Buffer', () => {
terminal.cols = INIT_COLS;
terminal.rows = INIT_ROWS;
terminal.options.scrollback = 1000;
buffer = new Buffer(terminal);
buffer = new Buffer(terminal, true);
});

describe('constructor', () => {
it('should create a CircularList with max length equal to scrollback, for its lines', () => {
it('should create a CircularList with max length equal to rows + scrollback, for its lines', () => {
assert.instanceOf(buffer.lines, CircularList);
assert.equal(buffer.lines.maxLength, terminal.options.scrollback);
assert.equal(buffer.lines.maxLength, terminal.rows + terminal.options.scrollback);
});
it('should set the Buffer\'s scrollBottom value equal to the terminal\'s rows -1', () => {
assert.equal(buffer.scrollBottom, terminal.rows - 1);
Expand Down Expand Up @@ -88,6 +88,21 @@ describe('Buffer', () => {
assert.equal(buffer.ydisp, 5);
assert.equal(buffer.ybase, 5);
});

describe('no scrollback', () => {
it('should trim from the top of the buffer when the cursor reaches the bottom', () => {
terminal.options.scrollback = 0;
buffer = new Buffer(terminal, true);
assert.equal(buffer.lines.maxLength, INIT_ROWS);
buffer.y = INIT_ROWS - 1;
buffer.fillViewportRows();
buffer.lines.get(5)[0][1] = 'a';
buffer.lines.get(INIT_ROWS - 1)[0][1] = 'b';
buffer.resize(INIT_COLS, INIT_ROWS - 5);
assert.equal(buffer.lines.get(0)[0][1], 'a');
assert.equal(buffer.lines.get(INIT_ROWS - 1 - 5)[0][1], 'b');
});
});
});

describe('row size increased', () => {
Expand Down Expand Up @@ -156,4 +171,20 @@ describe('Buffer', () => {
});
});
});

describe('buffer marked to have no scrollback', () => {
it('should always have a scrollback of 0', () => {
assert.equal(terminal.options.scrollback, 1000);
// Test size on initialization
buffer = new Buffer(terminal, false);
buffer.fillViewportRows();
assert.equal(buffer.lines.maxLength, INIT_ROWS);
// Test size on buffer increase
buffer.resize(INIT_COLS, INIT_ROWS * 2);
assert.equal(buffer.lines.maxLength, INIT_ROWS * 2);
// Test size on buffer decrease
buffer.resize(INIT_COLS, INIT_ROWS / 2);
assert.equal(buffer.lines.maxLength, INIT_ROWS / 2);
});
});
});
45 changes: 38 additions & 7 deletions src/Buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@ export class Buffer implements IBuffer {

/**
* Create a new Buffer.
* @param {Terminal} _terminal - The terminal the Buffer will belong to
* @param {number} ydisp - The scroll position of the Buffer in the viewport
* @param {number} ybase - The scroll position of the y cursor (ybase + y = the y position within the Buffer)
* @param {number} y - The cursor's y position after ybase
* @param {number} x - The cursor's x position after ybase
* @param _terminal The terminal the Buffer will belong to.
* @param _hasScrollback Whether the buffer should respecr the scrollback of
* the terminal..
*/
constructor(
private _terminal: ITerminal
private _terminal: ITerminal,
private _hasScrollback: boolean
) {
this.clear();
}
Expand All @@ -47,6 +46,18 @@ export class Buffer implements IBuffer {
return this._lines;
}

/**
* 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.
* @param rows The terminal rows to use in the calculation.
*/
private _getCorrectBufferLength(rows: number): number {
if (!this._hasScrollback) {
return rows;
}
return rows + this._terminal.options.scrollback;
}

/**
* Fills the buffer's viewport with blank lines.
*/
Expand All @@ -70,7 +81,7 @@ export class Buffer implements IBuffer {
this.scrollBottom = 0;
this.scrollTop = 0;
this.tabs = {};
this._lines = new CircularList<LineData>(this._terminal.options.scrollback);
this._lines = new CircularList<LineData>(this._getCorrectBufferLength(this._terminal.rows));
this.scrollBottom = this._terminal.rows - 1;
}

Expand All @@ -85,6 +96,13 @@ export class Buffer implements IBuffer {
return;
}

// Increase max length if needed before adjustments to allow space to fill
// as required.
const newMaxLength = this._getCorrectBufferLength(newRows);
if (newMaxLength > this._lines.maxLength) {
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?
Expand Down Expand Up @@ -136,6 +154,19 @@ export class Buffer implements IBuffer {
}
}

// 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;
}

// Make sure that the cursor stays on screen
if (this.y >= newRows) {
this.y = newRows - 1;
Expand Down
8 changes: 5 additions & 3 deletions src/BufferSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ export class BufferSet extends EventEmitter implements IBufferSet {
*/
constructor(private _terminal: ITerminal) {
super();
this._normal = new Buffer(this._terminal);
this._normal = new Buffer(this._terminal, true);
this._normal.fillViewportRows();
this._alt = new Buffer(this._terminal);

// The alt buffer should never have scrollback.
// See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-The-Alternate-Screen-Buffer
this._alt = new Buffer(this._terminal, false);
this._activeBuffer = this._normal;
}

Expand Down Expand Up @@ -71,7 +74,6 @@ export class BufferSet extends EventEmitter implements IBufferSet {
// Since the alt buffer is always cleared when the normal buffer is
// activated, we want to fill it when switching to it.
this._alt.fillViewportRows();

this._activeBuffer = this._alt;
this.emit('activate', this._alt);
}
Expand Down
27 changes: 5 additions & 22 deletions src/InputHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,23 +443,13 @@ export class InputHandler implements IInputHandler {
}
let row: number = this._terminal.buffer.y + this._terminal.buffer.ybase;

let j: number;
j = this._terminal.rows - 1 - this._terminal.buffer.scrollBottom;
j = this._terminal.rows - 1 + this._terminal.buffer.ybase - j + 1;

let scrollBottomRowsOffset = this._terminal.rows - 1 - this._terminal.buffer.scrollBottom;
let scrollBottomAbsolute = this._terminal.rows - 1 + this._terminal.buffer.ybase - scrollBottomRowsOffset + 1;
while (param--) {
if (this._terminal.buffer.lines.length === this._terminal.buffer.lines.maxLength) {
// Trim the start of lines to make room for the new line
this._terminal.buffer.lines.trimStart(1);
this._terminal.buffer.ybase--;
this._terminal.buffer.ydisp--;
row--;
j--;
}
// test: echo -e '\e[44m\e[1L\e[0m'
// blankLine(true) - xterm/linux behavior
this._terminal.buffer.lines.splice(scrollBottomAbsolute - 1, 1);
this._terminal.buffer.lines.splice(row, 0, this._terminal.blankLine(true));
this._terminal.buffer.lines.splice(j, 1);
}

// this.maxRange();
Expand All @@ -481,18 +471,11 @@ 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;

while (param--) {
if (this._terminal.buffer.lines.length === this._terminal.buffer.lines.maxLength) {
// Trim the start of lines to make room for the new line
this._terminal.buffer.lines.trimStart(1);
this._terminal.buffer.ybase -= 1;
this._terminal.buffer.ydisp -= 1;
}
// test: echo -e '\e[44m\e[1M\e[0m'
// blankLine(true) - xterm/linux behavior
this._terminal.buffer.lines.splice(j + 1, 0, this._terminal.blankLine(true));
this._terminal.buffer.lines.splice(row, 1);
this._terminal.buffer.lines.splice(row - 1, 1);
this._terminal.buffer.lines.splice(j, 0, this._terminal.blankLine(true));
}

// this.maxRange();
Expand Down
15 changes: 0 additions & 15 deletions src/Terminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,6 @@ describe('term.js addons', () => {
});

describe('setOption', () => {
let originalWarn;
let warnCallCount;
beforeEach(() => {
originalWarn = console.warn;
warnCallCount = 0;
console.warn = () => warnCallCount++;
});
afterEach(() => {
console.warn = originalWarn;
});
it('should set the option correctly', () => {
term.setOption('cursorBlink', true);
assert.equal(term.options.cursorBlink, true);
Expand All @@ -114,11 +104,6 @@ describe('term.js addons', () => {
it('should throw when setting a non-existant option', () => {
assert.throws(term.setOption.bind(term, 'fake', true));
});
it('should warn and do nothing when scrollback is less than number of rows', () => {
term.setOption('scrollback', term.rows - 1);
assert.equal(term.getOption('scrollback'), 1000);
assert.equal(warnCallCount, 1);
});
});

describe('clear', () => {
Expand Down
33 changes: 11 additions & 22 deletions src/Terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,18 +453,10 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
}
break;
case 'scrollback':
if (value < this.rows) {
let msg = 'Setting the scrollback value less than the number of rows ';

msg += `(${this.rows}) is not allowed.`;

console.warn(msg);
return;
}

if (this.options[key] !== value) {
if (this.buffer.lines.length > value) {
const amountToTrim = this.buffer.lines.length - value;
const newBufferLength = this.rows + value;
if (this.buffer.lines.length > newBufferLength) {
const amountToTrim = this.buffer.lines.length - newBufferLength;
const needsRefresh = (this.buffer.ydisp - amountToTrim < 0);
this.buffer.lines.trimStart(amountToTrim);
this.buffer.ybase = Math.max(this.buffer.ybase - amountToTrim, 0);
Expand All @@ -473,8 +465,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
this.refresh(0, this.rows - 1);
}
}
this.buffer.lines.maxLength = value;
this.viewport.syncScrollArea();
}
break;
}
Expand All @@ -487,6 +477,10 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
this.element.classList.toggle(`xterm-cursor-style-underline`, value === 'underline');
this.element.classList.toggle(`xterm-cursor-style-bar`, value === 'bar');
break;
case 'scrollback':
this.buffers.resize(this.cols, this.rows);
this.viewport.syncScrollArea();
break;
case 'tabStopWidth': this.setupStops(); break;
}
}
Expand Down Expand Up @@ -1178,17 +1172,16 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
let row;

// Make room for the new row in lines
if (this.buffer.lines.length === this.buffer.lines.maxLength) {
const bufferNeedsTrimming = this.buffer.lines.length === this.buffer.lines.maxLength;
if (bufferNeedsTrimming) {
this.buffer.lines.trimStart(1);
this.buffer.ybase--;
if (this.buffer.ydisp !== 0) {
this.buffer.ydisp--;
}
this.buffer.ydisp = Math.max(this.buffer.ydisp - 1, 0);
}

this.buffer.ybase++;

// TODO: Why is this done twice?
// Scroll the viewport down to the bottom if the user is not scrolling
if (!this.userScrolling) {
this.buffer.ydisp = this.buffer.ybase;
}
Expand Down Expand Up @@ -1918,10 +1911,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT
return;
}

if (y > this.getOption('scrollback')) {
this.setOption('scrollback', y);
}

let line;
let el;
let i;
Expand Down
4 changes: 4 additions & 0 deletions src/utils/CircularList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
import { assert } from 'chai';
import { CircularList } from './CircularList';

class TestCircularList<T> extends CircularList<T> {
public get array(): T[] { return this._array; }
}

describe('CircularList', () => {
describe('push', () => {
it('should push values onto the array', () => {
Expand Down
20 changes: 14 additions & 6 deletions src/utils/CircularList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,37 @@ import { EventEmitter } from '../EventEmitter';
import { ICircularList } from '../Interfaces';

export class CircularList<T> extends EventEmitter implements ICircularList<T> {
private _array: T[];
protected _array: T[];
private _startIndex: number;
private _length: number;

constructor(maxLength: number) {
constructor(
private _maxLength: number
) {
super();
this._array = new Array<T>(maxLength);
this._array = new Array<T>(this._maxLength);
this._startIndex = 0;
this._length = 0;
}

public get maxLength(): number {
return this._array.length;
return this._maxLength;
}

public set maxLength(newMaxLength: number) {
// There was no change in maxLength, return early.
if (this._maxLength === newMaxLength) {
return;
}

// Reconstruct array, starting at index 0. Only transfer values from the
// indexes 0 to length.
let newArray = new Array<T>(newMaxLength);
for (let i = 0; i < Math.min(newMaxLength, this.length); i++) {
newArray[i] = this._array[this._getCyclicIndex(i)];
}
this._array = newArray;
this._maxLength = newMaxLength;
this._startIndex = 0;
}

Expand Down Expand Up @@ -89,9 +97,9 @@ export class CircularList<T> extends EventEmitter implements ICircularList<T> {
*/
public push(value: T): void {
this._array[this._getCyclicIndex(this._length)] = value;
if (this._length === this.maxLength) {
if (this._length === this._maxLength) {
this._startIndex++;
if (this._startIndex === this.maxLength) {
if (this._startIndex === this._maxLength) {
this._startIndex = 0;
}
this.emit('trim', 1);
Expand Down