From b92d65dbe713cef16d4e51245ed66f5e1dad2d7d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 4 Jan 2020 09:05:20 +0100 Subject: [PATCH] repl: activate previews for lines exceeding the terminal columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This improves the completion previews by activating them for lines that exceed the current terminal columns. As a drive-by fix it also simplifies some statements. PR-URL: https://github.com/nodejs/node/pull/31112 Reviewed-By: Michaël Zasso Reviewed-By: James M Snell --- lib/internal/repl/utils.js | 67 +++++++++++-------- test/parallel/test-repl-history-navigation.js | 3 + 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index 1b9a0209f898cb..69910e886848bb 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -136,14 +136,16 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { 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 cursorPos = repl.line.length !== repl.cursor ? + repl.getCursorPos() : + displayPos; + return { displayPos, cursorPos }; } const clearPreview = () => { if (inputPreview !== null) { - const { rows } = getPreviewPos(); + const { displayPos, cursorPos } = getPreviewPos(); + const rows = displayPos.rows - cursorPos.rows + 1; moveCursor(repl.output, 0, rows); clearLine(repl.output); moveCursor(repl.output, 0, -rows); @@ -153,12 +155,25 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { if (completionPreview !== null) { // Prevent cursor moves if not necessary! const move = repl.line.length !== repl.cursor; + let pos, rows; if (move) { - cursorTo(repl.output, repl._prompt.length + repl.line.length); + pos = getPreviewPos(); + cursorTo(repl.output, pos.displayPos.cols); + rows = pos.displayPos.rows - pos.cursorPos.rows; + moveCursor(repl.output, 0, rows); + } + const totalLine = `${repl._prompt}${repl.line}${completionPreview}`; + const newPos = repl._getDisplayPos(totalLine); + // Minimize work for the terminal. It is enough to clear the right part of + // the current line in case the preview is visible on a single line. + if (newPos.rows === 0 || (pos && pos.displayPos.rows === newPos.rows)) { + clearLine(repl.output, 1); + } else { + clearScreenDown(repl.output); } - clearLine(repl.output, 1); if (move) { - cursorTo(repl.output, repl._prompt.length + repl.cursor); + cursorTo(repl.output, pos.cursorPos.cols); + moveCursor(repl.output, 0, -rows); } completionPreview = null; } @@ -198,17 +213,6 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { const suffix = prefix.slice(completeOn.length); - const totalLength = repl.line.length + - repl._prompt.length + - suffix.length + - (repl.useColors ? 0 : 4); - - // TODO(BridgeAR): Fix me. This should not be necessary. See similar - // comment in `showPreview()`. - if (totalLength > repl.columns) { - return; - } - if (insertPreview) { repl._insertString(suffix); return; @@ -220,11 +224,17 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { `\u001b[90m${suffix}\u001b[39m` : ` // ${suffix}`; + const { cursorPos, displayPos } = getPreviewPos(); if (repl.line.length !== repl.cursor) { - cursorTo(repl.output, repl._prompt.length + repl.line.length); + cursorTo(repl.output, displayPos.cols); + moveCursor(repl.output, 0, displayPos.rows - cursorPos.rows); } repl.output.write(result); - cursorTo(repl.output, repl._prompt.length + repl.cursor); + cursorTo(repl.output, cursorPos.cols); + const totalLine = `${repl._prompt}${repl.line}${suffix}`; + const newPos = repl._getDisplayPos(totalLine); + const rows = newPos.rows - cursorPos.rows - (newPos.cols === 0 ? 1 : 0); + moveCursor(repl.output, 0, -rows); }); } @@ -288,6 +298,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { }, () => callback(new ERR_INSPECTOR_NOT_AVAILABLE())); } + // TODO(BridgeAR): Prevent previews while pasting code. const showPreview = () => { // Prevent duplicated previews after a refresh. if (inputPreview !== null) { @@ -373,12 +384,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 { cursorPos, displayPos } = getPreviewPos(); + const rows = displayPos.rows - cursorPos.rows; + moveCursor(repl.output, 0, rows); const { cols: resultCols } = repl._getDisplayPos(result); repl.output.write(`\n${result}`); - moveCursor(repl.output, cursorCols - resultCols, -previewRows); + moveCursor(repl.output, cursorPos.cols - resultCols, -rows - 1); }); }; @@ -452,8 +463,8 @@ function setupReverseSearch(repl) { // Reset the already matched set in case the direction is changed. That // way it's possible to find those entries again. alreadyMatched.clear(); + dir = keyName; } - dir = keyName; return true; } @@ -598,16 +609,14 @@ function setupReverseSearch(repl) { // Clear screen and write the current repl.line before exiting. cursorTo(repl.output, promptPos.cols); - if (promptPos.rows !== 0) - moveCursor(repl.output, 0, promptPos.rows); + moveCursor(repl.output, 0, promptPos.rows); clearScreenDown(repl.output); if (repl.line !== '') { repl.output.write(repl.line); if (repl.line.length !== repl.cursor) { const { cols, rows } = repl.getCursorPos(); cursorTo(repl.output, cols); - if (rows !== 0) - moveCursor(repl.output, 0, rows); + moveCursor(repl.output, 0, rows); } } } diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index b1cdc5cbdfc41e..c3710fa5a9b03d 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -201,6 +201,9 @@ const tests = [ // 236 + 2 + 4 + 8 '\x1B[1G', '\x1B[0J', `${prompt}${' '.repeat(236)} fun`, '\x1B[243G', + ' // ction', '\x1B[243G', + ' // ction', '\x1B[243G', + '\x1B[0K', // 2. UP '\x1B[1G', '\x1B[0J', `${prompt}${' '.repeat(235)} fun`, '\x1B[242G',