Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
readline: introduce promise-based API
PR-URL: #37947
Fixes: #37287
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
  • Loading branch information
aduh95 committed Sep 16, 2021
commit 8122d243ae010f3a5c1d50e4d0ef6374d4e407b4
467 changes: 417 additions & 50 deletions doc/api/readline.md

Large diffs are not rendered by default.

131 changes: 131 additions & 0 deletions lib/internal/readline/promises.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
'use strict';

const {
ArrayPrototypeJoin,
ArrayPrototypePush,
Promise,
} = primordials;

const { CSI } = require('internal/readline/utils');
const { validateInteger } = require('internal/validators');
const { isWritable } = require('internal/streams/utils');
const { codes: { ERR_INVALID_ARG_TYPE } } = require('internal/errors');

const {
kClearToLineBeginning,
kClearToLineEnd,
kClearLine,
kClearScreenDown,
} = CSI;

class Readline {
#stream;
#todo = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not blocking but private fields are still very slow in V8. Hidden symbols would probably be a lot more efficient (and consistent with what we do elsewhere)


constructor(stream) {
if (!isWritable(stream))
throw new ERR_INVALID_ARG_TYPE('stream', 'Writable', stream);
this.#stream = stream;
}

/**
* Moves the cursor to the x and y coordinate on the given stream.
* @param {integer} x
* @param {integer} [y]
* @returns {Readline} this
*/
cursorTo(x, y = undefined) {
validateInteger(x, 'x');
if (y != null) validateInteger(y, 'y');

ArrayPrototypePush(
this.#todo,
y == null ? CSI`${x + 1}G` : CSI`${y + 1};${x + 1}H`
);

return this;
}

/**
* Moves the cursor relative to its current location.
* @param {integer} dx
* @param {integer} dy
* @returns {Readline} this
*/
moveCursor(dx, dy) {
if (dx || dy) {
validateInteger(dx, 'dx');
validateInteger(dy, 'dy');

let data = '';

if (dx < 0) {
data += CSI`${-dx}D`;
} else if (dx > 0) {
data += CSI`${dx}C`;
}

if (dy < 0) {
data += CSI`${-dy}A`;
} else if (dy > 0) {
data += CSI`${dy}B`;
}
ArrayPrototypePush(this.#todo, data);
}
return this;
}

/**
* Clears the current line the cursor is on.
* @param {-1|0|1} dir Direction to clear:
* -1 for left of the cursor
* +1 for right of the cursor
* 0 for the entire line
* @returns {Readline} this
*/
clearLine(dir) {
validateInteger(dir, 'dir', -1, 1);

ArrayPrototypePush(
this.#todo,
dir < 0 ? kClearToLineBeginning : dir > 0 ? kClearToLineEnd : kClearLine
);
return this;
}

/**
* Clears the screen from the current position of the cursor down.
* @returns {Readline} this
*/
clearScreenDown() {
ArrayPrototypePush(this.#todo, kClearScreenDown);
return this;
}

/**
* Sends all the pending actions to the associated `stream` and clears the
* internal list of pending actions.
* @returns {Promise<void>} Resolves when all pending actions have been
* flushed to the associated `stream`.
*/
commit() {
return new Promise((resolve) => {
this.#stream.write(ArrayPrototypeJoin(this.#todo, ''), resolve);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should happen here if the the this.#stream errors?

Copy link
Contributor Author

@aduh95 aduh95 Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The promise would be rejected, I can add a test for this.
EDIT: test added in ad13959.

this.#todo = [];
});
}

/**
* Clears the internal list of pending actions without sending it to the
* associated `stream`.
* @returns {Readline} this
*/
rollback() {
this.#todo = [];
return this;
}
}

module.exports = {
Readline,
};
4 changes: 3 additions & 1 deletion lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const {
moveCursor,
} = require('internal/readline/callbacks');
const emitKeypressEvents = require('internal/readline/emitKeypressEvents');
const promises = require('readline/promises');

const {
AbortError,
Expand Down Expand Up @@ -462,5 +463,6 @@ module.exports = {
createInterface,
cursorTo,
emitKeypressEvents,
moveCursor
moveCursor,
promises,
};
51 changes: 51 additions & 0 deletions lib/readline/promises.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
'use strict';

const {
Promise,
} = primordials;

const {
Readline,
} = require('internal/readline/promises');

const {
Interface: _Interface,
kQuestionCancel,
} = require('internal/readline/interface');

const {
AbortError,
} = require('internal/errors');

class Interface extends _Interface {
// eslint-disable-next-line no-useless-constructor
constructor(input, output, completer, terminal) {
super(input, output, completer, terminal);
}
question(query, options = {}) {
return new Promise((resolve, reject) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use createDeferredPromise() as an alternative.

if (options.signal) {
if (options.signal.aborted) {
return reject(new AbortError());
}

options.signal.addEventListener('abort', () => {
this[kQuestionCancel]();
reject(new AbortError());
}, { once: true });
}

super.question(query, resolve);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to remove the event listener added on options.signal (and should probably use validateOptions?) after super.question finished.

});
}
}

function createInterface(input, output, completer, terminal) {
return new Interface(input, output, completer, terminal);
}

module.exports = {
Interface,
Readline,
createInterface,
};
163 changes: 163 additions & 0 deletions test/parallel/test-readline-promises-csi.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// Flags: --expose-internals


import '../common/index.mjs';
import assert from 'assert';
import { Readline } from 'readline/promises';
import { Writable } from 'stream';

import utils from 'internal/readline/utils';
const { CSI } = utils;

const INVALID_ARG = {
name: 'TypeError',
code: 'ERR_INVALID_ARG_TYPE',
};

class TestWritable extends Writable {
data = '';
_write(chunk, encoding, callback) {
this.data += chunk.toString();
callback();
}
}

[
undefined, null,
0, 1, 1n, 1.1, NaN, Infinity,
true, false,
Symbol(),
'', '1',
[], {}, () => {},
].forEach((arg) =>
assert.throws(() => new Readline(arg), INVALID_ARG)
);

{
const writable = new TestWritable();
const readline = new Readline(writable);

await readline.clearScreenDown().commit();
assert.deepStrictEqual(writable.data, CSI.kClearScreenDown);
await readline.clearScreenDown().commit();

writable.data = '';
await readline.clearScreenDown().rollback();
assert.deepStrictEqual(writable.data, '');

writable.data = '';
await readline.clearLine(-1).commit();
assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);

writable.data = '';
await readline.clearLine(1).commit();
assert.deepStrictEqual(writable.data, CSI.kClearToLineEnd);

writable.data = '';
await readline.clearLine(0).commit();
assert.deepStrictEqual(writable.data, CSI.kClearLine);

writable.data = '';
await readline.clearLine(-1).commit();
assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);

await readline.clearLine(0, null).commit();

// Nothing is written when moveCursor 0, 0
for (const set of
[
[0, 0, ''],
[1, 0, '\x1b[1C'],
[-1, 0, '\x1b[1D'],
[0, 1, '\x1b[1B'],
[0, -1, '\x1b[1A'],
[1, 1, '\x1b[1C\x1b[1B'],
[-1, 1, '\x1b[1D\x1b[1B'],
[-1, -1, '\x1b[1D\x1b[1A'],
[1, -1, '\x1b[1C\x1b[1A'],
]) {
writable.data = '';
await readline.moveCursor(set[0], set[1]).commit();
assert.deepStrictEqual(writable.data, set[2]);
writable.data = '';
await readline.moveCursor(set[0], set[1]).commit();
assert.deepStrictEqual(writable.data, set[2]);
}


await readline.moveCursor(1, 1, null).commit();

writable.data = '';
[
undefined, null,
true, false,
Symbol(),
'', '1',
[], {}, () => {},
].forEach((arg) =>
assert.throws(() => readline.cursorTo(arg), INVALID_ARG)
);
assert.strictEqual(writable.data, '');

writable.data = '';
assert.throws(() => readline.cursorTo('a', 'b'), INVALID_ARG);
assert.strictEqual(writable.data, '');

writable.data = '';
assert.throws(() => readline.cursorTo('a', 1), INVALID_ARG);
assert.strictEqual(writable.data, '');

writable.data = '';
assert.throws(() => readline.cursorTo(1, 'a'), INVALID_ARG);
assert.strictEqual(writable.data, '');

writable.data = '';
await readline.cursorTo(1).commit();
assert.strictEqual(writable.data, '\x1b[2G');

writable.data = '';
await readline.cursorTo(1, 2).commit();
assert.strictEqual(writable.data, '\x1b[3;2H');

writable.data = '';
await readline.cursorTo(1, 2).commit();
assert.strictEqual(writable.data, '\x1b[3;2H');

writable.data = '';
await readline.cursorTo(1).cursorTo(1, 2).commit();
assert.strictEqual(writable.data, '\x1b[2G\x1b[3;2H');

writable.data = '';
await readline.cursorTo(1).commit();
assert.strictEqual(writable.data, '\x1b[2G');

// Verify that cursorTo() rejects if x or y is NaN.
[1.1, NaN, Infinity].forEach((arg) => {
assert.throws(() => readline.cursorTo(arg), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
});
});

[1.1, NaN, Infinity].forEach((arg) => {
assert.throws(() => readline.cursorTo(1, arg), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
});
});

assert.throws(() => readline.cursorTo(NaN, NaN), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
});
}

{
const error = new Error();
const writable = new class extends Writable {
_write() { throw error; }
}();
const readline = new Readline(writable);

await assert.rejects(readline.cursorTo(1).commit(), error);
}
Loading