Skip to content

Commit

Permalink
Merge pull request #2199 from jerch/fix_REP
Browse files Browse the repository at this point in the history
fix repeat preceding character sequence (REP)
  • Loading branch information
Tyriar authored Jun 7, 2019
2 parents 4cfba55 + 925e58d commit 7139ee1
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 10 deletions.
58 changes: 58 additions & 0 deletions src/InputHandler.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,56 @@ describe('InputHandler Integration Tests', function(): void {
});
});
});

it('REP: Repeat preceding character, ECMA48 - CSI Ps b', async function(): Promise<any> {
// default to 1
await page.evaluate(`
window.term.resize(10, 10);
window.term.write('#\x1b[b');
window.term.writeln('');
window.term.write('#\x1b[0b');
window.term.writeln('');
window.term.write('#\x1b[1b');
window.term.writeln('');
window.term.write('#\x1b[5b');
`);
assert.deepEqual(await getLinesAsArray(4), ['##', '##', '##', '######']);
assert.deepEqual(await getCursor(), {col: 6, row: 3});
// should not repeat on fullwidth chars
await page.evaluate(`
window.term.reset();
window.term.write('¥\x1b[10b');
`);
assert.deepEqual(await getLinesAsArray(1), ['¥']);
// should repeat only base char of combining
await page.evaluate(`
window.term.reset();
window.term.write('e\u0301\x1b[5b');
`);
assert.deepEqual(await getLinesAsArray(1), ['e\u0301eeeee']);
// should wrap correctly
await page.evaluate(`
window.term.reset();
window.term.write('#\x1b[15b');
`);
assert.deepEqual(await getLinesAsArray(2), ['##########', '######']);
await page.evaluate(`
window.term.reset();
window.term.write('\x1b[?7l'); // disable wrap around
window.term.write('#\x1b[15b');
`);
assert.deepEqual(await getLinesAsArray(2), ['##########', '']);
// any successful sequence should reset REP
await page.evaluate(`
window.term.reset();
window.term.write('\x1b[?7h'); // re-enable wrap around
window.term.write('#\\n\x1b[3b');
window.term.write('#\\r\x1b[3b');
window.term.writeln('');
window.term.write('abcdefg\x1b[3D\x1b[10b#\x1b[3b');
`);
assert.deepEqual(await getLinesAsArray(3), ['#', ' #', 'abcd####']);
});
});
});

Expand Down Expand Up @@ -274,3 +324,11 @@ async function simulatePaste(text: string): Promise<string> {
`);
return await page.evaluate(`window.result_${id}`);
}

async function getCursor(): Promise<{col: number, row: number}> {
return page.evaluate(`
(function() {
return {col: term.buffer.cursorX, row: term.buffer.cursorY};
})();
`);
}
52 changes: 43 additions & 9 deletions src/InputHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,20 @@ export class InputHandler extends Disposable implements IInputHandler {
}
}
}
// store last char in Parser.precedingCodepoint for REP to work correctly
// This needs to check whether:
// - fullwidth + surrogates: reset
// - combining: only base char gets carried on (bug in xterm?)
if (end) {
bufferRow.loadCell(buffer.x - 1, this._workCell);
if (this._workCell.getWidth() === 2 || this._workCell.getCode() > 0xFFFF) {
this._parser.precedingCodepoint = 0;
} else if (this._workCell.isCombined()) {
this._parser.precedingCodepoint = this._workCell.getChars().charCodeAt(0);
} else {
this._parser.precedingCodepoint = this._workCell.content;
}
}
this._terminal.updateRange(buffer.y);
}

Expand Down Expand Up @@ -1004,17 +1018,37 @@ export class InputHandler extends Disposable implements IInputHandler {

/**
* CSI Ps b Repeat the preceding graphic character Ps times (REP).
* From ECMA 48 (@see http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-048.pdf)
* Notation: (Pn)
* Representation: CSI Pn 06/02
* Parameter default value: Pn = 1
* REP is used to indicate that the preceding character in the data stream,
* if it is a graphic character (represented by one or more bit combinations) including SPACE,
* is to be repeated n times, where n equals the value of Pn.
* If the character preceding REP is a control function or part of a control function,
* the effect of REP is not defined by this Standard.
*
* Since we propagate the terminal as xterm-256color we have to follow xterm's behavior:
* - fullwidth + surrogate chars are ignored
* - for combining chars only the base char gets repeated
* - text attrs are applied normally
* - wrap around is respected
* - any valid sequence resets the carried forward char
*
* Note: To get reset on a valid sequence working correctly without much runtime penalty,
* the preceding codepoint is stored on the parser in `this.print` and reset during `parser.parse`.
*/
public repeatPrecedingCharacter(params: number[]): void {
// make buffer local for faster access
const buffer = this._terminal.buffer;
const line = buffer.lines.get(buffer.ybase + buffer.y);
line.loadCell(buffer.x - 1, this._workCell);
line.replaceCells(buffer.x,
buffer.x + (params[0] || 1),
(this._workCell.content !== undefined) ? this._workCell : buffer.getNullCell(DEFAULT_ATTR_DATA)
);
// FIXME: no updateRange here?
if (!this._parser.precedingCodepoint) {
return;
}
// call print to insert the chars and handle correct wrapping
const length = params[0] || 1;
const data = new Uint32Array(length);
for (let i = 0; i < length; ++i) {
data[i] = this._parser.precedingCodepoint;
}
this.print(data, 0, data.length);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Terminal.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ if (os.platform() !== 'win32') {
// for (let i = 0; i < files.length; ++i) console.debug(i, files[i]);
// only successful tests for now
const skip = [
10, 16, 17, 19, 32, 33, 34, 35, 36, 39,
10, 16, 17, 19, 32, 34, 35, 36, 39,
40, 42, 43, 44, 45, 46, 47, 48, 49, 50,
51, 52, 54, 55, 56, 57, 58, 59, 60, 61,
63, 68
Expand Down
11 changes: 11 additions & 0 deletions src/core/parser/EscapeSequenceParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ class DcsDummy implements IDcsHandler {
export class EscapeSequenceParser extends Disposable implements IEscapeSequenceParser {
public initialState: number;
public currentState: number;
public precedingCodepoint: number;

// buffers over several parse calls
protected _osc: string;
Expand Down Expand Up @@ -264,6 +265,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
this._osc = '';
this._params = [0];
this._collect = '';
this.precedingCodepoint = 0;

// set default fallback handlers and handler lookup containers
this._printHandlerFb = (data, start, end): void => { };
Expand Down Expand Up @@ -394,6 +396,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
this._params = [0];
this._collect = '';
this._activeDcsHandler = null;
this.precedingCodepoint = 0;
}

/**
Expand Down Expand Up @@ -453,6 +456,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
}
break;
case ParserAction.EXECUTE:
this.precedingCodepoint = 0;
callback = this._executeHandlers[code];
if (callback) callback();
else this._executeHandlerFb(code);
Expand All @@ -474,6 +478,10 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
// inject values: currently not implemented
break;
case ParserAction.CSI_DISPATCH:
// dont reset preceding codepoint for REP itself
if (code !== 98) { // 'b'
this.precedingCodepoint = 0;
}
// Trigger CSI Handler
const handlers = this._csiHandlers[code];
let j = handlers ? handlers.length - 1 : -1;
Expand All @@ -499,6 +507,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
collect += String.fromCharCode(code);
break;
case ParserAction.ESC_DISPATCH:
this.precedingCodepoint = 0;
callback = this._escHandlers[collect + String.fromCharCode(code)];
if (callback) callback(collect, code);
else this._escHandlerFb(collect, code);
Expand All @@ -509,6 +518,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
collect = '';
break;
case ParserAction.DCS_HOOK:
this.precedingCodepoint = 0;
dcsHandler = this._dcsHandlers[collect + String.fromCharCode(code)];
if (!dcsHandler) dcsHandler = this._dcsHandlerFb;
dcsHandler.hook(collect, params, code);
Expand Down Expand Up @@ -550,6 +560,7 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
}
break;
case ParserAction.OSC_END:
this.precedingCodepoint = 0;
if (osc && code !== 0x18 && code !== 0x1a) {
// NOTE: OSC subparsing is not part of the original parser
// we do basic identifier parsing here to offer a jump table for OSC as well
Expand Down
6 changes: 6 additions & 0 deletions src/core/parser/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ export interface IDcsHandler {
* EscapeSequenceParser interface.
*/
export interface IEscapeSequenceParser extends IDisposable {
/**
* Preceding codepoint to get REP working correctly.
* This must be set by the print handler as last action.
* It gets reset by the parser for any valid sequence beside REP itself.
*/
precedingCodepoint: number;
/**
* Reset the parser to its initial state (handlers are kept).
*/
Expand Down

0 comments on commit 7139ee1

Please sign in to comment.