Skip to content

Commit

Permalink
readline: set null as callback return in case there's no error
Browse files Browse the repository at this point in the history
The cursor move functions accept a callback. It was possible that
`undefined` was returned in case there was no error instead of null.

PR-URL: #31006
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
BridgeAR committed Jan 3, 2020
1 parent 92dcf3e commit ffbf790
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
8 changes: 4 additions & 4 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ function cursorTo(stream, x, y, callback) {

if (stream == null || (typeof x !== 'number' && typeof y !== 'number')) {
if (typeof callback === 'function')
process.nextTick(callback);
process.nextTick(callback, null);
return true;
}

Expand All @@ -1221,7 +1221,7 @@ function moveCursor(stream, dx, dy, callback) {

if (stream == null || !(dx || dy)) {
if (typeof callback === 'function')
process.nextTick(callback);
process.nextTick(callback, null);
return true;
}

Expand Down Expand Up @@ -1255,7 +1255,7 @@ function clearLine(stream, dir, callback) {

if (stream === null || stream === undefined) {
if (typeof callback === 'function')
process.nextTick(callback);
process.nextTick(callback, null);
return true;
}

Expand All @@ -1277,7 +1277,7 @@ function clearScreenDown(stream, callback) {

if (stream === null || stream === undefined) {
if (typeof callback === 'function')
process.nextTick(callback);
process.nextTick(callback, null);
return true;
}

Expand Down
16 changes: 12 additions & 4 deletions test/parallel/test-readline-csi.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ assert.throws(() => {
}, /ERR_INVALID_CALLBACK/);

// Verify that clearScreenDown() does not throw on null or undefined stream.
assert.strictEqual(readline.clearScreenDown(null, common.mustCall()), true);
assert.strictEqual(readline.clearScreenDown(null, common.mustCall((err) => {
assert.strictEqual(err, null);
})), true);
assert.strictEqual(readline.clearScreenDown(undefined, common.mustCall()),
true);

Expand Down Expand Up @@ -67,7 +69,9 @@ assert.throws(() => {
// Verify that clearLine() does not throw on null or undefined stream.
assert.strictEqual(readline.clearLine(null, 0), true);
assert.strictEqual(readline.clearLine(undefined, 0), true);
assert.strictEqual(readline.clearLine(null, 0, common.mustCall()), true);
assert.strictEqual(readline.clearLine(null, 0, common.mustCall((err) => {
assert.strictEqual(err, null);
})), true);
assert.strictEqual(readline.clearLine(undefined, 0, common.mustCall()), true);

// Nothing is written when moveCursor 0, 0
Expand Down Expand Up @@ -101,15 +105,19 @@ assert.throws(() => {
// Verify that moveCursor() does not throw on null or undefined stream.
assert.strictEqual(readline.moveCursor(null, 1, 1), true);
assert.strictEqual(readline.moveCursor(undefined, 1, 1), true);
assert.strictEqual(readline.moveCursor(null, 1, 1, common.mustCall()), true);
assert.strictEqual(readline.moveCursor(null, 1, 1, common.mustCall((err) => {
assert.strictEqual(err, null);
})), true);
assert.strictEqual(readline.moveCursor(undefined, 1, 1, common.mustCall()),
true);

// Undefined or null as stream should not throw.
assert.strictEqual(readline.cursorTo(null), true);
assert.strictEqual(readline.cursorTo(), true);
assert.strictEqual(readline.cursorTo(null, 1, 1, common.mustCall()), true);
assert.strictEqual(readline.cursorTo(undefined, 1, 1, common.mustCall()), true);
assert.strictEqual(readline.cursorTo(undefined, 1, 1, common.mustCall((err) => {
assert.strictEqual(err, null);
})), true);

writable.data = '';
assert.strictEqual(readline.cursorTo(writable, 'a'), true);
Expand Down

0 comments on commit ffbf790

Please sign in to comment.