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

repl: added support for custom completions. #7527

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 3 additions & 0 deletions doc/api/repl.md
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ added: v0.1.91
`undefined`. Defaults to `false`.
* `writer` {Function} The function to invoke to format the output of each
command before writing to `output`. Defaults to [`util.inspect()`][].
* `completer` {Function} An optional function used for custom Tab auto
completion. See [`readline.InterfaceCompleter`][] for an example.
* `replMode` - A flag that specifies whether the default evaluator executes
all JavaScript commands in strict mode, default mode, or a hybrid mode
("magic" mode.) Acceptable values are:
Expand Down Expand Up @@ -526,3 +528,4 @@ see: https://gist.github.com/2053342
[`util.inspect()`]: util.html#util_util_inspect_object_options
[here]: util.html#util_custom_inspect_function_on_objects
[`readline.Interface`]: readline.html#readline_class_interface
[`readline.InterfaceCompleter`]: readline.html#readline_use_of_the_completer_function
17 changes: 11 additions & 6 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,14 +386,15 @@ function REPLServer(prompt,
self.bufferedCommand = '';
self.lines.level = [];

function complete(text, callback) {
self.complete(text, callback);
}
// Figure out which "complete" function to use.
self.completer = (typeof options.completer === 'function')
? options.completer
: complete;

Interface.call(this, {
input: self.inputStream,
output: self.outputStream,
completer: complete,
completer: self.completer,
terminal: options.terminal,
historySize: options.historySize,
prompt
Expand Down Expand Up @@ -698,6 +699,10 @@ function filteredOwnPropertyNames(obj) {
return Object.getOwnPropertyNames(obj).filter(intFilter);
}

REPLServer.prototype.complete = function() {
Copy link
Member

Choose a reason for hiding this comment

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

The way I read the code, it seems this function was added to REPLServer.prototype in order to allow for simpler testing. The result is a a greater API surface area. Is there a need for this function to be available at the user level? If not, perhaps is there a way to accomplish the testing requirements without adding to the prototype.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lance I'm not sure about its origins or all its purposes (besides the one related to testing), since it was already there when I added the feature, I just updated it so nothing is broken.

Copy link
Member

Choose a reason for hiding this comment

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

You are correct. My mistake!

Copy link
Member

@lance lance Jul 6, 2016

Choose a reason for hiding this comment

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

Not really related to your PR, @diosney, but just to follow up on my comment. I've noticed this in a number of places - methods attached to an object's prototype that are ostensibly public, but undocumented. In my view, this is not ideal and should be rectified by deciding what is intended to be public and what is not, and structuring the code and modifying the docs so they reflect the true intent.

@Trott we spoke briefly about issues similar to this (e.g. someObj._someSemiPrivateProperty) during onboarding. I scanned through the issues but could not find anything that addressed this as an overarching concern. If there is some place where this is being discussed, can you point me in the right direction? If not, would it be reasonable to start this discussion with an issue on nodejs/node?

Copy link
Member

Choose a reason for hiding this comment

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

@lance If you can't find a place where it's been discussed, opening an issue is totally appropriate in my opinion. Worst case scenario is someone points you to another repo or venue to have the conversation and closes the issue.

this.completer.apply(this, arguments);
};

// Provide a list of completions for the given leading text. This is
// given to the readline interface for handling tab completion.
//
Expand All @@ -708,7 +713,7 @@ function filteredOwnPropertyNames(obj) {
//
// Warning: This eval's code like "foo.bar.baz", so it will run property
// getter code.
REPLServer.prototype.complete = function(line, callback) {
function complete(line, callback) {
// There may be local variables to evaluate, try a nested REPL
if (this.bufferedCommand !== undefined && this.bufferedCommand.length) {
// Get a new array of inputed lines
Expand Down Expand Up @@ -967,7 +972,7 @@ REPLServer.prototype.complete = function(line, callback) {

callback(null, [completions || [], completeOn]);
}
};
}


/**
Expand Down
67 changes: 66 additions & 1 deletion test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ testMe.complete('console.lo', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, [['console.log'], 'console.lo']);
}));

// Tab Complete will return globaly scoped variables
// Tab Complete will return globally scoped variables
putIn.run(['};']);
testMe.complete('inner.o', common.mustCall(function(error, data) {
assert.deepStrictEqual(data, works);
Expand Down Expand Up @@ -283,3 +283,68 @@ if (typeof Intl === 'object') {
testNonGlobal.complete('I', common.mustCall((error, data) => {
assert.deepStrictEqual(data, builtins);
}));

// To test custom completer function.
// Sync mode.
const customCompletions = 'aaa aa1 aa2 bbb bb1 bb2 bb3 ccc ddd eee'.split(' ');
const testCustomCompleterSyncMode = repl.start({
prompt: '',
input: putIn,
output: putIn,
completer: function completerSyncMode(line) {
const hits = customCompletions.filter((c) => {
return c.indexOf(line) === 0;
});
// Show all completions if none found.
return [hits.length ? hits : customCompletions, line];
}
});

// On empty line should output all the custom completions
// without complete anything.
testCustomCompleterSyncMode.complete('', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [
customCompletions,
''
]);
}));

// On `a` should output `aaa aa1 aa2` and complete until `aa`.
testCustomCompleterSyncMode.complete('a', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [
'aaa aa1 aa2'.split(' '),
'a'
]);
}));

// To test custom completer function.
// Async mode.
const testCustomCompleterAsyncMode = repl.start({
prompt: '',
input: putIn,
output: putIn,
completer: function completerAsyncMode(line, callback) {
const hits = customCompletions.filter((c) => {
return c.indexOf(line) === 0;
});
// Show all completions if none found.
callback(null, [hits.length ? hits : customCompletions, line]);
}
});

// On empty line should output all the custom completions
// without complete anything.
testCustomCompleterAsyncMode.complete('', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [
customCompletions,
''
]);
}));

// On `a` should output `aaa aa1 aa2` and complete until `aa`.
testCustomCompleterAsyncMode.complete('a', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [
'aaa aa1 aa2'.split(' '),
'a'
]);
}));