From 925dd8e7f9d7842f9e695c147e43cca3b8c61154 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 17 Dec 2019 18:00:13 +0100 Subject: [PATCH] repl: fix preview of lines that exceed the terminal columns This adds support for very long input lines to still display the input preview correct. PR-URL: https://github.com/nodejs/node/pull/31006 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anto Aravinth Reviewed-By: Rich Trott --- lib/internal/repl/utils.js | 29 ++++---- test/parallel/test-repl-history-navigation.js | 71 +++++++++++++++---- test/parallel/test-repl-preview.js | 14 ++-- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index c54e173bdf3e1c..906046c40c5ede 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -132,11 +132,19 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { let previewCompletionCounter = 0; let completionPreview = null; + function getPreviewPos() { + const displayPos = repl._getDisplayPos(`${repl._prompt}${repl.line}`); + const cursorPos = repl._getCursorPos(); + const rows = 1 + displayPos.rows - cursorPos.rows; + return { rows, cols: cursorPos.cols }; + } + const clearPreview = () => { if (inputPreview !== null) { - moveCursor(repl.output, 0, 1); + const { rows } = getPreviewPos(); + moveCursor(repl.output, 0, rows); clearLine(repl.output); - moveCursor(repl.output, 0, -1); + moveCursor(repl.output, 0, -rows); lastInputPreview = inputPreview; inputPreview = null; } @@ -280,16 +288,6 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { return; } - // Do not show previews in case the current line is longer than the column - // width. - // TODO(BridgeAR): Fix me. This should not be necessary. It currently breaks - // the output though. We also have to check for characters that have more - // than a single byte as length. Check Interface.prototype._moveCursor. It - // contains the necessary logic. - if (repl.line.length + repl._prompt.length > repl.columns) { - return; - } - // Add the autocompletion preview. // TODO(BridgeAR): Trigger the input preview after the completion preview. // That way it's possible to trigger the input prefix including the @@ -344,9 +342,12 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { `\u001b[90m${inspected}\u001b[39m` : `// ${inspected}`; + const { rows: previewRows, cols: cursorCols } = getPreviewPos(); + if (previewRows !== 1) + moveCursor(repl.output, 0, previewRows - 1); + const { cols: resultCols } = repl._getDisplayPos(result); repl.output.write(`\n${result}`); - moveCursor(repl.output, 0, -1); - cursorTo(repl.output, repl._prompt.length + repl.cursor); + moveCursor(repl.output, cursorCols - resultCols, -previewRows); }); }; diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index f73fbb9b0fd278..766b3f0424711d 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -108,7 +108,7 @@ const tests = [ env: { NODE_REPL_HISTORY: defaultHistoryPath }, skip: !process.features.inspector, test: [ - `const ${'veryLongName'.repeat(30)} = 'I should not be previewed'`, + `const ${'veryLongName'.repeat(30)} = 'I should be previewed'`, ENTER, 'const e = new RangeError("visible\\ninvisible")', ENTER, @@ -127,27 +127,70 @@ const tests = [ { env: { NODE_REPL_HISTORY: defaultHistoryPath }, columns: 250, + showEscapeCodes: true, skip: !process.features.inspector, test: [ UP, UP, UP, + WORD_LEFT, UP, BACKSPACE ], + // A = Cursor n up + // B = Cursor n down + // C = Cursor n forward + // D = Cursor n back + // G = Cursor to column n + // J = Erase in screen; 0 = right; 1 = left; 2 = total + // K = Erase in line; 0 = right; 1 = left; 2 = total expected: [ - prompt, + // 0. Start + '\x1B[1G', '\x1B[0J', + prompt, '\x1B[3G', + // 1. UP // This exceeds the maximum columns (250): // Whitespace + prompt + ' // '.length + 'function'.length // 236 + 2 + 4 + 8 - `${prompt}${' '.repeat(236)} fun`, - `${prompt}${' '.repeat(235)} fun`, - ' // ction', - ' // ction', - `${prompt}${'veryLongName'.repeat(30)}`, - `${prompt}e`, - '\n// RangeError: visible', - prompt + '\x1B[1G', '\x1B[0J', + `${prompt}${' '.repeat(236)} fun`, '\x1B[243G', + // 2. UP + '\x1B[1G', '\x1B[0J', + `${prompt}${' '.repeat(235)} fun`, '\x1B[242G', + // TODO(BridgeAR): Investigate why the preview is generated twice. + ' // ction', '\x1B[242G', + ' // ction', '\x1B[242G', + // Preview cleanup + '\x1B[0K', + // 3. UP + '\x1B[1G', '\x1B[0J', + // 'veryLongName'.repeat(30).length === 360 + // prompt.length === 2 + // 360 % 250 + 2 === 112 (+1) + `${prompt}${'veryLongName'.repeat(30)}`, '\x1B[113G', + // "// 'I should be previewed'".length + 86 === 112 (+1) + "\n// 'I should be previewed'", '\x1B[86C\x1B[1A', + // Preview cleanup + '\x1B[1B', '\x1B[2K', '\x1B[1A', + // 4. WORD LEFT + // Almost identical as above. Just one extra line. + // Math.floor(360 / 250) === 1 + '\x1B[1A', + '\x1B[1G', '\x1B[0J', + `${prompt}${'veryLongName'.repeat(30)}`, '\x1B[3G', '\x1B[1A', + '\x1B[1B', "\n// 'I should be previewed'", '\x1B[24D\x1B[2A', + // Preview cleanup + '\x1B[2B', '\x1B[2K', '\x1B[2A', + // 5. UP + '\x1B[1G', '\x1B[0J', + `${prompt}e`, '\x1B[4G', + // '// RangeError: visible'.length - 19 === 3 (+1) + '\n// RangeError: visible', '\x1B[19D\x1B[1A', + // Preview cleanup + '\x1B[1B', '\x1B[2K', '\x1B[1A', + // 6. Backspace + '\x1B[1G', '\x1B[0J', + prompt, '\x1B[3G' ], clean: true }, @@ -169,11 +212,11 @@ const tests = [ WORD_RIGHT, ENTER ], - // C = Cursor forward - // D = Cursor back + // C = Cursor n forward + // D = Cursor n back // G = Cursor to column n - // J = Erase in screen - // K = Erase in line + // J = Erase in screen; 0 = right; 1 = left; 2 = total + // K = Erase in line; 0 = right; 1 = left; 2 = total expected: [ // 0. // 'f' diff --git a/test/parallel/test-repl-preview.js b/test/parallel/test-repl-preview.js index cd34c461d80671..4846248bdba294 100644 --- a/test/parallel/test-repl-preview.js +++ b/test/parallel/test-repl-preview.js @@ -68,12 +68,12 @@ async function tests(options) { const testCases = [ ['foo', [2, 4], '[Function: foo]', 'foo', - '\x1B[90m[Function: foo]\x1B[39m\x1B[1A\x1B[11G\x1B[1B\x1B[2K\x1B[1A\r', + '\x1B[90m[Function: foo]\x1B[39m\x1B[5D\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[36m[Function: foo]\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['koo', [2, 4], '[Function: koo]', 'k\x1B[90moo\x1B[39m\x1B[9G\x1B[0Ko\x1B[90mo\x1B[39m\x1B[10G\x1B[0Ko', - '\x1B[90m[Function: koo]\x1B[39m\x1B[1A\x1B[11G\x1B[1B\x1B[2K\x1B[1A\r', + '\x1B[90m[Function: koo]\x1B[39m\x1B[5D\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[36m[Function: koo]\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['a', [1, 2], undefined], @@ -83,19 +83,19 @@ async function tests(options) { '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['1n + 2n', [2, 5], '\x1B[33m3n\x1B[39m', '1n + 2', - '\x1B[90mType[39m\x1B[1A\x1B[14G\x1B[1B\x1B[2K\x1B[1An', - '\x1B[90m3n\x1B[39m\x1B[1A\x1B[15G\x1B[1B\x1B[2K\x1B[1A\r', + '\x1B[90mType[39m\x1B[57D\x1B[1A\x1B[1B\x1B[2K\x1B[1An', + '\x1B[90m3n\x1B[39m\x1B[12C\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[33m3n\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['{ a: true };', [2, 4], '\x1B[33mtrue\x1B[39m', '{ a: tru\x1B[90me\x1B[39m\x1B[16G\x1B[0Ke };', - '\x1B[90mtrue\x1B[39m\x1B[1A\x1B[20G\x1B[1B\x1B[2K\x1B[1A\r', + '\x1B[90mtrue\x1B[39m\x1B[15C\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[33mtrue\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], [' \t { a: true};', [2, 5], '\x1B[33mtrue\x1B[39m', ' \t { a: tru\x1B[90me\x1B[39m\x1B[19G\x1B[0Ke}', - '\x1B[90m{ a: true }\x1B[39m\x1B[1A\x1B[21G\x1B[1B\x1B[2K\x1B[1A;', - '\x1B[90mtrue\x1B[39m\x1B[1A\x1B[22G\x1B[1B\x1B[2K\x1B[1A\r', + '\x1B[90m{ a: true }\x1B[39m\x1B[8C\x1B[1A\x1B[1B\x1B[2K\x1B[1A;', + '\x1B[90mtrue\x1B[39m\x1B[16C\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[33mtrue\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'] ];