Skip to content

Commit

Permalink
repl: improve completion
Browse files Browse the repository at this point in the history
This improves the completion output by removing the nested special
handling. It never fully worked as expected and required a lot of
hacks to even keep it working halfway reliable. Our tests did not
cover syntax errors though and those can not be handled by this
implementation. Those break the layout and confuse the REPL.

Besides that the completion now also works in case the current line
has leading whitespace.

Also improve the error output in case the completion fails.

PR-URL: #30907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
BridgeAR authored and MylesBorins committed Dec 17, 2019
1 parent 3906e14 commit 1a8f828
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 83 deletions.
2 changes: 1 addition & 1 deletion lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) {
self.resume();

if (err) {
self._writeToOutput(`tab completion error ${inspect(err)}`);
self._writeToOutput(`Tab completion error: ${inspect(err)}`);
return;
}

Expand Down
70 changes: 11 additions & 59 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ const {
deprecate
} = require('internal/util');
const { inspect } = require('internal/util/inspect');
const Stream = require('stream');
const vm = require('vm');
const path = require('path');
const fs = require('fs');
Expand Down Expand Up @@ -115,7 +114,6 @@ const {
} = internalBinding('contextify');

const history = require('internal/repl/history');
const { setImmediate } = require('timers');

// Lazy-loaded.
let processTopLevelAwait;
Expand All @@ -124,7 +122,6 @@ const globalBuiltins =
new Set(vm.runInNewContext('Object.getOwnPropertyNames(globalThis)'));

const parentModule = module;
const replMap = new WeakMap();
const domainSet = new WeakSet();

const kBufferedCommandSymbol = Symbol('bufferedCommand');
Expand Down Expand Up @@ -550,14 +547,13 @@ function REPLServer(prompt,
self.lastError = e;
}

const top = replMap.get(self);
if (options[kStandaloneREPL] &&
process.listenerCount('uncaughtException') !== 0) {
process.nextTick(() => {
process.emit('uncaughtException', e);
top.clearBufferedCommand();
top.lines.level = [];
top.displayPrompt();
self.clearBufferedCommand();
self.lines.level = [];
self.displayPrompt();
});
} else {
if (errStack === '') {
Expand All @@ -583,10 +579,10 @@ function REPLServer(prompt,
}
// Normalize line endings.
errStack += errStack.endsWith('\n') ? '' : '\n';
top.outputStream.write(errStack);
top.clearBufferedCommand();
top.lines.level = [];
top.displayPrompt();
self.outputStream.write(errStack);
self.clearBufferedCommand();
self.lines.level = [];
self.displayPrompt();
}
});

Expand Down Expand Up @@ -897,7 +893,6 @@ exports.start = function(prompt,
ignoreUndefined,
replMode);
if (!exports.repl) exports.repl = repl;
replMap.set(repl, repl);
return repl;
};

Expand Down Expand Up @@ -1034,23 +1029,6 @@ REPLServer.prototype.turnOffEditorMode = deprecate(
'REPLServer.turnOffEditorMode() is deprecated',
'DEP0078');

// A stream to push an array into a REPL
// used in REPLServer.complete
function ArrayStream() {
Stream.call(this);

this.run = function(data) {
for (let n = 0; n < data.length; n++)
this.emit('data', `${data[n]}\n`);
};
}
ObjectSetPrototypeOf(ArrayStream.prototype, Stream.prototype);
ObjectSetPrototypeOf(ArrayStream, Stream);
ArrayStream.prototype.readable = true;
ArrayStream.prototype.writable = true;
ArrayStream.prototype.resume = function() {};
ArrayStream.prototype.write = function() {};

const requireRE = /\brequire\s*\(['"](([\w@./-]+\/)?(?:[\w@./-]*))/;
const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/;
const simpleExpressionRE =
Expand Down Expand Up @@ -1110,38 +1088,13 @@ REPLServer.prototype.complete = function() {
// Warning: This eval's code like "foo.bar.baz", so it will run property
// getter code.
function complete(line, callback) {
// There may be local variables to evaluate, try a nested REPL
if (this[kBufferedCommandSymbol] !== undefined &&
this[kBufferedCommandSymbol].length) {
// Get a new array of inputted lines
const tmp = this.lines.slice();
// Kill off all function declarations to push all local variables into
// global scope
for (let n = 0; n < this.lines.level.length; n++) {
const kill = this.lines.level[n];
if (kill.isFunction)
tmp[kill.line] = '';
}
const flat = new ArrayStream(); // Make a new "input" stream.
const magic = new REPLServer('', flat); // Make a nested REPL.
replMap.set(magic, replMap.get(this));
flat.run(tmp); // `eval` the flattened code.
// All this is only profitable if the nested REPL does not have a
// bufferedCommand.
if (!magic[kBufferedCommandSymbol]) {
magic._domain.on('error', (err) => {
setImmediate(() => {
throw err;
});
});
return magic.complete(line, callback);
}
}

// List of completion lists, one for each inheritance "level"
let completionGroups = [];
let completeOn, group;

// Ignore right whitespace. It could change the outcome.
line = line.trimLeft();

// REPL commands (e.g. ".break").
let filter;
let match = line.match(/^\s*\.(\w*)$/);
Expand Down Expand Up @@ -1465,8 +1418,7 @@ function _memory(cmd) {
// scope will not work for this function.
self.lines.level.push({
line: self.lines.length - 1,
depth: depth,
isFunction: /\bfunction\b/.test(cmd)
depth: depth
});
} else if (depth < 0) {
// Going... up.
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/repl-tab-completion-nested-repls.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const repl = require('repl');
const putIn = new ArrayStream();
const testMe = repl.start('', putIn);

// Some errors are passed to the domain, but do not callback
// Some errors are passed to the domain, but do not callback.
testMe._domain.on('error', function(err) {
throw err;
});
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-repl-save-load.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ testMe._domain.on('error', function(reason) {
});

const testFile = [
'let top = function() {',
'let inner = {one:1};'
'let inner = (function() {',
' return {one:1};',
'})()'
];
const saveFileName = join(tmpdir.path, 'test.save.js');

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-repl-tab-complete-nested-repls.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ const result = spawnSync(process.execPath, [testFile]);
// test here is to make sure that the error information bubbles up to the
// calling process.
assert.ok(result.status, 'testFile swallowed its error');
const err = result.stderr.toString();
assert.ok(err.includes('fhqwhgads'), err);
29 changes: 9 additions & 20 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,9 @@ const putIn = new ArrayStream();
const testMe = repl.start('', putIn);

// Some errors are passed to the domain, but do not callback
testMe._domain.on('error', function(err) {
assert.ifError(err);
});
testMe._domain.on('error', assert.ifError);

// Tab Complete will not break in an object literal
putIn.run(['.clear']);
putIn.run([
'var inner = {',
'one:1'
Expand Down Expand Up @@ -93,9 +90,7 @@ putIn.run([
'var top = function() {',
'var inner = {one:1};'
]);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));
testMe.complete('inner.o', getNoResultsFunction());

// When you close the function scope tab complete will not return the
// locally scoped variable
Expand All @@ -111,9 +106,7 @@ putIn.run([
' one:1',
'};'
]);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));
testMe.complete('inner.o', getNoResultsFunction());

putIn.run(['.clear']);

Expand All @@ -125,9 +118,7 @@ putIn.run([
' one:1',
'};'
]);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));
testMe.complete('inner.o', getNoResultsFunction());

putIn.run(['.clear']);

Expand All @@ -140,9 +131,7 @@ putIn.run([
' one:1',
'};'
]);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));
testMe.complete('inner.o', getNoResultsFunction());

putIn.run(['.clear']);

Expand All @@ -155,9 +144,7 @@ putIn.run([
' one:1',
'};'
]);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
}));
testMe.complete('inner.o', getNoResultsFunction());

putIn.run(['.clear']);

Expand Down Expand Up @@ -204,7 +191,9 @@ const spaceTimeout = setTimeout(function() {
}, 1000);

testMe.complete(' ', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, [[], undefined]);
assert.ifError(error);
assert.strictEqual(data[1], '');
assert.ok(data[0].includes('globalThis'));
clearTimeout(spaceTimeout);
}));

Expand Down

0 comments on commit 1a8f828

Please sign in to comment.