Skip to content

readline: show completions only after 2nd TAB#7754

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.

Sign up for GitHub

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

Closed
addaleax wants to merge 2 commits into from
Closed

readline: show completions only after 2nd TAB #7754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dca1f27
readline: show completions only after 2nd TAB
addaleax Jul 15, 2016
1ea8fb5
[squash] move optimization for a single completion to `commonPrefix()`
addaleax Aug 2, 2016
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
28 changes: 15 additions & 13 deletions lib/readline.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -41,6 +41,7 @@ function Interface(input, output, completer, terminal) {

this._sawReturn = false;
this.isCompletionEnabled = true;
this._previousKey = null;

EventEmitter.call(this);
var historySize;
Expand DownExpand Up@@ -391,7 +392,7 @@ Interface.prototype._insertString = function(c) {
}
};

Interface.prototype._tabComplete = function() {
Interface.prototype._tabComplete = function(lastKeypressWasTab) {
var self = this;

self.pause();
Expand All@@ -407,9 +408,7 @@ Interface.prototype._tabComplete = function() {
const completeOn = rv[1]; // the text that was completed
if (completions && completions.length) {
// Apply/show completions.
if (completions.length === 1) {
self._insertString(completions[0].slice(completeOn.length));
} else {
if (lastKeypressWasTab) {
self._writeToOutput('\r\n');
var width = completions.reduce(function(a, b) {
return a.length > b.length ? a : b;
Expand All@@ -429,16 +428,15 @@ Interface.prototype._tabComplete = function() {
}
}
handleGroup(self, group, width, maxColumns);
}

// If there is a common prefix to all matches, then apply that
// portion.
var f = completions.filter(function(e) { if (e) return e; });
var prefix = commonPrefix(f);
if (prefix.length > completeOn.length) {
self._insertString(prefix.slice(completeOn.length));
}

// If there is a common prefix to all matches, then apply that portion.
const f = completions.filter(function(e) { if (e) return e; });

@princejwesleyprincejwesleyAug 2, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@addaleax
we don't need to run this block of code when completions.length === 1, right?
I mean, self._insertString(completions[0].slice(completeOn.length)) is sufficient.

@addaleaxaddaleaxAug 2, 2016

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@princejwesley Yeah… I’m not sure if that’s worth it, at least for tab completion. ;) If you want, I can add a strings.length === 1 shortcut case to commonPrefix(), that’s probably where most of the unnecessary work would happen otherwise?

@princejwesleyprincejwesleyAug 2, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@addaleax Yes. Its good to add length check in commonPrefix().

@addaleaxaddaleaxAug 2, 2016

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@princejwesley Done! :)

const prefix = commonPrefix(f);
if (prefix.length > completeOn.length) {
self._insertString(prefix.slice(completeOn.length));
}

self._refreshLine();
}
});
Expand DownExpand Up@@ -474,6 +472,7 @@ function commonPrefix(strings) {
if (!strings || strings.length == 0) {
return '';
}
if (strings.length === 1) return strings[0];
var sorted = strings.slice().sort();
var min = sorted[0];
var max = sorted[sorted.length - 1];
Expand DownExpand Up@@ -688,7 +687,9 @@ Interface.prototype._moveCursor = function(dx) {

// handle a write from the tty
Interface.prototype._ttyWrite = function(s, key) {
const previousKey = this._previousKey;
key = key || {};
this._previousKey = key;

// Ignore escape key - Fixes #2876
if (key.name == 'escape') return;
Expand DownExpand Up@@ -892,7 +893,8 @@ Interface.prototype._ttyWrite = function(s, key) {
case 'tab':
// If tab completion enabled, do that...
if (typeof this.completer === 'function' && this.isCompletionEnabled) {
this._tabComplete();
const lastKeypressWasTab = previousKey && previousKey.name === 'tab';
this._tabComplete(lastKeypressWasTab);
break;
}
// falls through
Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-readline-undefined-columns.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
'use strict';

require('../common');
const common = require('../common');
const assert = require('assert');
const PassThrough = require('stream').PassThrough;
const readline = require('readline');
Expand All@@ -26,12 +26,17 @@ oStream.on('data', function(data) {
output += data;
});

oStream.on('end', function() {
oStream.on('end', common.mustCall(() => {
const expect = 'process.stdout\r\n' +
'process.stdin\r\n' +
'process.stderr';
assert(new RegExp(expect).test(output));
});
}));

iStream.write('process.s\t');

assert(/process.std\b/.test(output)); // Completion works.
assert(!/stdout/.test(output)); // Completion doesn’t show all results yet.

iStream.write('process.std\t');
iStream.write('\t');
oStream.end();