From 83324e01267337803fe0faf9f5103b42c036dbba Mon Sep 17 00:00:00 2001 From: Giovanni Date: Sat, 8 Mar 2025 10:57:52 +0100 Subject: [PATCH 01/13] repl: add support for multiline history --- benchmark/repl/parse-history-from-file.js | 97 +++++++++ lib/internal/repl/history-utils.js | 77 ++++++++ lib/internal/repl/history.js | 24 ++- lib/repl.js | 9 + test/parallel/test-repl-history-navigation.js | 63 ++++-- test/parallel/test-repl-history-utils.js | 187 ++++++++++++++++++ 6 files changed, 443 insertions(+), 14 deletions(-) create mode 100644 benchmark/repl/parse-history-from-file.js create mode 100644 lib/internal/repl/history-utils.js create mode 100644 test/parallel/test-repl-history-utils.js diff --git a/benchmark/repl/parse-history-from-file.js b/benchmark/repl/parse-history-from-file.js new file mode 100644 index 00000000000000..999451e7cbf512 --- /dev/null +++ b/benchmark/repl/parse-history-from-file.js @@ -0,0 +1,97 @@ +'use strict'; + +const common = require('../common.js'); + +const bench = common.createBenchmark(main, { + n: [1e1, 1e2], + size: [1e3, 1e4], +}, { + flags: ['--expose-internals'], +}); + +const bechText = ` +a ={ +a: 1, +b: 2, +} + +\`const a ={ + a: 1, + b: 2, + c: 3, + d: [1, 2, 3], +}\` + +\`const b = [ + 1, + 2, + 3, + 4, +]\` + +const d = [ + { + a: 1, + b: 2, + }, + { + a: 3, + b: 4, + } +] + +var e = [ + { + a: 1, + b: 2, + }, + { + a: 3, + b: 4, + c: [ + { + a: 1, + b: 2, + }, + { + a: 3, + b: 4, + } + ] + } +] + +a = \` +I am a multiline string +I can be as long as I want\` + + +b = \` +111111111111 +222222222222\` + + +c = \` +111111=11111 +222222222222\` + +function sum(a, b) { + return a + b +} + +console.log( + 'something' +) +`; + +async function main({ n, size }) { + const { parseHistoryFromFile } = require('internal/repl/history-utils'); + + bench.start(); + + for (let i = 0; i < n; i++) { + parseHistoryFromFile(bechText.repeat(size)); + } + + bench.end(n); +} diff --git a/lib/internal/repl/history-utils.js b/lib/internal/repl/history-utils.js new file mode 100644 index 00000000000000..c52988dad42105 --- /dev/null +++ b/lib/internal/repl/history-utils.js @@ -0,0 +1,77 @@ +'use strict'; + +const { + Array, + ArrayPrototypeJoin, + ArrayPrototypeReverse, + StringPrototypeCharCodeAt, + StringPrototypeTrimEnd, +} = primordials; +const os = require('os'); + +const kBrackets = { '{': 1, '[': 1, '(': 1, '}': -1, ']': -1, ')': -1 }; + +function parseHistoryFromFile(historyText, historySize) { + const lines = historyText.trimEnd().split(os.EOL); + let linesLength = lines.length; + if (linesLength > historySize) linesLength = historySize; + + const commands = new Array(linesLength); + let commandsIndex = 0; + const currentCommand = new Array(linesLength); + let currentCommandIndex = 0; + let bracketCount = 0; + let inString = false; + let stringDelimiter = ''; + + for (let lineIdx = linesLength - 1; lineIdx >= 0; lineIdx--) { + const line = lines[lineIdx]; + currentCommand[currentCommandIndex++] = line; + + let isConcatenation = false; + let isArrowFunction = false; + let lastChar = ''; + + for (let charIdx = 0, len = line.length; charIdx < len; charIdx++) { + const char = line[charIdx]; + + if ((char === "'" || char === '"' || char === '`') && + (charIdx === 0 || StringPrototypeCharCodeAt(line, charIdx - 1) !== 92)) { // 92 is '\\' + if (!inString) { + inString = true; + stringDelimiter = char; + } else if (char === stringDelimiter) { + inString = false; + } + } + + if (!inString) { + const bracketValue = kBrackets[char]; + if (bracketValue) bracketCount += bracketValue; + } + + lastChar = char; + } + + if (!inString) { + const trimmedLine = StringPrototypeTrimEnd(line); + isConcatenation = lastChar === '+'; + isArrowFunction = lastChar === '>' && trimmedLine[trimmedLine.length - 2] === '='; + } + + if (!inString && bracketCount <= 0 && !isConcatenation && !isArrowFunction) { + commands[commandsIndex++] = ArrayPrototypeJoin(currentCommand.slice(0, currentCommandIndex), '\n'); + currentCommandIndex = 0; + bracketCount = 0; + } + } + + if (currentCommandIndex > 0) { + commands[commandsIndex++] = ArrayPrototypeJoin(currentCommand.slice(0, currentCommandIndex), '\n'); + } + + commands.length = commandsIndex; + return ArrayPrototypeReverse(commands); +} + +module.exports = { parseHistoryFromFile }; diff --git a/lib/internal/repl/history.js b/lib/internal/repl/history.js index d8c36170639fc9..84bcbbde212797 100644 --- a/lib/internal/repl/history.js +++ b/lib/internal/repl/history.js @@ -2,9 +2,10 @@ const { ArrayPrototypeJoin, + ArrayPrototypePush, Boolean, FunctionPrototype, - RegExpPrototypeSymbolSplit, + StringPrototypeSplit, StringPrototypeTrim, } = primordials; @@ -17,6 +18,7 @@ let debug = require('internal/util/debuglog').debuglog('repl', (fn) => { }); const permission = require('internal/process/permission'); const { clearTimeout, setTimeout } = require('timers'); +const { parseHistoryFromFile } = require('internal/repl/history-utils'); const noop = FunctionPrototype; @@ -97,7 +99,7 @@ function setupHistory(repl, historyPath, ready) { } if (data) { - repl.history = RegExpPrototypeSymbolSplit(/[\n\r]+/, data, repl.historySize); + repl.history = parseHistoryFromFile(data, repl.historySize); } else { repl.history = []; } @@ -134,6 +136,22 @@ function setupHistory(repl, historyPath, ready) { timer = setTimeout(flushHistory, kDebounceHistoryMS); } + function parseHistoryData() { + const eol = os.EOL; + const result = []; + const historyLength = repl.history.length; + + for (let i = 0; i < historyLength; i++) { + const entry = repl.history[i]; + const lines = StringPrototypeSplit(entry, eol); + for (let j = lines.length - 1; j >= 0; j--) { + ArrayPrototypePush(result, lines[j]); + } + } + + return ArrayPrototypeJoin(result, eol) + eol; + } + function flushHistory() { timer = null; if (writing) { @@ -141,7 +159,7 @@ function setupHistory(repl, historyPath, ready) { return; } writing = true; - const historyData = ArrayPrototypeJoin(repl.history, os.EOL); + const historyData = parseHistoryData(); fs.write(repl._historyHandle, historyData, 0, 'utf8', onwritten); } diff --git a/lib/repl.js b/lib/repl.js index 6cfceacac620f0..e257e5cddac998 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -117,6 +117,7 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const vm = require('vm'); +const os = require('os'); const { runInThisContext, runInContext } = vm.Script.prototype; @@ -952,6 +953,14 @@ function REPLServer(prompt, self._domain.emit('error', e.err || e); } + if (self[kBufferedCommandSymbol]) { + // Remove the first N lines from the self.history array + // where N is the number of lines in the buffered command + const lines = StringPrototypeSplit(self[kBufferedCommandSymbol], os.EOL); + self.history = ArrayPrototypeSlice(self.history, lines.length); + // And replace them with the single command divided into lines + ArrayPrototypeUnshift(self.history, self[kBufferedCommandSymbol] + cmd); + } // Clear buffer if no SyntaxErrors self.clearBufferedCommand(); sawCtrlD = false; diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 64317be960e8d1..beef072b784bec 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -236,17 +236,22 @@ const tests = [ // K = Erase in line; 0 = right; 1 = left; 2 = total expected: [ // 0. Start - '\x1B[1G', '\x1B[0J', - prompt, '\x1B[3G', - // 1. UP - // This exceeds the maximum columns (250): - // Whitespace + prompt + ' // '.length + 'autocompleteMe'.length - // 230 + 2 + 4 + 14 - '\x1B[1G', '\x1B[0J', - `${prompt}${' '.repeat(230)} aut`, '\x1B[237G', - ' // ocompleteMe', '\x1B[237G', - '\n// 123', '\x1B[237G', - '\x1B[1A', '\x1B[1B', '\x1B[2K', '\x1B[1A', + '\x1B[1G', + '\x1B[0J', + `${prompt}`, + '\x1B[3G', + '\x1B[1G', + '\x1B[0J', + `${prompt}aut`, + '\x1B[6G', + ' // ocompleteMe', + '\x1B[6G', + '\n// 123', + '\x1B[6G', + '\x1B[1A', + '\x1B[1B', + '\x1B[2K', + '\x1B[1A', '\x1B[0K', // 2. UP '\x1B[1G', '\x1B[0J', @@ -633,6 +638,42 @@ const tests = [ ], clean: false }, + { + // Test that the multiline history is correctly navigated and it can be edited + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + skip: !process.features.inspector, + test: [ + 'let a = ``', + ENTER, + 'a = `I am a multiline strong', + ENTER, + 'which ends here`', + ENTER, + UP, + // press LEFT 19 times to reach the typo + ...Array(19).fill(LEFT), + BACKSPACE, + 'i', + ENTER, + ], + expected: [ + prompt, ...'let a = ``', + 'undefined\n', + prompt, ...'a = `I am a multiline strong', + '... ', + ...'which ends here`', + "'I am a multiline strong\\nwhich ends here'\n", + prompt, + `${prompt}a = \`I am a multiline strong\nwhich ends here\``, + `${prompt}a = \`I am a multiline strong\nwhich ends here\``, + `${prompt}a = \`I am a multiline strng\nwhich ends here\``, + `${prompt}a = \`I am a multiline string\nwhich ends here\``, + `${prompt}a = \`I am a multiline string\nwhich ends here\``, + "'I am a multiline string\\nwhich ends here'\n", + prompt, + ], + clean: true + }, ]; const numtests = tests.length; diff --git a/test/parallel/test-repl-history-utils.js b/test/parallel/test-repl-history-utils.js new file mode 100644 index 00000000000000..4bd74eae4f6f06 --- /dev/null +++ b/test/parallel/test-repl-history-utils.js @@ -0,0 +1,187 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); + +const assert = require('assert'); +const { parseHistoryFromFile } = require('internal/repl/history-utils'); + +function reverseString(str) { + return str.split('\n').reverse().join('\n'); +} + +{ + // It properly loads a single command + const historyFile = "console.log('lello')"; + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, ['console.log(\'lello\')']); +} + +{ + // It properly loads a single command on a multiline + const historyFile = reverseString("console.log(\n'lello'\n)"); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, ['console.log(\n\'lello\'\n)']); +} + +for (const declarator of ['const', 'let', 'var']) { + { + // It properly loads a multiline string + const historyFile = reverseString(`${declarator} a = \` +I am a multiline string +With some very interesting content\``); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ + `${declarator} a = \`\nI am a multiline string\nWith some very interesting content\``, + ]); + } + + { + // It properly loads a multiline string with fat arrow and plus operator endings + const historyFile = reverseString(`${declarator} a = \` +I am a multiline string + +With some very interesting content => +With special chars here and there\``); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ + `${declarator} a = \`\n` + + 'I am a multiline string +\n' + + 'With some very interesting content =>\n' + + 'With special chars here and there`', + ]); + } + + { + // It properly loads a multiline string concatenated with the plus operator + const historyFile = reverseString(`${declarator} a = "hello" + +'world'`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ + `${declarator} a = "hello" +\n'world'`, + ]); + } + + { + // It properly loads a multiline object + const historyFile = reverseString(` +${declarator} a = { +a: 1, +b: 2, +c: 3 +}`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ `${declarator} a = {\na: 1,\nb: 2,\nc: 3\n}` ]); + } + + { + // It properly loads a multiline stringified object + const historyFile = reverseString(`\`${declarator} a ={ + a: 1, + b: 2, + c: 3, + d: [1, 2, 3], + }\``); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ `\`${declarator} a ={\n a: 1,\n b: 2,\n c: 3,\n d: [1, 2, 3],\n }\`` ]); + } + + { + // It properly loads a multiline stringified array + const historyFile = reverseString(`\`${declarator} b = [ + 1, + 2, + 3, + 4, + ]\``); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ `\`${declarator} b = [\n 1,\n 2,\n 3,\n 4,\n ]\`` ]); + } + + { + // It properly loads a multiline array + const historyFile = reverseString(`${declarator} a = [ + 1, + 2, + 3, + ]`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ `${declarator} a = [\n 1,\n 2,\n 3,\n ]` ]); + } + + { + // It properly loads a multiline complex array of objects + const historyFile = reverseString(`\`${declarator} c = [ + { + a: 1, + b: 2, + }, + { + a: 3, + b: 4, + c: [ + 1, + 2, + 3, + ], + } + ]\``); + + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ + `\`${declarator} c = [\n` + + ' {\n' + + ' a: 1,\n' + + ' b: 2,\n' + + ' },\n' + + ' {\n' + + ' a: 3,\n' + + ' b: 4,\n' + + ' c: [\n' + + ' 1,\n' + + ' 2,\n' + + ' 3,\n' + + ' ],\n' + + ' }\n' + + ' ]`', + ]); + } + + { + // It properly loads a multiline function + const historyFile = reverseString(`function sum(a, b) { + return a + b; + }`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ 'function sum(a, b) {\n return a + b;\n }' ]); + } + + { + // It properly loads a multiline function as a fat arrow function + const historyFile = reverseString(`${declarator} sum = (a, b) => + a + b;`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ `${declarator} sum = (a, b) =>\n a + b;` ]); + } + + { + // It properly loads if statements + const historyFile = reverseString(`if (true) { + console.log('lello'); + }`); + const history = parseHistoryFromFile(historyFile); + + assert.deepStrictEqual(history, [ 'if (true) {\n console.log(\'lello\');\n }' ]); + } +} From 8b261ae4528d10c6dd976a810db53eab297aebb9 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Mon, 10 Mar 2025 14:48:56 +0100 Subject: [PATCH 02/13] repl: do not use the custom parser but split lines with \r --- benchmark/repl/parse-history-from-file.js | 97 --------- lib/internal/readline/interface.js | 5 +- lib/internal/repl/history-utils.js | 77 -------- lib/internal/repl/history.js | 24 +-- lib/repl.js | 19 +- test/fixtures/.node_repl_history-multiline | 4 + test/parallel/test-repl-history-navigation.js | 27 ++- test/parallel/test-repl-history-utils.js | 187 ------------------ .../test-repl-load-multiline-from-history.js | 90 +++++++++ 9 files changed, 125 insertions(+), 405 deletions(-) delete mode 100644 benchmark/repl/parse-history-from-file.js delete mode 100644 lib/internal/repl/history-utils.js create mode 100644 test/fixtures/.node_repl_history-multiline delete mode 100644 test/parallel/test-repl-history-utils.js create mode 100644 test/parallel/test-repl-load-multiline-from-history.js diff --git a/benchmark/repl/parse-history-from-file.js b/benchmark/repl/parse-history-from-file.js deleted file mode 100644 index 999451e7cbf512..00000000000000 --- a/benchmark/repl/parse-history-from-file.js +++ /dev/null @@ -1,97 +0,0 @@ -'use strict'; - -const common = require('../common.js'); - -const bench = common.createBenchmark(main, { - n: [1e1, 1e2], - size: [1e3, 1e4], -}, { - flags: ['--expose-internals'], -}); - -const bechText = ` -a ={ -a: 1, -b: 2, -} - -\`const a ={ - a: 1, - b: 2, - c: 3, - d: [1, 2, 3], -}\` - -\`const b = [ - 1, - 2, - 3, - 4, -]\` - -const d = [ - { - a: 1, - b: 2, - }, - { - a: 3, - b: 4, - } -] - -var e = [ - { - a: 1, - b: 2, - }, - { - a: 3, - b: 4, - c: [ - { - a: 1, - b: 2, - }, - { - a: 3, - b: 4, - } - ] - } -] - -a = \` -I am a multiline string -I can be as long as I want\` - - -b = \` -111111111111 -222222222222\` - - -c = \` -111111=11111 -222222222222\` - -function sum(a, b) { - return a + b -} - -console.log( - 'something' -) -`; - -async function main({ n, size }) { - const { parseHistoryFromFile } = require('internal/repl/history-utils'); - - bench.start(); - - for (let i = 0; i < n; i++) { - parseHistoryFromFile(bechText.repeat(size)); - } - - bench.end(n); -} diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index c831c79756f343..9599b728048f8b 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -25,6 +25,7 @@ const { StringPrototypeCodePointAt, StringPrototypeEndsWith, StringPrototypeRepeat, + StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeStartsWith, StringPrototypeTrim, @@ -950,7 +951,7 @@ class Interface extends InterfaceConstructor { if (index === -1) { this.line = search; } else { - this.line = this.history[index]; + this.line = StringPrototypeReplaceAll(this.history[index], '\r', '\n'); } this.historyIndex = index; this.cursor = this.line.length; // Set cursor to end of line. @@ -973,7 +974,7 @@ class Interface extends InterfaceConstructor { if (index === this.history.length) { this.line = search; } else { - this.line = this.history[index]; + this.line = StringPrototypeReplaceAll(this.history[index], '\r', '\n'); } this.historyIndex = index; this.cursor = this.line.length; // Set cursor to end of line. diff --git a/lib/internal/repl/history-utils.js b/lib/internal/repl/history-utils.js deleted file mode 100644 index c52988dad42105..00000000000000 --- a/lib/internal/repl/history-utils.js +++ /dev/null @@ -1,77 +0,0 @@ -'use strict'; - -const { - Array, - ArrayPrototypeJoin, - ArrayPrototypeReverse, - StringPrototypeCharCodeAt, - StringPrototypeTrimEnd, -} = primordials; -const os = require('os'); - -const kBrackets = { '{': 1, '[': 1, '(': 1, '}': -1, ']': -1, ')': -1 }; - -function parseHistoryFromFile(historyText, historySize) { - const lines = historyText.trimEnd().split(os.EOL); - let linesLength = lines.length; - if (linesLength > historySize) linesLength = historySize; - - const commands = new Array(linesLength); - let commandsIndex = 0; - const currentCommand = new Array(linesLength); - let currentCommandIndex = 0; - let bracketCount = 0; - let inString = false; - let stringDelimiter = ''; - - for (let lineIdx = linesLength - 1; lineIdx >= 0; lineIdx--) { - const line = lines[lineIdx]; - currentCommand[currentCommandIndex++] = line; - - let isConcatenation = false; - let isArrowFunction = false; - let lastChar = ''; - - for (let charIdx = 0, len = line.length; charIdx < len; charIdx++) { - const char = line[charIdx]; - - if ((char === "'" || char === '"' || char === '`') && - (charIdx === 0 || StringPrototypeCharCodeAt(line, charIdx - 1) !== 92)) { // 92 is '\\' - if (!inString) { - inString = true; - stringDelimiter = char; - } else if (char === stringDelimiter) { - inString = false; - } - } - - if (!inString) { - const bracketValue = kBrackets[char]; - if (bracketValue) bracketCount += bracketValue; - } - - lastChar = char; - } - - if (!inString) { - const trimmedLine = StringPrototypeTrimEnd(line); - isConcatenation = lastChar === '+'; - isArrowFunction = lastChar === '>' && trimmedLine[trimmedLine.length - 2] === '='; - } - - if (!inString && bracketCount <= 0 && !isConcatenation && !isArrowFunction) { - commands[commandsIndex++] = ArrayPrototypeJoin(currentCommand.slice(0, currentCommandIndex), '\n'); - currentCommandIndex = 0; - bracketCount = 0; - } - } - - if (currentCommandIndex > 0) { - commands[commandsIndex++] = ArrayPrototypeJoin(currentCommand.slice(0, currentCommandIndex), '\n'); - } - - commands.length = commandsIndex; - return ArrayPrototypeReverse(commands); -} - -module.exports = { parseHistoryFromFile }; diff --git a/lib/internal/repl/history.js b/lib/internal/repl/history.js index 84bcbbde212797..b6b596939683cd 100644 --- a/lib/internal/repl/history.js +++ b/lib/internal/repl/history.js @@ -2,10 +2,9 @@ const { ArrayPrototypeJoin, - ArrayPrototypePush, Boolean, FunctionPrototype, - StringPrototypeSplit, + RegExpPrototypeSymbolSplit, StringPrototypeTrim, } = primordials; @@ -18,7 +17,6 @@ let debug = require('internal/util/debuglog').debuglog('repl', (fn) => { }); const permission = require('internal/process/permission'); const { clearTimeout, setTimeout } = require('timers'); -const { parseHistoryFromFile } = require('internal/repl/history-utils'); const noop = FunctionPrototype; @@ -99,7 +97,7 @@ function setupHistory(repl, historyPath, ready) { } if (data) { - repl.history = parseHistoryFromFile(data, repl.historySize); + repl.history = RegExpPrototypeSymbolSplit(/[\n]+/, data, repl.historySize); } else { repl.history = []; } @@ -136,22 +134,6 @@ function setupHistory(repl, historyPath, ready) { timer = setTimeout(flushHistory, kDebounceHistoryMS); } - function parseHistoryData() { - const eol = os.EOL; - const result = []; - const historyLength = repl.history.length; - - for (let i = 0; i < historyLength; i++) { - const entry = repl.history[i]; - const lines = StringPrototypeSplit(entry, eol); - for (let j = lines.length - 1; j >= 0; j--) { - ArrayPrototypePush(result, lines[j]); - } - } - - return ArrayPrototypeJoin(result, eol) + eol; - } - function flushHistory() { timer = null; if (writing) { @@ -159,7 +141,7 @@ function setupHistory(repl, historyPath, ready) { return; } writing = true; - const historyData = parseHistoryData(); + const historyData = ArrayPrototypeJoin(repl.history, os.EOL); fs.write(repl._historyHandle, historyData, 0, 'utf8', onwritten); } diff --git a/lib/repl.js b/lib/repl.js index e257e5cddac998..c638d6b90c54fe 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -83,7 +83,9 @@ const { StringPrototypeCodePointAt, StringPrototypeEndsWith, StringPrototypeIncludes, + StringPrototypeIndexOf, StringPrototypeRepeat, + StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeSplit, StringPrototypeStartsWith, @@ -117,7 +119,6 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const vm = require('vm'); -const os = require('os'); const { runInThisContext, runInContext } = vm.Script.prototype; @@ -953,14 +954,22 @@ function REPLServer(prompt, self._domain.emit('error', e.err || e); } - if (self[kBufferedCommandSymbol]) { + // In the next two if blocks, we do not use os.EOL instead of '\n' + // because on Windows it is '\r\n' + if (cmd && StringPrototypeIndexOf(cmd, '\n') !== -1) { // If you are editing a multiline command + self.history[0] = StringPrototypeReplaceAll(cmd, '\n', '\r'); + } else if (self[kBufferedCommandSymbol]) { // If a new multiline command was entered // Remove the first N lines from the self.history array // where N is the number of lines in the buffered command - const lines = StringPrototypeSplit(self[kBufferedCommandSymbol], os.EOL); + + const lines = StringPrototypeSplit(self[kBufferedCommandSymbol], '\n'); self.history = ArrayPrototypeSlice(self.history, lines.length); - // And replace them with the single command divided into lines - ArrayPrototypeUnshift(self.history, self[kBufferedCommandSymbol] + cmd); + ArrayPrototypePop(lines); + // And replace them with the single command split by '\r' + ArrayPrototypePush(lines, cmd); + ArrayPrototypeUnshift(self.history, ArrayPrototypeJoin(lines, '\r')); } + // Clear buffer if no SyntaxErrors self.clearBufferedCommand(); sawCtrlD = false; diff --git a/test/fixtures/.node_repl_history-multiline b/test/fixtures/.node_repl_history-multiline new file mode 100644 index 00000000000000..c7e45f0154fdb4 --- /dev/null +++ b/test/fixtures/.node_repl_history-multiline @@ -0,0 +1,4 @@ +var d = [ { a: 1, b: 2, }, { a: 3, b: 4, c: [{ a: 1, b: 2 }, { a: 3, b: 4, } ] } ] +const c = [ { a: 1, b: 2, } ] +`const b = [ 1, 2, 3, 4, ]` +a = ` I am a multiline string I can be as long as I want` \ No newline at end of file diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index beef072b784bec..88493beb3f9ae3 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -236,22 +236,17 @@ const tests = [ // K = Erase in line; 0 = right; 1 = left; 2 = total expected: [ // 0. Start - '\x1B[1G', - '\x1B[0J', - `${prompt}`, - '\x1B[3G', - '\x1B[1G', - '\x1B[0J', - `${prompt}aut`, - '\x1B[6G', - ' // ocompleteMe', - '\x1B[6G', - '\n// 123', - '\x1B[6G', - '\x1B[1A', - '\x1B[1B', - '\x1B[2K', - '\x1B[1A', + '\x1B[1G', '\x1B[0J', + prompt, '\x1B[3G', + // 1. UP + // This exceeds the maximum columns (250): + // Whitespace + prompt + ' // '.length + 'autocompleteMe'.length + // 230 + 2 + 4 + 14 + '\x1B[1G', '\x1B[0J', + `${prompt}${' '.repeat(230)} aut`, '\x1B[237G', + ' // ocompleteMe', '\x1B[237G', + '\n// 123', '\x1B[237G', + '\x1B[1A', '\x1B[1B', '\x1B[2K', '\x1B[1A', '\x1B[0K', // 2. UP '\x1B[1G', '\x1B[0J', diff --git a/test/parallel/test-repl-history-utils.js b/test/parallel/test-repl-history-utils.js deleted file mode 100644 index 4bd74eae4f6f06..00000000000000 --- a/test/parallel/test-repl-history-utils.js +++ /dev/null @@ -1,187 +0,0 @@ -// Flags: --expose-internals -'use strict'; - -require('../common'); - -const assert = require('assert'); -const { parseHistoryFromFile } = require('internal/repl/history-utils'); - -function reverseString(str) { - return str.split('\n').reverse().join('\n'); -} - -{ - // It properly loads a single command - const historyFile = "console.log('lello')"; - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, ['console.log(\'lello\')']); -} - -{ - // It properly loads a single command on a multiline - const historyFile = reverseString("console.log(\n'lello'\n)"); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, ['console.log(\n\'lello\'\n)']); -} - -for (const declarator of ['const', 'let', 'var']) { - { - // It properly loads a multiline string - const historyFile = reverseString(`${declarator} a = \` -I am a multiline string -With some very interesting content\``); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ - `${declarator} a = \`\nI am a multiline string\nWith some very interesting content\``, - ]); - } - - { - // It properly loads a multiline string with fat arrow and plus operator endings - const historyFile = reverseString(`${declarator} a = \` -I am a multiline string + -With some very interesting content => -With special chars here and there\``); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ - `${declarator} a = \`\n` + - 'I am a multiline string +\n' + - 'With some very interesting content =>\n' + - 'With special chars here and there`', - ]); - } - - { - // It properly loads a multiline string concatenated with the plus operator - const historyFile = reverseString(`${declarator} a = "hello" + -'world'`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ - `${declarator} a = "hello" +\n'world'`, - ]); - } - - { - // It properly loads a multiline object - const historyFile = reverseString(` -${declarator} a = { -a: 1, -b: 2, -c: 3 -}`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ `${declarator} a = {\na: 1,\nb: 2,\nc: 3\n}` ]); - } - - { - // It properly loads a multiline stringified object - const historyFile = reverseString(`\`${declarator} a ={ - a: 1, - b: 2, - c: 3, - d: [1, 2, 3], - }\``); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ `\`${declarator} a ={\n a: 1,\n b: 2,\n c: 3,\n d: [1, 2, 3],\n }\`` ]); - } - - { - // It properly loads a multiline stringified array - const historyFile = reverseString(`\`${declarator} b = [ - 1, - 2, - 3, - 4, - ]\``); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ `\`${declarator} b = [\n 1,\n 2,\n 3,\n 4,\n ]\`` ]); - } - - { - // It properly loads a multiline array - const historyFile = reverseString(`${declarator} a = [ - 1, - 2, - 3, - ]`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ `${declarator} a = [\n 1,\n 2,\n 3,\n ]` ]); - } - - { - // It properly loads a multiline complex array of objects - const historyFile = reverseString(`\`${declarator} c = [ - { - a: 1, - b: 2, - }, - { - a: 3, - b: 4, - c: [ - 1, - 2, - 3, - ], - } - ]\``); - - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ - `\`${declarator} c = [\n` + - ' {\n' + - ' a: 1,\n' + - ' b: 2,\n' + - ' },\n' + - ' {\n' + - ' a: 3,\n' + - ' b: 4,\n' + - ' c: [\n' + - ' 1,\n' + - ' 2,\n' + - ' 3,\n' + - ' ],\n' + - ' }\n' + - ' ]`', - ]); - } - - { - // It properly loads a multiline function - const historyFile = reverseString(`function sum(a, b) { - return a + b; - }`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ 'function sum(a, b) {\n return a + b;\n }' ]); - } - - { - // It properly loads a multiline function as a fat arrow function - const historyFile = reverseString(`${declarator} sum = (a, b) => - a + b;`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ `${declarator} sum = (a, b) =>\n a + b;` ]); - } - - { - // It properly loads if statements - const historyFile = reverseString(`if (true) { - console.log('lello'); - }`); - const history = parseHistoryFromFile(historyFile); - - assert.deepStrictEqual(history, [ 'if (true) {\n console.log(\'lello\');\n }' ]); - } -} diff --git a/test/parallel/test-repl-load-multiline-from-history.js b/test/parallel/test-repl-load-multiline-from-history.js new file mode 100644 index 00000000000000..dc806d047ab16c --- /dev/null +++ b/test/parallel/test-repl-load-multiline-from-history.js @@ -0,0 +1,90 @@ +'use strict'; + +// Flags: --expose-internals + +const common = require('../common'); + +const assert = require('assert'); +const repl = require('internal/repl'); +const stream = require('stream'); +const fixtures = require('../common/fixtures'); + +class ActionStream extends stream.Stream { + run(data) { + const _iter = data[Symbol.iterator](); + const doAction = () => { + const next = _iter.next(); + if (next.done) { + // Close the repl. Note that it must have a clean prompt to do so. + this.emit('keypress', '', { ctrl: true, name: 'd' }); + return; + } + const action = next.value; + + if (typeof action === 'object') { + this.emit('keypress', '', action); + } + setImmediate(doAction); + }; + doAction(); + } + resume() {} + pause() {} +} +ActionStream.prototype.readable = true; + +{ + // Testing multiline history loading + const tmpdir = require('../common/tmpdir'); + tmpdir.refresh(); + const replHistoryPath = fixtures.path('.node_repl_history-multiline'); + + const checkResults = common.mustSucceed((r) => { + assert.strictEqual(r.history.length, 4); + + r.input.run([{ name: 'up' }]); + assert.strictEqual( + r.line, + 'var d = [\n' + + ' {\n' + + ' a: 1,\n' + + ' b: 2,\n' + + ' },\n' + + ' {\n' + + ' a: 3,\n' + + ' b: 4,\n' + + ' c: [{ a: 1, b: 2 },\n' + + ' {\n' + + ' a: 3,\n' + + ' b: 4,\n' + + ' }\n' + + ' ]\n' + + ' }\n' + + ']' + ); + + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.line, 'const c = [\n {\n a: 1,\n b: 2,\n }\n]'); + + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.line, '`const b = [\n 1,\n 2,\n 3,\n 4,\n]`'); + + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.line, 'a = `\nI am a multiline string\nI can be as long as I want`'); + + }); + + repl.createInternalRepl( + { NODE_REPL_HISTORY: replHistoryPath }, + { + terminal: true, + input: new ActionStream(), + output: new stream.Writable({ + write(chunk, _, next) { + next(); + } + }), + }, + checkResults + ); +} From 306ee84635bc602b6148dc07e21644efa670925a Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 18 Mar 2025 10:18:30 +0100 Subject: [PATCH 03/13] repl: replace ... with | and navigate multiline history with up / down --- lib/internal/readline/interface.js | 55 +++++++++++- lib/repl.js | 2 +- test/parallel/test-repl-history-navigation.js | 53 +++++++++++- .../test-repl-load-multiline-from-history.js | 12 ++- test/parallel/test-repl-multiline.js | 2 +- test/parallel/test-repl-recoverable.js | 2 +- test/parallel/test-repl-top-level-await.js | 20 ++--- .../test-repl-unexpected-token-recoverable.js | 2 +- test/parallel/test-repl.js | 84 +++++++++---------- 9 files changed, 168 insertions(+), 64 deletions(-) diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 9599b728048f8b..eec9a25fd94e1f 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -24,9 +24,11 @@ const { SafeStringIterator, StringPrototypeCodePointAt, StringPrototypeEndsWith, + StringPrototypeIndexOf, StringPrototypeRepeat, StringPrototypeReplaceAll, StringPrototypeSlice, + StringPrototypeSplit, StringPrototypeStartsWith, StringPrototypeTrim, Symbol, @@ -100,7 +102,9 @@ const kDeleteWordLeft = Symbol('_deleteWordLeft'); const kDeleteWordRight = Symbol('_deleteWordRight'); const kGetDisplayPos = Symbol('_getDisplayPos'); const kHistoryNext = Symbol('_historyNext'); +const kMoveDownOrHistoryNext = Symbol('_moveDownOrHistoryNext'); const kHistoryPrev = Symbol('_historyPrev'); +const kMoveUpOrHistoryPrev = Symbol('_moveUpOrHistoryPrev'); const kInsertString = Symbol('_insertString'); const kLine = Symbol('_line'); const kLine_buffer = Symbol('_line_buffer'); @@ -929,6 +933,26 @@ class Interface extends InterfaceConstructor { this[kRefreshLine](); } + [kMoveDownOrHistoryNext]() { + const { cols, rows } = this.getCursorPos(); + const splitLine = StringPrototypeSplit(this.line, '\n'); + // Go to the next history only if the cursor is in the first line of the multiline input. + // Otherwise treat the "arrow down" as a movement to the next row. + if (splitLine.length > 1 && rows < splitLine.length - 1) { + const currentLine = splitLine[rows]; + const nextLine = splitLine[rows + 1]; + const amountToMove = (cols > nextLine.length) ? + +cols + nextLine.length - 1 : + +currentLine.length + 1; + // Go to the same position on the next line, or the end of the next line + // If the current position does not exist in the next line. + this[kMoveCursor](amountToMove); + return; + } + + this[kHistoryNext](); + } + // TODO(BridgeAR): Add underscores to the search part and a red background in // case no match is found. This should only be the visual part and not the // actual line content! @@ -939,7 +963,9 @@ class Interface extends InterfaceConstructor { [kHistoryNext]() { if (this.historyIndex >= 0) { this[kBeforeEdit](this.line, this.cursor); - const search = this[kSubstringSearch] || ''; + const isLineMultiline = StringPrototypeIndexOf(this.line, '\n') !== -1; + + const search = isLineMultiline ? '' : this[kSubstringSearch] || ''; let index = this.historyIndex - 1; while ( index >= 0 && @@ -959,10 +985,31 @@ class Interface extends InterfaceConstructor { } } + [kMoveUpOrHistoryPrev]() { + const { cols, rows } = this.getCursorPos(); + const splitLine = StringPrototypeSplit(this.line, '\n'); + // Go to the previous history only if the cursor is in the first line of the multiline input. + // Otherwise treat the "arrow up" as a movement to the previous row. + if (splitLine.length > 1 && rows > 0) { + const previousLine = splitLine[rows - 1]; + const amountToMove = (cols > previousLine.length) ? + -cols - 1 : + -previousLine.length - 1; + // Go to the same position on the previous line, or the end of the previous line + // If the current position does not exist in the previous line. + this[kMoveCursor](amountToMove); + return; + } + + this[kHistoryPrev](); + } + [kHistoryPrev]() { if (this.historyIndex < this.history.length && this.history.length) { this[kBeforeEdit](this.line, this.cursor); - const search = this[kSubstringSearch] || ''; + const isLineMultiline = StringPrototypeIndexOf(this.line, '\n') !== -1; + + const search = isLineMultiline ? '' : this[kSubstringSearch] || ''; let index = this.historyIndex + 1; while ( index < this.history.length && @@ -1309,11 +1356,11 @@ class Interface extends InterfaceConstructor { break; case 'up': - this[kHistoryPrev](); + this[kMoveUpOrHistoryPrev](); break; case 'down': - this[kHistoryNext](); + this[kMoveDownOrHistoryNext](); break; case 'tab': diff --git a/lib/repl.js b/lib/repl.js index c638d6b90c54fe..89a0f83d10b911 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1207,7 +1207,7 @@ REPLServer.prototype.resetContext = function() { REPLServer.prototype.displayPrompt = function(preserveCursor) { let prompt = this._initialPrompt; if (this[kBufferedCommandSymbol].length) { - prompt = '...'; + prompt = '|'; const len = this.lines.level.length ? this.lines.level.length - 1 : 0; const levelInd = StringPrototypeRepeat('..', len); prompt += levelInd + ' '; diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 88493beb3f9ae3..3116bc027ceae3 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -655,7 +655,7 @@ const tests = [ prompt, ...'let a = ``', 'undefined\n', prompt, ...'a = `I am a multiline strong', - '... ', + '| ', ...'which ends here`', "'I am a multiline strong\\nwhich ends here'\n", prompt, @@ -669,6 +669,57 @@ const tests = [ ], clean: true }, + { + // Test that the previous multiline history can only be accessed going through the entirety of the current + // One navigating its all lines first. + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + skip: !process.features.inspector, + test: [ + 'let b = ``', + ENTER, + 'b = `I am a multiline strong', + ENTER, + 'which ends here`', + ENTER, + 'let c = `I', + ENTER, + 'am another one`', + ENTER, + UP, + UP, + UP, + UP, + // press RIGHT 10 times to reach the typo + ...Array(10).fill(RIGHT), + BACKSPACE, + 'i', + ENTER, + ], + expected: [ + prompt, ...'let b = ``', + 'undefined\n', + prompt, ...'b = `I am a multiline strong', + '| ', + ...'which ends here`', + "'I am a multiline strong\\nwhich ends here'\n", + prompt, ...'let c = `I', + '| ', + ...'am another one`', + 'undefined\n', + prompt, + `${prompt}let c = \`I\nam another one\``, + `${prompt}let c = \`I\nam another one\``, + + `${prompt}b = \`I am a multiline strong\nwhich ends here\``, + `${prompt}b = \`I am a multiline strong\nwhich ends here\``, + `${prompt}b = \`I am a multiline strng\nwhich ends here\``, + `${prompt}b = \`I am a multiline string\nwhich ends here\``, + `${prompt}b = \`I am a multiline string\nwhich ends here\``, + "'I am a multiline string\\nwhich ends here'\n", + prompt, + ], + clean: true + }, ]; const numtests = tests.length; diff --git a/test/parallel/test-repl-load-multiline-from-history.js b/test/parallel/test-repl-load-multiline-from-history.js index dc806d047ab16c..0998d664fa0bd1 100644 --- a/test/parallel/test-repl-load-multiline-from-history.js +++ b/test/parallel/test-repl-load-multiline-from-history.js @@ -63,13 +63,19 @@ ActionStream.prototype.readable = true; ']' ); - r.input.run([{ name: 'up' }]); + for (let i = 0; i < r.line.split('\n').length; i++) { + r.input.run([{ name: 'up' }]); + } assert.strictEqual(r.line, 'const c = [\n {\n a: 1,\n b: 2,\n }\n]'); - r.input.run([{ name: 'up' }]); + for (let i = 0; i < r.line.split('\n').length; i++) { + r.input.run([{ name: 'up' }]); + } assert.strictEqual(r.line, '`const b = [\n 1,\n 2,\n 3,\n 4,\n]`'); - r.input.run([{ name: 'up' }]); + for (let i = 0; i < r.line.split('\n').length; i++) { + r.input.run([{ name: 'up' }]); + } assert.strictEqual(r.line, 'a = `\nI am a multiline string\nI can be as long as I want`'); }); diff --git a/test/parallel/test-repl-multiline.js b/test/parallel/test-repl-multiline.js index e458555cd87f76..0a2f95668a1683 100644 --- a/test/parallel/test-repl-multiline.js +++ b/test/parallel/test-repl-multiline.js @@ -26,7 +26,7 @@ function run({ useColors }) { // Validate the output, which contains terminal escape codes. assert.strictEqual(actual.length, 6); assert.ok(actual[0].endsWith(input[0])); - assert.ok(actual[1].includes('... ')); + assert.ok(actual[1].includes('| ')); assert.ok(actual[1].endsWith(input[1])); assert.ok(actual[2].includes('undefined')); assert.ok(actual[3].endsWith(input[2])); diff --git a/test/parallel/test-repl-recoverable.js b/test/parallel/test-repl-recoverable.js index a975dac782cfe4..74dcd5dfbf585d 100644 --- a/test/parallel/test-repl-recoverable.js +++ b/test/parallel/test-repl-recoverable.js @@ -18,7 +18,7 @@ function customEval(code, context, file, cb) { const putIn = new ArrayStream(); putIn.write = function(msg) { - if (msg === '... ') { + if (msg === '| ') { recovered = true; } diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js index c8bc26fad62e5c..e07b27948b93d8 100644 --- a/test/parallel/test-repl-top-level-await.js +++ b/test/parallel/test-repl-top-level-await.js @@ -96,7 +96,7 @@ async function ordinaryTests() { ['const m = foo(await koo());'], ['m', '4'], ['const n = foo(await\nkoo());', - ['const n = foo(await\r', '... koo());\r', 'undefined']], + ['const n = foo(await\r', '| koo());\r', 'undefined']], ['n', '4'], // eslint-disable-next-line no-template-curly-in-string ['`status: ${(await Promise.resolve({ status: 200 })).status}`', @@ -145,20 +145,20 @@ async function ordinaryTests() { ], ['for (const x of [1,2,3]) {\nawait x\n}', [ 'for (const x of [1,2,3]) {\r', - '... await x\r', - '... }\r', + '| await x\r', + '| }\r', 'undefined', ]], ['for (const x of [1,2,3]) {\nawait x;\n}', [ 'for (const x of [1,2,3]) {\r', - '... await x;\r', - '... }\r', + '| await x;\r', + '| }\r', 'undefined', ]], ['for await (const x of [1,2,3]) {\nconsole.log(x)\n}', [ 'for await (const x of [1,2,3]) {\r', - '... console.log(x)\r', - '... }\r', + '| console.log(x)\r', + '| }\r', '1', '2', '3', @@ -166,8 +166,8 @@ async function ordinaryTests() { ]], ['for await (const x of [1,2,3]) {\nconsole.log(x);\n}', [ 'for await (const x of [1,2,3]) {\r', - '... console.log(x);\r', - '... }\r', + '| console.log(x);\r', + '| }\r', '1', '2', '3', @@ -192,7 +192,7 @@ async function ordinaryTests() { } else if ('line' in options) { assert.strictEqual(lines[toBeRun.length + options.line], expected); } else { - const echoed = toBeRun.map((a, i) => `${i > 0 ? '... ' : ''}${a}\r`); + const echoed = toBeRun.map((a, i) => `${i > 0 ? '| ' : ''}${a}\r`); assert.deepStrictEqual(lines, [...echoed, expected, PROMPT]); } } diff --git a/test/parallel/test-repl-unexpected-token-recoverable.js b/test/parallel/test-repl-unexpected-token-recoverable.js index ddab589ddec8b9..2f3bd38d809303 100644 --- a/test/parallel/test-repl-unexpected-token-recoverable.js +++ b/test/parallel/test-repl-unexpected-token-recoverable.js @@ -12,7 +12,7 @@ const child = spawn(process.execPath, args); const input = 'const foo = "bar\\\nbaz"'; // Match '...' as well since it marks a multi-line statement -const expectOut = /> \.\.\. undefined\n/; +const expectOut = /> \| undefined\n/; child.stderr.setEncoding('utf8'); child.stderr.on('data', (d) => { diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 6c16d2a9679b21..9f14ebb39eeac7 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -69,7 +69,7 @@ async function runReplTests(socket, prompt, tests) { // Allow to match partial text if no newline was received, because // sending newlines from the REPL itself would be redundant - // (e.g. in the `... ` multiline prompt: The user already pressed + // (e.g. in the `| ` multiline prompt: The user already pressed // enter for that, so the REPL shouldn't do it again!). if (lineBuffer === expectedLine && !expectedLine.includes('\n')) lineBuffer += '\n'; @@ -154,7 +154,7 @@ const errorTests = [ // Common syntax error is treated as multiline command { send: 'function test_func() {', - expect: '... ' + expect: '| ' }, // You can recover with the .break command { @@ -169,7 +169,7 @@ const errorTests = [ // Can handle multiline template literals { send: '`io.js', - expect: '... ' + expect: '| ' }, // Special REPL commands still available { @@ -179,7 +179,7 @@ const errorTests = [ // Template expressions { send: '`io.js ${"1.0"', - expect: '... ' + expect: '| ' }, { send: '+ ".2"}`', @@ -187,7 +187,7 @@ const errorTests = [ }, { send: '`io.js ${', - expect: '... ' + expect: '| ' }, { send: '"1.0" + ".2"}`', @@ -196,7 +196,7 @@ const errorTests = [ // Dot prefix in multiline commands aren't treated as commands { send: '("a"', - expect: '... ' + expect: '| ' }, { send: '.charAt(0))', @@ -330,7 +330,7 @@ const errorTests = [ // Multiline object { send: '{ a: ', - expect: '... ' + expect: '| ' }, { send: '1 }', @@ -339,7 +339,7 @@ const errorTests = [ // Multiline string-keyed object (e.g. JSON) { send: '{ "a": ', - expect: '... ' + expect: '| ' }, { send: '1 }', @@ -348,12 +348,12 @@ const errorTests = [ // Multiline class with private member. { send: 'class Foo { #private = true ', - expect: '... ' + expect: '| ' }, // Class field with bigint. { send: 'num = 123456789n', - expect: '... ' + expect: '| ' }, // Static class features. { @@ -363,15 +363,15 @@ const errorTests = [ // Multiline anonymous function with comment { send: '(function() {', - expect: '... ' + expect: '| ' }, { send: '// blah', - expect: '... ' + expect: '| ' }, { send: 'return 1n;', - expect: '... ' + expect: '| ' }, { send: '})()', @@ -380,11 +380,11 @@ const errorTests = [ // Multiline function call { send: 'function f(){}; f(f(1,', - expect: '... ' + expect: '| ' }, { send: '2)', - expect: '... ' + expect: '| ' }, { send: ')', @@ -405,16 +405,16 @@ const errorTests = [ ...possibleTokensAfterIdentifierWithLineBreak.map((token) => ( { send: `npm ${token}; undefined`, - expect: '... undefined' + expect: '| undefined' } )), { send: '(function() {\n\nreturn 1;\n})()', - expect: '... ... ... 1' + expect: '| | | 1' }, { send: '{\n\na: 1\n}', - expect: '... ... ... { a: 1 }' + expect: '| | | { a: 1 }' }, { send: 'url.format("http://google.com")', @@ -449,7 +449,7 @@ const errorTests = [ // Do not fail when a String is created with line continuation { send: '\'the\\\nfourth\\\neye\'', - expect: ['... ... \'thefourtheye\''] + expect: ['| | \'thefourtheye\''] }, // Don't fail when a partial String is created and line continuation is used // with whitespace characters at the end of the string. We are to ignore it. @@ -462,17 +462,17 @@ const errorTests = [ // Multiline strings preserve whitespace characters in them { send: '\'the \\\n fourth\t\t\\\n eye \'', - expect: '... ... \'the fourth\\t\\t eye \'' + expect: '| | \'the fourth\\t\\t eye \'' }, // More than one multiline strings also should preserve whitespace chars { send: '\'the \\\n fourth\' + \'\t\t\\\n eye \'', - expect: '... ... \'the fourth\\t\\t eye \'' + expect: '| | \'the fourth\\t\\t eye \'' }, // using REPL commands within a string literal should still work { send: '\'\\\n.break', - expect: '... ' + prompt_unix + expect: '| ' + prompt_unix }, // Using REPL command "help" within a string literal should still work { @@ -519,7 +519,7 @@ const errorTests = [ // Empty lines in the string literals should not affect the string { send: '\'the\\\n\\\nfourtheye\'\n', - expect: '... ... \'thefourtheye\'' + expect: '| | \'thefourtheye\'' }, // Regression test for https://github.com/nodejs/node/issues/597 { @@ -536,32 +536,32 @@ const errorTests = [ // Regression tests for https://github.com/nodejs/node/issues/2749 { send: 'function x() {\nreturn \'\\n\';\n }', - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x() {\nreturn \'\\\\\';\n }', - expect: '... ... undefined' + expect: '| | undefined' }, // Regression tests for https://github.com/nodejs/node/issues/3421 { send: 'function x() {\n//\'\n }', - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x() {\n//"\n }', - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x() {//\'\n }', - expect: '... undefined' + expect: '| undefined' }, { send: 'function x() {//"\n }', - expect: '... undefined' + expect: '| undefined' }, { send: 'function x() {\nvar i = "\'";\n }', - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x(/*optional*/) {}', @@ -589,7 +589,7 @@ const errorTests = [ }, { send: '/* \'\n"\n\'"\'\n*/', - expect: '... ... ... undefined' + expect: '| | | undefined' }, // REPL should get a normal require() function, not one that allows // access to internal modules without the --expose-internals flag. @@ -614,19 +614,19 @@ const errorTests = [ // REPL should handle quotes within regexp literal in multiline mode { send: "function x(s) {\nreturn s.replace(/'/,'');\n}", - expect: '... ... undefined' + expect: '| | undefined' }, { send: "function x(s) {\nreturn s.replace(/'/,'');\n}", - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x(s) {\nreturn s.replace(/"/,"");\n}', - expect: '... ... undefined' + expect: '| | undefined' }, { send: 'function x(s) {\nreturn s.replace(/.*/,"");\n}', - expect: '... ... undefined' + expect: '| | undefined' }, { send: '{ var x = 4; }', @@ -697,17 +697,17 @@ const errorTests = [ // https://github.com/nodejs/node/issues/9300 { send: 'function foo() {\nvar bar = 1 / 1; // "/"\n}', - expect: '... ... undefined' + expect: '| | undefined' }, { send: '(function() {\nreturn /foo/ / /bar/;\n}())', - expect: '... ... NaN' + expect: '| | NaN' }, { send: '(function() {\nif (false) {} /bar"/;\n}())', - expect: '... ... undefined' + expect: '| | undefined' }, // https://github.com/nodejs/node/issues/16483 @@ -723,7 +723,7 @@ const errorTests = [ // Newline within template string maintains whitespace. { send: '`foo \n`', - expect: '... \'foo \\n\'' + expect: '| \'foo \\n\'' }, // Whitespace is not evaluated. { @@ -757,7 +757,7 @@ const errorTests = [ { send: 'x = {\nfield\n{', expect: [ - '... ... {', + '| | {', kArrow, '', /^Uncaught SyntaxError: /, @@ -774,11 +774,11 @@ const errorTests = [ }, { send: 'if (typeof process === "object"); {', - expect: '... ' + expect: '| ' }, { send: 'console.log("process is defined");', - expect: '... ' + expect: '| ' }, { send: '} else {', From 96fb15628241bb175cdb5e0417882b483db2f576 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 18 Mar 2025 10:48:31 +0100 Subject: [PATCH 04/13] repl: recover lines with syntax errors in history --- lib/repl.js | 5 +- test/parallel/test-repl-history-navigation.js | 48 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index 89a0f83d10b911..4ec7fcdd23dd6a 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -951,7 +951,6 @@ function REPLServer(prompt, self.displayPrompt(); return; } - self._domain.emit('error', e.err || e); } // In the next two if blocks, we do not use os.EOL instead of '\n' @@ -970,6 +969,10 @@ function REPLServer(prompt, ArrayPrototypeUnshift(self.history, ArrayPrototypeJoin(lines, '\r')); } + if (e) { + self._domain.emit('error', e.err || e); + } + // Clear buffer if no SyntaxErrors self.clearBufferedCommand(); sawCtrlD = false; diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 3116bc027ceae3..1c043e5eff0adc 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -720,6 +720,53 @@ const tests = [ ], clean: true }, + { + // Test that we can recover from a line with a syntax error + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + skip: !process.features.inspector, + test: [ + 'let d = ``', + ENTER, + 'd = `I am a', + ENTER, + 'super', + ENTER, + 'broken` line\'', + ENTER, + UP, + BACKSPACE, + '`', + // press LEFT 6 times to reach the typo + ...Array(6).fill(LEFT), + BACKSPACE, + ENTER, + ], + expected: [ + prompt, ...'let d = ``', + 'undefined\n', + prompt, ...'d = `I am a', + '| ', + ...'super', + '| ', + ...'broken` line\'', + "[broken` line'\n" + + ' ^^^^\n' + + '\n' + + "Uncaught SyntaxError: Unexpected identifier 'line'\n" + + '] {\n' + + ' [stack]: [Getter/Setter],\n' + + ` [message]: "Unexpected identifier 'line'"\n` + + '}\n', + prompt, + `${prompt}d = \`I am a\nsuper\nbroken\` line'`, + `${prompt}d = \`I am a\nsuper\nbroken\` line`, + '`', + `${prompt}d = \`I am a\nsuper\nbroken line\``, + "'I am a\\nsuper\\nbroken line'\n", + prompt, + ], + clean: true + }, ]; const numtests = tests.length; @@ -767,6 +814,7 @@ function runTest() { try { assert.strictEqual(output, expected[i]); } catch (e) { + console.log({ output, expected: expected[i] }); console.error(`Failed test # ${numtests - tests.length}`); console.error('Last outputs: ' + inspect(lastChunks, { breakLength: 5, colors: true From 27412587a1b4f338cb871be3af827806d5fb0f7f Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 18 Mar 2025 11:34:03 +0100 Subject: [PATCH 05/13] repl: consider removeHistoryDuplicates when saving multiline history --- lib/internal/readline/interface.js | 24 +++-- lib/repl.js | 5 +- test/parallel/test-repl-history-navigation.js | 24 ++++- .../test-repl-multiline-navigation.js | 94 +++++++++++++++++++ 4 files changed, 135 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-repl-multiline-navigation.js diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index eec9a25fd94e1f..29e4fbb61380b3 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -454,8 +454,9 @@ class Interface extends InterfaceConstructor { // If the trimmed line is empty then return the line if (StringPrototypeTrim(this.line).length === 0) return this.line; + const normalizedLine = StringPrototypeReplaceAll(this.line, '\n', '\r'); - if (this.history.length === 0 || this.history[0] !== this.line) { + if (this.history.length === 0 || this.history[0] !== normalizedLine) { if (this.removeHistoryDuplicates) { // Remove older history line if identical to new one const dupIndex = ArrayPrototypeIndexOf(this.history, this.line); @@ -936,14 +937,17 @@ class Interface extends InterfaceConstructor { [kMoveDownOrHistoryNext]() { const { cols, rows } = this.getCursorPos(); const splitLine = StringPrototypeSplit(this.line, '\n'); + if (!this.historyIndex && rows === splitLine.length) { + return; + } // Go to the next history only if the cursor is in the first line of the multiline input. // Otherwise treat the "arrow down" as a movement to the next row. if (splitLine.length > 1 && rows < splitLine.length - 1) { const currentLine = splitLine[rows]; const nextLine = splitLine[rows + 1]; const amountToMove = (cols > nextLine.length) ? - +cols + nextLine.length - 1 : - +currentLine.length + 1; + currentLine.length - cols + nextLine.length + 1 : + currentLine.length + 1; // Go to the same position on the next line, or the end of the next line // If the current position does not exist in the next line. this[kMoveCursor](amountToMove); @@ -963,9 +967,7 @@ class Interface extends InterfaceConstructor { [kHistoryNext]() { if (this.historyIndex >= 0) { this[kBeforeEdit](this.line, this.cursor); - const isLineMultiline = StringPrototypeIndexOf(this.line, '\n') !== -1; - - const search = isLineMultiline ? '' : this[kSubstringSearch] || ''; + const search = this[kSubstringSearch] || ''; let index = this.historyIndex - 1; while ( index >= 0 && @@ -987,6 +989,9 @@ class Interface extends InterfaceConstructor { [kMoveUpOrHistoryPrev]() { const { cols, rows } = this.getCursorPos(); + if (this.historyIndex === this.history.length && rows) { + return; + } const splitLine = StringPrototypeSplit(this.line, '\n'); // Go to the previous history only if the cursor is in the first line of the multiline input. // Otherwise treat the "arrow up" as a movement to the previous row. @@ -1007,9 +1012,7 @@ class Interface extends InterfaceConstructor { [kHistoryPrev]() { if (this.historyIndex < this.history.length && this.history.length) { this[kBeforeEdit](this.line, this.cursor); - const isLineMultiline = StringPrototypeIndexOf(this.line, '\n') !== -1; - - const search = isLineMultiline ? '' : this[kSubstringSearch] || ''; + const search = this[kSubstringSearch] || ''; let index = this.historyIndex + 1; while ( index < this.history.length && @@ -1122,7 +1125,8 @@ class Interface extends InterfaceConstructor { !key.meta && !key.shift ) { - if (this[kSubstringSearch] === null) { + const isLineMultiline = StringPrototypeIndexOf(this.line, '\n') !== -1; + if (this[kSubstringSearch] === null && !isLineMultiline) { this[kSubstringSearch] = StringPrototypeSlice( this.line, 0, diff --git a/lib/repl.js b/lib/repl.js index 4ec7fcdd23dd6a..de64cb1fca89ff 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -966,7 +966,10 @@ function REPLServer(prompt, ArrayPrototypePop(lines); // And replace them with the single command split by '\r' ArrayPrototypePush(lines, cmd); - ArrayPrototypeUnshift(self.history, ArrayPrototypeJoin(lines, '\r')); + const newHistoryLine = ArrayPrototypeJoin(lines, '\r'); + if (self.history[0] !== newHistoryLine) { + ArrayPrototypeUnshift(self.history, newHistoryLine); + } } if (e) { diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 1c043e5eff0adc..35a90db6c36a5d 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -767,6 +767,29 @@ const tests = [ ], clean: true }, + { + // Test that multiline history is not duplicated + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + skip: !process.features.inspector, + test: [ + 'let f = `multiline', + ENTER, + 'string`', + ENTER, + UP, UP, UP, + ], + expected: [ + prompt, ...'let f = `multiline', + '| ', + ...'string`', + 'undefined\n', + prompt, + `${prompt}let f = \`multiline\nstring\``, + `${prompt}let f = \`multiline\nstring\``, + prompt, + ], + clean: true + }, ]; const numtests = tests.length; @@ -814,7 +837,6 @@ function runTest() { try { assert.strictEqual(output, expected[i]); } catch (e) { - console.log({ output, expected: expected[i] }); console.error(`Failed test # ${numtests - tests.length}`); console.error('Last outputs: ' + inspect(lastChunks, { breakLength: 5, colors: true diff --git a/test/parallel/test-repl-multiline-navigation.js b/test/parallel/test-repl-multiline-navigation.js new file mode 100644 index 00000000000000..5b617b6736f72a --- /dev/null +++ b/test/parallel/test-repl-multiline-navigation.js @@ -0,0 +1,94 @@ +'use strict'; + +// Flags: --expose-internals + +const common = require('../common'); + +const assert = require('assert'); +const repl = require('internal/repl'); +const stream = require('stream'); +const fs = require('fs'); + +class ActionStream extends stream.Stream { + run(data) { + const _iter = data[Symbol.iterator](); + const doAction = () => { + const next = _iter.next(); + if (next.done) { + // Close the repl. Note that it must have a clean prompt to do so. + this.emit('keypress', '', { ctrl: true, name: 'd' }); + return; + } + const action = next.value; + + if (typeof action === 'object') { + this.emit('keypress', '', action); + } + setImmediate(doAction); + }; + doAction(); + } + resume() {} + pause() {} +} +ActionStream.prototype.readable = true; + +function cleanupTmpFile() { + try { + // Write over the file, clearing any history + fs.writeFileSync(defaultHistoryPath, ''); + } catch (err) { + if (err.code === 'ENOENT') return true; + throw err; + } + return true; +} + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); +const defaultHistoryPath = tmpdir.resolve('.node_repl_history'); + +{ + cleanupTmpFile(); + // It moves the cursor at the right places + const checkResults = common.mustSucceed((r) => { + r.write('let str = `'); + r.input.run([{ name: 'enter' }]); + r.write('111'); + r.input.run([{ name: 'enter' }]); + r.write('22222222222222'); + r.input.run([{ name: 'enter' }]); + r.write('3`'); + r.input.run([{ name: 'enter' }]); + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.cursor, 33); + + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.cursor, 18); + + r.input.run([{ name: 'right' }]); + r.input.run([{ name: 'right' }]); + r.input.run([{ name: 'right' }]); + r.input.run([{ name: 'right' }]); + r.input.run([{ name: 'right' }]); + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.cursor, 15); + + r.input.run([{ name: 'down' }]); + assert.strictEqual(r.cursor, 19); + }); + + repl.createInternalRepl( + { NODE_REPL_HISTORY: defaultHistoryPath }, + { + terminal: true, + input: new ActionStream(), + output: new stream.Writable({ + write(chunk, _, next) { + next(); + } + }), + }, + checkResults + ); +} From b41273af816d5274ac61c1e5cf099929b369f078 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Wed, 19 Mar 2025 11:12:24 +0100 Subject: [PATCH 06/13] repl: add pipe char for browsing multiline history --- lib/internal/readline/interface.js | 48 ++++++-- lib/internal/repl/utils.js | 22 ++++ lib/repl.js | 3 + test/parallel/test-repl-history-navigation.js | 64 +++++++--- .../test-repl-multiline-navigation.js | 114 +++++++++++++++++- 5 files changed, 219 insertions(+), 32 deletions(-) diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 29e4fbb61380b3..deeca2de42527c 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -457,6 +457,12 @@ class Interface extends InterfaceConstructor { const normalizedLine = StringPrototypeReplaceAll(this.line, '\n', '\r'); if (this.history.length === 0 || this.history[0] !== normalizedLine) { + if (this.lastCommandErrored && this.historyIndex === 0) { + // If the last command errored, remove it from history. + // The user is issuing a new command starting from the errored command, + // Hopefully with the fix + ArrayPrototypeShift(this.history); + } if (this.removeHistoryDuplicates) { // Remove older history line if identical to new one const dupIndex = ArrayPrototypeIndexOf(this.history, this.line); @@ -504,8 +510,22 @@ class Interface extends InterfaceConstructor { // erase data clearScreenDown(this.output); - // Write the prompt and the current buffer content. - this[kWriteToOutput](line); + // Check if this is a multiline entry + const lines = StringPrototypeSplit(this.line, '\n'); + const isMultiline = lines.length > 1; + + if (!isMultiline) { + // Write the prompt and the current buffer content. + this[kWriteToOutput](line); + } else { + // Write first line with normal prompt + this[kWriteToOutput](this[kPrompt] + lines[0]); + + // For continuation lines, add the "|" prefix + for (let i = 1; i < lines.length; i++) { + this[kWriteToOutput]('\n| ' + lines[i]); + } + } // Force terminal to allocate a new line if (lineCols === 0) { @@ -945,8 +965,8 @@ class Interface extends InterfaceConstructor { if (splitLine.length > 1 && rows < splitLine.length - 1) { const currentLine = splitLine[rows]; const nextLine = splitLine[rows + 1]; - const amountToMove = (cols > nextLine.length) ? - currentLine.length - cols + nextLine.length + 1 : + const amountToMove = (cols > nextLine.length + 1) ? + currentLine.length - cols + nextLine.length + 3 : currentLine.length + 1; // Go to the same position on the next line, or the end of the next line // If the current position does not exist in the next line. @@ -997,8 +1017,8 @@ class Interface extends InterfaceConstructor { // Otherwise treat the "arrow up" as a movement to the previous row. if (splitLine.length > 1 && rows > 0) { const previousLine = splitLine[rows - 1]; - const amountToMove = (cols > previousLine.length) ? - -cols - 1 : + const amountToMove = (cols > previousLine.length + 1) ? + -cols + 1 : -previousLine.length - 1; // Go to the same position on the previous line, or the end of the previous line // If the current position does not exist in the previous line. @@ -1075,9 +1095,19 @@ class Interface extends InterfaceConstructor { * }} */ getCursorPos() { - const strBeforeCursor = - this[kPrompt] + StringPrototypeSlice(this.line, 0, this.cursor); - return this[kGetDisplayPos](strBeforeCursor); + // Handle multiline input by accounting for "| " prefixes on continuation lines + const lineUptoCursor = StringPrototypeSlice(this.line, 0, this.cursor); + const lines = StringPrototypeSplit(lineUptoCursor, '\n'); + + // First line uses normal prompt, subsequent lines use "| " (2 chars) + let displayText = this[kPrompt] + lines[0]; + + // Add continuation lines with their prefixes + for (let i = 1; i < lines.length; i++) { + displayText += '\n| ' + lines[i]; + } + + return this[kGetDisplayPos](displayText); } // This function moves cursor dx places to the right diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index 126f8ae85d0977..bc768d6f454d9f 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -155,8 +155,15 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { let escaped = null; + let justExecutedMultilineCommand = false; + function getPreviewPos() { const displayPos = repl._getDisplayPos(`${repl.getPrompt()}${repl.line}`); + // If the line is a multi line + if (StringPrototypeIndexOf(repl.line, '\n') !== -1) { + // This is to consider the additional "| " that prefixes the line + displayPos.cols += 2; + } const cursorPos = repl.line.length !== repl.cursor ? repl.getCursorPos() : displayPos; @@ -177,6 +184,11 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { clearLine(repl.output); moveCursor(repl.output, 0, -rows); inputPreview = null; + // If pressing enter on some history line, + // The next preview should not be generated + if ((key.name === 'return' || key.name === 'enter') && !key.meta && repl.historyIndex !== -1) { + justExecutedMultilineCommand = true; + } } if (completionPreview !== null) { // Prevent cursor moves if not necessary! @@ -373,6 +385,11 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { return; } + if (justExecutedMultilineCommand) { + justExecutedMultilineCommand = false; + return; + } + hasCompletions = false; // Add the autocompletion preview. @@ -444,9 +461,14 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { const { cursorPos, displayPos } = getPreviewPos(); const rows = displayPos.rows - cursorPos.rows; + // Moves one line below all the user lines moveCursor(repl.output, 0, rows); + // Writes the preview there repl.output.write(`\n${result}`); + + // Go back to the horizontal position of the cursor cursorTo(repl.output, cursorPos.cols); + // Go back to the vertical position of the cursor moveCursor(repl.output, 0, -rows - 1); }; diff --git a/lib/repl.js b/lib/repl.js index de64cb1fca89ff..705be580b6e6a7 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -974,6 +974,9 @@ function REPLServer(prompt, if (e) { self._domain.emit('error', e.err || e); + self.lastCommandErrored = true; + } else { + self.lastCommandErrored = false; } // Clear buffer if no SyntaxErrors diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 35a90db6c36a5d..4325f90148eb0b 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -659,11 +659,19 @@ const tests = [ ...'which ends here`', "'I am a multiline strong\\nwhich ends here'\n", prompt, - `${prompt}a = \`I am a multiline strong\nwhich ends here\``, - `${prompt}a = \`I am a multiline strong\nwhich ends here\``, - `${prompt}a = \`I am a multiline strng\nwhich ends here\``, - `${prompt}a = \`I am a multiline string\nwhich ends here\``, - `${prompt}a = \`I am a multiline string\nwhich ends here\``, + `${prompt}a = \`I am a multiline strong`, + `\n| which ends here\``, + `${prompt}a = \`I am a multiline strong`, + `\n| which ends here\``, + + `${prompt}a = \`I am a multiline strng`, + `\n| which ends here\``, + + `${prompt}a = \`I am a multiline string`, + `\n| which ends here\``, + + `${prompt}a = \`I am a multiline string`, + `\n| which ends here\``, "'I am a multiline string\\nwhich ends here'\n", prompt, ], @@ -707,14 +715,24 @@ const tests = [ ...'am another one`', 'undefined\n', prompt, - `${prompt}let c = \`I\nam another one\``, - `${prompt}let c = \`I\nam another one\``, - - `${prompt}b = \`I am a multiline strong\nwhich ends here\``, - `${prompt}b = \`I am a multiline strong\nwhich ends here\``, - `${prompt}b = \`I am a multiline strng\nwhich ends here\``, - `${prompt}b = \`I am a multiline string\nwhich ends here\``, - `${prompt}b = \`I am a multiline string\nwhich ends here\``, + `${prompt}let c = \`I`, + `\n| am another one\``, + + `${prompt}let c = \`I`, + `\n| am another one\``, + + `${prompt}b = \`I am a multiline strong`, + `\n| which ends here\``, + `${prompt}b = \`I am a multiline strong`, + `\n| which ends here\``, + `${prompt}b = \`I am a multiline strng`, + `\n| which ends here\``, + + `${prompt}b = \`I am a multiline string`, + `\n| which ends here\``, + + `${prompt}b = \`I am a multiline string`, + `\n| which ends here\``, "'I am a multiline string\\nwhich ends here'\n", prompt, ], @@ -758,10 +776,18 @@ const tests = [ ` [message]: "Unexpected identifier 'line'"\n` + '}\n', prompt, - `${prompt}d = \`I am a\nsuper\nbroken\` line'`, - `${prompt}d = \`I am a\nsuper\nbroken\` line`, + `${prompt}d = \`I am a`, + `\n| super`, + `\n| broken\` line'`, + + `${prompt}d = \`I am a`, + `\n| super`, + '\n| broken` line', '`', - `${prompt}d = \`I am a\nsuper\nbroken line\``, + + `${prompt}d = \`I am a`, + `\n| super`, + `\n| broken line\``, "'I am a\\nsuper\\nbroken line'\n", prompt, ], @@ -784,8 +810,10 @@ const tests = [ ...'string`', 'undefined\n', prompt, - `${prompt}let f = \`multiline\nstring\``, - `${prompt}let f = \`multiline\nstring\``, + `${prompt}let f = \`multiline`, + `\n| string\``, + `${prompt}let f = \`multiline`, + `\n| string\``, prompt, ], clean: true diff --git a/test/parallel/test-repl-multiline-navigation.js b/test/parallel/test-repl-multiline-navigation.js index 5b617b6736f72a..996910ce17149e 100644 --- a/test/parallel/test-repl-multiline-navigation.js +++ b/test/parallel/test-repl-multiline-navigation.js @@ -28,6 +28,15 @@ class ActionStream extends stream.Stream { }; doAction(); } + write(chunk) { + const chunkLines = chunk.toString('utf8').split('\n'); + this.lines[this.lines.length - 1] += chunkLines[0]; + if (chunkLines.length > 1) { + this.lines.push(...chunkLines.slice(1)); + } + this.emit('line', this.lines[this.lines.length - 1]); + return true; + } resume() {} pause() {} } @@ -66,12 +75,19 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history'); r.input.run([{ name: 'up' }]); assert.strictEqual(r.cursor, 18); - r.input.run([{ name: 'right' }]); - r.input.run([{ name: 'right' }]); - r.input.run([{ name: 'right' }]); - r.input.run([{ name: 'right' }]); - r.input.run([{ name: 'right' }]); + for (let i = 0; i < 5; i++) { + r.input.run([{ name: 'right' }]); + } + r.input.run([{ name: 'up' }]); + assert.strictEqual(r.cursor, 15); r.input.run([{ name: 'up' }]); + + for (let i = 0; i < 5; i++) { + r.input.run([{ name: 'right' }]); + } + assert.strictEqual(r.cursor, 8); + + r.input.run([{ name: 'down' }]); assert.strictEqual(r.cursor, 15); r.input.run([{ name: 'down' }]); @@ -92,3 +108,91 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history'); checkResults ); } + +{ + cleanupTmpFile(); + // If the last command errored and the user is trying to edit it, + // The errored line should be removed from history + const checkResults = common.mustSucceed((r) => { + r.write('let lineWithMistake = `I have some'); + r.input.run([{ name: 'enter' }]); + r.write('problem with` my syntax\''); + r.input.run([{ name: 'enter' }]); + r.input.run([{ name: 'up' }]); + r.input.run([{ name: 'backspace' }]); + r.write('`'); + for (let i = 0; i < 11; i++) { + r.input.run([{ name: 'left' }]); + } + r.input.run([{ name: 'backspace' }]); + r.input.run([{ name: 'enter' }]); + + assert.strictEqual(r.history.length, 1); + assert.strictEqual(r.history[0], 'let lineWithMistake = `I have some\rproblem with my syntax`'); + assert.strictEqual(r.line, ''); + }); + + repl.createInternalRepl( + { NODE_REPL_HISTORY: defaultHistoryPath }, + { + terminal: true, + input: new ActionStream(), + output: new stream.Writable({ + write(chunk, _, next) { + next(); + } + }), + }, + checkResults + ); +} + +{ + cleanupTmpFile(); + const outputBuffer = []; + + // Test that the REPL preview is properly shown on multiline commands + // And deleted when enter is pressed + const checkResults = common.mustSucceed((r) => { + r.write('Array(100).fill('); + r.input.run([{ name: 'enter' }]); + r.write('123'); + r.input.run([{ name: 'enter' }]); + r.write(')'); + r.input.run([{ name: 'enter' }]); + r.input.run([{ name: 'up' }]); + r.input.run([{ name: 'up' }]); + + assert.deepStrictEqual(r.last, new Array(100).fill(123)); + r.input.run([{ name: 'enter' }]); + assert.strictEqual(outputBuffer.includes('[\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' + + ' 123\n' + + ']\n'), true); + }); + + repl.createInternalRepl( + { NODE_REPL_HISTORY: defaultHistoryPath }, + { + preview: true, + terminal: true, + input: new ActionStream(), + output: new stream.Writable({ + write(chunk, _, next) { + // Store each chunk in the buffer + outputBuffer.push(chunk.toString()); + next(); + } + }), + }, + checkResults + ); +} From eb931ad6b609db513f93b312b31611653ab6364c Mon Sep 17 00:00:00 2001 From: Giovanni Date: Wed, 26 Mar 2025 18:15:32 +0100 Subject: [PATCH 07/13] repl: update the cursor logic in kGetDisplayPos instead of getCursorPos --- doc/api/repl.md | 10 +- lib/internal/readline/interface.js | 177 +++++++++++------- lib/internal/repl/history.js | 2 +- lib/internal/repl/utils.js | 11 +- lib/repl.js | 25 ++- ...multiline => .node_repl_history_multiline} | 0 test/parallel/test-repl-history-navigation.js | 60 +++--- .../test-repl-load-multiline-from-history.js | 40 ++-- .../test-repl-multiline-navigation.js | 4 +- 9 files changed, 184 insertions(+), 145 deletions(-) rename test/fixtures/{.node_repl_history-multiline => .node_repl_history_multiline} (100%) diff --git a/doc/api/repl.md b/doc/api/repl.md index a134c493a54812..3b2f06fec780fd 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -605,8 +605,8 @@ The `replServer.displayPrompt()` method readies the REPL instance for input from the user, printing the configured `prompt` to a new line in the `output` and resuming the `input` to accept new input. -When multi-line input is being entered, an ellipsis is printed rather than the -'prompt'. +When multi-line input is being entered, the "|" character is printed rather +than the 'prompt'. When `preserveCursor` is `true`, the cursor placement will not be reset to `0`. @@ -657,6 +657,12 @@ A list of the names of all Node.js modules, e.g., `'http'`.