From a019c628c481cf29a521d790d9c2b9f2cdd492e5 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 9 Jan 2020 03:45:10 +0100 Subject: [PATCH 1/2] 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. --- 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 9cbb3c55c8d951..697c397b1e2b6b 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(); + }); +}); From 23c754f98c7a5e12b9342d71330e108560b9be2a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 10 Jan 2020 13:52:33 +0100 Subject: [PATCH 2/2] repl,readline: code cleanup This simplifies code that was more complicated than it had to be and removes code that should never be reached. --- lib/internal/readline/utils.js | 6 +----- lib/internal/repl/utils.js | 6 ++---- lib/readline.js | 37 +++++++++------------------------- lib/repl.js | 12 +++++------ 4 files changed, 18 insertions(+), 43 deletions(-) diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js index b867dec0cdb338..ffe0cee9d4ba3d 100644 --- a/lib/internal/readline/utils.js +++ b/lib/internal/readline/utils.js @@ -136,8 +136,7 @@ if (internalBinding('config').hasIntl) { (code >= 0x1b000 && code <= 0x1b001) || // Enclosed Ideographic Supplement (code >= 0x1f200 && code <= 0x1f251) || - // Miscellaneous Symbols and Pictographs 0x1f300 - 0x1f5ff - // Emoticons 0x1f600 - 0x1f64f + // Miscellaneous Symbols and Pictographs .. Emoticons (code >= 0x1f300 && code <= 0x1f64f) || // CJK Unified Ideographs Extension B .. Tertiary Ideographic Plane (code >= 0x20000 && code <= 0x3fffd) @@ -459,9 +458,6 @@ function* emitKeys(stream) { // This runs in O(n log n). function commonPrefix(strings) { - if (!strings || strings.length === 0) { - return ''; - } if (strings.length === 1) { return strings[0]; } diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index 20904d9d6a2445..826a3364c82ca4 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -320,10 +320,8 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { // Enter should automatically add the suffix to the current line as long as // escape was not pressed. We might even remove the preview in case any // cursor movement is triggered. - if (typeof repl.completer === 'function') { - const insertPreview = false; - showCompletionPreview(repl.line, insertPreview); - } + const insertPreview = false; + showCompletionPreview(repl.line, insertPreview); // Do not preview if the command is buffered. if (repl[bufferSymbol]) { diff --git a/lib/readline.js b/lib/readline.js index 697c397b1e2b6b..9a8b83834ddc50 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -735,27 +735,13 @@ Interface.prototype._getDisplayPos = function(str) { return { cols, rows }; }; - // Returns current cursor's position and line Interface.prototype.getCursorPos = function() { - const columns = this.columns; const strBeforeCursor = this._prompt + this.line.substring(0, this.cursor); - const dispPos = this._getDisplayPos(strBeforeCursor); - let cols = dispPos.cols; - let rows = dispPos.rows; - // If the cursor is on a full-width character which steps over the line, - // move the cursor to the beginning of the next line. - if (cols + 1 === columns && - this.cursor < this.line.length && - getStringWidth(this.line[this.cursor]) > 1) { - rows++; - cols = 0; - } - return { cols, rows }; + return this._getDisplayPos(strBeforeCursor); }; Interface.prototype._getCursorPos = Interface.prototype.getCursorPos; - // This function moves cursor dx places to the right // (-dx for left) and refreshes the line if it is needed. Interface.prototype._moveCursor = function(dx) { @@ -1119,31 +1105,32 @@ function emitKeypressEvents(stream, iface = {}) { stream[ESCAPE_DECODER] = emitKeys(stream); stream[ESCAPE_DECODER].next(); - const escapeCodeTimeout = () => stream[ESCAPE_DECODER].next(''); + const triggerEscape = () => stream[ESCAPE_DECODER].next(''); + const { escapeCodeTimeout = ESCAPE_CODE_TIMEOUT } = iface; let timeoutId; - function onData(b) { + function onData(input) { if (stream.listenerCount('keypress') > 0) { - const string = stream[KEYPRESS_DECODER].write(b); + const string = stream[KEYPRESS_DECODER].write(input); if (string) { clearTimeout(timeoutId); // This supports characters of length 2. iface._sawKeyPress = charLengthAt(string, 0) === string.length; - const escapeTimeout = iface.escapeCodeTimeout || ESCAPE_CODE_TIMEOUT; + iface.isCompletionEnabled = false; let length = 0; for (const character of string) { length += character.length; - if (character === '\t' && length !== string.length) { - iface.isCompletionEnabled = false; + if (length === string.length) { + iface.isCompletionEnabled = true; } try { stream[ESCAPE_DECODER].next(character); // Escape letter at the tail position - if (character === kEscape && length === string.length) { - timeoutId = setTimeout(escapeCodeTimeout, escapeTimeout); + if (length === string.length && character === kEscape) { + timeoutId = setTimeout(triggerEscape, escapeCodeTimeout); } } catch (err) { // If the generator throws (it could happen in the `keypress` @@ -1151,10 +1138,6 @@ function emitKeypressEvents(stream, iface = {}) { stream[ESCAPE_DECODER] = emitKeys(stream); stream[ESCAPE_DECODER].next(); throw err; - } finally { - if (iface) { - iface.isCompletionEnabled = true; - } } } } diff --git a/lib/repl.js b/lib/repl.js index 94457557c865ce..5ba0ed59af6b0f 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1361,15 +1361,13 @@ REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => { if (err) return callback(err); const [completions, completeOn = ''] = results; - const prefixLength = completeOn.length; + let result = completions.filter((v) => v); - if (prefixLength === 0) return callback(null, [[], completeOn]); - - const isNotEmpty = (v) => v.length > 0; - const trimCompleteOnPrefix = (v) => v.substring(prefixLength); - const data = completions.filter(isNotEmpty).map(trimCompleteOnPrefix); + if (completeOn && result.length !== 0) { + result = [commonPrefix(result)]; + } - callback(null, [[`${completeOn}${commonPrefix(data)}`], completeOn]); + callback(null, [result, completeOn]); }; REPLServer.prototype.defineCommand = function(keyword, cmd) {