Skip to content

repl: fix multiline history editing string order #57874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2025
Merged

repl: fix multiline history editing string order #57874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions lib/internal/readline/interface.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,15 @@ class Interface extends InterfaceConstructor {
}

// Convert newlines to a consistent format for history storage
[kNormalizeHistoryLineEndings](line, from, to) {
[kNormalizeHistoryLineEndings](line, from, to, reverse = true) {
// Multiline history entries are saved reversed
if (StringPrototypeIncludes(line, '\r')) {
// History is structured with the newest entries at the top
// and the oldest at the bottom. Multiline histories, however, only occupy
// one line in the history file. When loading multiline history with
// an old node binary, the history will be saved in the old format.
// This is why we need to reverse the multilines.
// Reversing the multilines is necessary when adding / editing and displaying them
if (reverse) {
// First reverse the lines for proper order, then convert separators
return ArrayPrototypeJoin(
ArrayPrototypeReverse(StringPrototypeSplit(line, from)),
Expand All @@ -488,7 +494,7 @@ 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 = this[kNormalizeHistoryLineEndings](this.line, '\n', '\r');
const normalizedLine = this[kNormalizeHistoryLineEndings](this.line, '\n', '\r', false);

if (this.history.length === 0 || this.history[0] !== normalizedLine) {
if (this[kLastCommandErrored] && this.historyIndex === 0) {
Expand Down
32 changes: 12 additions & 20 deletions test/parallel/test-repl-multiline-navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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) {
Expand Down Expand Up @@ -42,23 +41,11 @@ class ActionStream extends stream.Stream {
}
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();
const historyPath = tmpdir.resolve(`.${Math.floor(Math.random() * 10000)}`);
// Make sure the cursor is at the right places.
// If the cursor is at the end of a long line and the down key is pressed,
// Move the cursor to the end of the next line, if shorter.
Expand Down Expand Up @@ -97,7 +84,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
});

repl.createInternalRepl(
{ NODE_REPL_HISTORY: defaultHistoryPath },
{ NODE_REPL_HISTORY: historyPath },
{
terminal: true,
input: new ActionStream(),
Expand All @@ -112,7 +99,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
}

{
cleanupTmpFile();
const historyPath = tmpdir.resolve(`.${Math.floor(Math.random() * 10000)}`);
// 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) => {
Expand All @@ -130,12 +117,17 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
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`');
// Check that the line is properly set in the history structure
assert.strictEqual(r.history[0], 'problem with my syntax`\rlet lineWithMistake = `I have some');
assert.strictEqual(r.line, '');

r.input.run([{ name: 'up' }]);
// Check that the line is properly displayed
assert.strictEqual(r.line, 'let lineWithMistake = `I have some\nproblem with my syntax`');
});

repl.createInternalRepl(
{ NODE_REPL_HISTORY: defaultHistoryPath },
{ NODE_REPL_HISTORY: historyPath },
{
terminal: true,
input: new ActionStream(),
Expand All @@ -150,7 +142,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
}

{
cleanupTmpFile();
const historyPath = tmpdir.resolve(`.${Math.floor(Math.random() * 10000)}`);
const outputBuffer = [];

// Test that the REPL preview is properly shown on multiline commands
Expand Down Expand Up @@ -182,7 +174,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
});

repl.createInternalRepl(
{ NODE_REPL_HISTORY: defaultHistoryPath },
{ NODE_REPL_HISTORY: historyPath },
{
preview: true,
terminal: true,
Expand Down
Loading