From 7582bce58dab01508da31587b459902a0beb0171 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 9 Jan 2020 03:45:10 +0100 Subject: [PATCH] readline: improve unicode support and tab completion 1. This reduces the number of write operations used during tab completion. 2. The tab completion calculated the string width using the length of the string instead of using the actual width. That is fixed. 3. The key decoder is now capable of handling characters composed out of two code points. That reduces the number of "keypress" events that are emitted which again lowers the amount of writes triggered. PR-URL: https://github.com/nodejs/node/pull/31288 Reviewed-By: James M Snell Reviewed-By: Rich Trott --- lib/internal/readline/utils.js | 3 +- lib/readline.js | 123 +++++++++----------- test/parallel/test-readline-tab-complete.js | 68 +++++++++++ 3 files changed, 123 insertions(+), 71 deletions(-) create mode 100644 test/parallel/test-readline-tab-complete.js diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js index 3257ee3a1e4a1f..b867dec0cdb338 100644 --- a/lib/internal/readline/utils.js +++ b/lib/internal/readline/utils.js @@ -163,7 +163,6 @@ function stripVTControlCharacters(str) { return str.replace(ansi, ''); } - /* Some patterns seen in terminal key escape codes, derived from combos seen at http://www.midnight-commander.org/browser/lib/tty/key.c @@ -450,7 +449,7 @@ function* emitKeys(stream) { if (s.length !== 0 && (key.name !== undefined || escaped)) { /* Named character or sequence */ stream.emit('keypress', escaped ? undefined : s, key); - } else if (s.length === 1) { + } else if (charLengthAt(s, 0) === s.length) { /* Single unnamed character, e.g. "." */ stream.emit('keypress', s, key); } diff --git a/lib/readline.js b/lib/readline.js index c16bb059183598..2a475fcf671a55 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -494,81 +494,67 @@ Interface.prototype._insertString = function(c) { }; Interface.prototype._tabComplete = function(lastKeypressWasTab) { - const self = this; - - self.pause(); - self.completer(self.line.slice(0, self.cursor), function onComplete(err, rv) { - self.resume(); + this.pause(); + this.completer(this.line.slice(0, this.cursor), (err, value) => { + this.resume(); if (err) { - self._writeToOutput(`Tab completion error: ${inspect(err)}`); + this._writeToOutput(`Tab completion error: ${inspect(err)}`); return; } // Result and the text that was completed. - const [completions, completeOn] = rv; + const [completions, completeOn] = value; if (!completions || completions.length === 0) { return; } - // Apply/show completions. - if (lastKeypressWasTab) { - self._writeToOutput('\r\n'); - const width = completions.reduce((a, b) => { - return a.length > b.length ? a : b; - }).length + 2; // 2 space padding - let maxColumns = MathFloor(self.columns / width); - if (!maxColumns || maxColumns === Infinity) { - maxColumns = 1; - } - let group = []; - for (const c of completions) { - if (c === '') { - handleGroup(self, group, width, maxColumns); - group = []; - } else { - group.push(c); - } - } - handleGroup(self, group, width, maxColumns); - } - // If there is a common prefix to all matches, then apply that portion. - const f = completions.filter((e) => e); - const prefix = commonPrefix(f); + const prefix = commonPrefix(completions.filter((e) => e !== '')); if (prefix.length > completeOn.length) { - self._insertString(prefix.slice(completeOn.length)); + this._insertString(prefix.slice(completeOn.length)); + return; } - self._refreshLine(); - }); -}; + if (!lastKeypressWasTab) { + return; + } -// this = Interface instance -function handleGroup(self, group, width, maxColumns) { - if (group.length === 0) { - return; - } - const minRows = MathCeil(group.length / maxColumns); - for (let row = 0; row < minRows; row++) { - for (let col = 0; col < maxColumns; col++) { - const idx = row * maxColumns + col; - if (idx >= group.length) { - break; + // Apply/show completions. + const completionsWidth = completions.map((e) => getStringWidth(e)); + const width = MathMax(...completionsWidth) + 2; // 2 space padding + let maxColumns = MathFloor(this.columns / width) || 1; + if (maxColumns === Infinity) { + maxColumns = 1; + } + let output = '\r\n'; + let lineIndex = 0; + let whitespace = 0; + for (let i = 0; i < completions.length; i++) { + const completion = completions[i]; + if (completion === '' || lineIndex === maxColumns) { + output += '\r\n'; + lineIndex = 0; + whitespace = 0; + } else { + output += ' '.repeat(whitespace); } - const item = group[idx]; - self._writeToOutput(item); - if (col < maxColumns - 1) { - for (let s = 0; s < width - item.length; s++) { - self._writeToOutput(' '); - } + if (completion !== '') { + output += completion; + whitespace = width - completionsWidth[i]; + lineIndex++; + } else { + output += '\r\n'; } } - self._writeToOutput('\r\n'); - } - self._writeToOutput('\r\n'); -} + if (lineIndex !== 0) { + output += '\r\n\r\n'; + } + this._writeToOutput(output); + this._refreshLine(); + }); +}; Interface.prototype._wordLeft = function() { if (this.cursor > 0) { @@ -1125,7 +1111,7 @@ Interface.prototype[SymbolAsyncIterator] = function() { * accepts a readable Stream instance and makes it emit "keypress" events */ -function emitKeypressEvents(stream, iface) { +function emitKeypressEvents(stream, iface = {}) { if (stream[KEYPRESS_DECODER]) return; stream[KEYPRESS_DECODER] = new StringDecoder('utf8'); @@ -1138,26 +1124,25 @@ function emitKeypressEvents(stream, iface) { function onData(b) { if (stream.listenerCount('keypress') > 0) { - const r = stream[KEYPRESS_DECODER].write(b); - if (r) { + const string = stream[KEYPRESS_DECODER].write(b); + if (string) { clearTimeout(timeoutId); - let escapeTimeout = ESCAPE_CODE_TIMEOUT; - - if (iface) { - iface._sawKeyPress = r.length === 1; - escapeTimeout = iface.escapeCodeTimeout; - } + // This supports characters of length 2. + iface._sawKeyPress = charLengthAt(string, 0) === string.length; + const escapeTimeout = iface.escapeCodeTimeout || ESCAPE_CODE_TIMEOUT; - for (let i = 0; i < r.length; i++) { - if (r[i] === '\t' && typeof r[i + 1] === 'string' && iface) { + let length = 0; + for (const character of string) { + length += character.length; + if (character === '\t' && length !== string.length) { iface.isCompletionEnabled = false; } try { - stream[ESCAPE_DECODER].next(r[i]); + stream[ESCAPE_DECODER].next(character); // Escape letter at the tail position - if (r[i] === kEscape && i + 1 === r.length) { + if (character === kEscape && length === string.length) { timeoutId = setTimeout(escapeCodeTimeout, escapeTimeout); } } catch (err) { diff --git a/test/parallel/test-readline-tab-complete.js b/test/parallel/test-readline-tab-complete.js new file mode 100644 index 00000000000000..bbdb18bd8015de --- /dev/null +++ b/test/parallel/test-readline-tab-complete.js @@ -0,0 +1,68 @@ +'use strict'; + +// Flags: --expose_internals + +const common = require('../common'); +const readline = require('readline'); +const assert = require('assert'); +const EventEmitter = require('events').EventEmitter; +const { getStringWidth } = require('internal/readline/utils'); + +// This test verifies that the tab completion supports unicode and the writes +// are limited to the minimum. +[ + 'あ', + '𐐷', + '🐕' +].forEach((char) => { + [true, false].forEach((lineBreak) => { + const completer = (line) => [ + [ + 'First group', + '', + `${char}${'a'.repeat(10)}`, `${char}${'b'.repeat(10)}`, char.repeat(11), + ], + line + ]; + + let output = ''; + const width = getStringWidth(char) - 1; + + class FakeInput extends EventEmitter { + columns = ((width + 1) * 10 + (lineBreak ? 0 : 10)) * 3 + + write = common.mustCall((data) => { + output += data; + }, 6) + + resume() {} + pause() {} + end() {} + } + + const fi = new FakeInput(); + const rli = new readline.Interface({ + input: fi, + output: fi, + terminal: true, + completer: completer + }); + + const last = '\r\nFirst group\r\n\r\n' + + `${char}${'a'.repeat(10)}${' '.repeat(2 + width * 10)}` + + `${char}${'b'.repeat(10)}` + + (lineBreak ? '\r\n' : ' '.repeat(2 + width * 10)) + + `${char.repeat(11)}\r\n` + + `\r\n\u001b[1G\u001b[0J> ${char}\u001b[${4 + width}G`; + + const expectations = [char, '', last]; + + rli.on('line', common.mustNotCall()); + for (const character of `${char}\t\t`) { + fi.emit('data', character); + assert.strictEqual(output, expectations.shift()); + output = ''; + } + rli.close(); + }); +});