Skip to content

Commit 2e54a99

Browse files
BridgeARMylesBorins
BridgeAR
authored and
MylesBorins
committed
readline,repl: improve history up/previous
Reaching the history end caused the last entry to be persistent. That way there's no actualy feedback to the user that the history end is reached. Instead, visualize the original input line and keep the history index at the history end in case the user wants to go back again. PR-URL: #31112 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent cecd256 commit 2e54a99

File tree

5 files changed

+90
-28
lines changed

5 files changed

+90
-28
lines changed

‎lib/readline.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ Interface.prototype._historyNext = function() {
718718
};
719719

720720
Interface.prototype._historyPrev = function() {
721-
if (this.historyIndex < this.history.length) {
721+
if (this.historyIndex < this.history.length && this.history.length) {
722722
const search = this[kSubstringSearch] || '';
723723
let index = this.historyIndex + 1;
724724
while (index < this.history.length &&
@@ -727,9 +727,7 @@ Interface.prototype._historyPrev = function() {
727727
index++;
728728
}
729729
if (index === this.history.length) {
730-
// TODO(BridgeAR): Change this to:
731-
// this.line = search;
732-
return;
730+
this.line = search;
733731
} else {
734732
this.line = this.history[index];
735733
}

‎test/parallel/test-readline-interface.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,20 @@ function isWarned(emitter) {
482482
fi.emit('keypress', '.', { name: 'up' }); // 'baz'
483483
assert.strictEqual(rli.historyIndex, 2);
484484
assert.strictEqual(rli.line, 'baz');
485-
fi.emit('keypress', '.', { name: 'up' }); // 'baz'
486-
assert.strictEqual(rli.historyIndex, 2);
487-
assert.strictEqual(rli.line, 'baz');
488-
fi.emit('keypress', '.', { name: 'up' }); // 'baz'
489-
assert.strictEqual(rli.historyIndex, 2);
490-
assert.strictEqual(rli.line, 'baz');
485+
fi.emit('keypress', '.', { name: 'up' }); // 'ba'
486+
assert.strictEqual(rli.historyIndex, 4);
487+
assert.strictEqual(rli.line, 'ba');
488+
fi.emit('keypress', '.', { name: 'up' }); // 'ba'
489+
assert.strictEqual(rli.historyIndex, 4);
490+
assert.strictEqual(rli.line, 'ba');
491+
// Deactivate substring history search and reset history index.
492+
fi.emit('keypress', '.', { name: 'right' }); // 'ba'
493+
assert.strictEqual(rli.historyIndex, -1);
494+
assert.strictEqual(rli.line, 'ba');
495+
// Substring history search activated.
496+
fi.emit('keypress', '.', { name: 'up' }); // 'ba'
497+
assert.strictEqual(rli.historyIndex, 0);
498+
assert.strictEqual(rli.line, 'bat');
491499
rli.close();
492500
}
493501

‎test/parallel/test-repl-history-navigation.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ const tests = [
7878
},
7979
{
8080
env: { NODE_REPL_HISTORY: defaultHistoryPath },
81-
checkTotal: true,
82-
test: [UP, UP, UP, UP, UP, DOWN, DOWN, DOWN, DOWN],
81+
test: [UP, UP, UP, UP, UP, DOWN, DOWN, DOWN, DOWN, DOWN],
8382
expected: [prompt,
8483
`${prompt}Array(100).fill(1).map((e, i) => i ** 2)`,
8584
prev && '\n// [ 0, 1, 4, 9, 16, 25, 36, 49, 64, 81, 100, 121, ' +
@@ -92,6 +91,8 @@ const tests = [
9291
`${prompt}555 + 909`,
9392
prev && '\n// 1464',
9493
`${prompt}let ab = 45`,
94+
prompt,
95+
`${prompt}let ab = 45`,
9596
`${prompt}555 + 909`,
9697
prev && '\n// 1464',
9798
`${prompt}{key : {key2 :[] }}`,
@@ -138,9 +139,12 @@ const tests = [
138139
// UP - skipping const foo = true
139140
'\x1B[1G', '\x1B[0J',
140141
'> 555 + 909', '\x1B[12G',
141-
// UP, UP, ENTER. UPs at the end of the history have no effect.
142-
'\r\n',
143-
'1464\n',
142+
// UP, UP
143+
// UPs at the end of the history reset the line to the original input.
144+
'\x1B[1G', '\x1B[0J',
145+
'> 55', '\x1B[5G',
146+
// ENTER
147+
'\r\n', '55\n',
144148
'\x1B[1G', '\x1B[0J',
145149
'> ', '\x1B[3G',
146150
'\r\n'

‎test/parallel/test-repl-persistent-history.js

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const assert = require('assert');
1010
const fs = require('fs');
1111
const path = require('path');
1212
const os = require('os');
13+
const util = require('util');
1314

1415
const tmpdir = require('../common/tmpdir');
1516
tmpdir.refresh();
@@ -51,6 +52,7 @@ ActionStream.prototype.readable = true;
5152

5253
// Mock keys
5354
const UP = { name: 'up' };
55+
const DOWN = { name: 'down' };
5456
const ENTER = { name: 'enter' };
5557
const CLEAR = { ctrl: true, name: 'u' };
5658

@@ -90,20 +92,42 @@ const tests = [
9092
},
9193
{
9294
env: {},
93-
test: [UP, '\'42\'', ENTER],
94-
expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
95+
test: [UP, '21', ENTER, "'42'", ENTER],
96+
expected: [
97+
prompt,
98+
'2', '1', '21\n', prompt, prompt,
99+
"'", '4', '2', "'", "'42'\n", prompt, prompt
100+
],
95101
clean: false
96102
},
97103
{ // Requires the above test case
98104
env: {},
99-
test: [UP, UP, ENTER],
100-
expected: [prompt, `${prompt}'42'`, '\'42\'\n', prompt]
105+
test: [UP, UP, CLEAR, ENTER, DOWN, CLEAR, ENTER, UP, ENTER],
106+
expected: [
107+
prompt,
108+
`${prompt}'42'`,
109+
`${prompt}21`,
110+
prompt,
111+
prompt,
112+
`${prompt}'42'`,
113+
prompt,
114+
prompt,
115+
`${prompt}21`,
116+
'21\n',
117+
prompt,
118+
]
101119
},
102120
{
103121
env: { NODE_REPL_HISTORY: historyPath,
104122
NODE_REPL_HISTORY_SIZE: 1 },
105-
test: [UP, UP, CLEAR],
106-
expected: [prompt, `${prompt}'you look fabulous today'`, prompt]
123+
test: [UP, UP, DOWN, CLEAR],
124+
expected: [
125+
prompt,
126+
`${prompt}'you look fabulous today'`,
127+
prompt,
128+
`${prompt}'you look fabulous today'`,
129+
prompt
130+
]
107131
},
108132
{
109133
env: { NODE_REPL_HISTORY: historyPathFail,
@@ -169,6 +193,8 @@ function runTest(assertCleaned) {
169193
const opts = tests.shift();
170194
if (!opts) return; // All done
171195

196+
console.log('NEW');
197+
172198
if (assertCleaned) {
173199
try {
174200
assert.strictEqual(fs.readFileSync(defaultHistoryPath, 'utf8'), '');
@@ -193,6 +219,7 @@ function runTest(assertCleaned) {
193219
output: new stream.Writable({
194220
write(chunk, _, next) {
195221
const output = chunk.toString();
222+
console.log('INPUT', util.inspect(output));
196223

197224
// Ignore escapes and blank lines
198225
if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output))
@@ -207,7 +234,7 @@ function runTest(assertCleaned) {
207234
next();
208235
}
209236
}),
210-
prompt: prompt,
237+
prompt,
211238
useColors: false,
212239
terminal: true
213240
}, function(err, repl) {

‎test/parallel/test-repl-programmatic-history.js

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const assert = require('assert');
88
const fs = require('fs');
99
const path = require('path');
1010
const os = require('os');
11+
const util = require('util');
1112

1213
const tmpdir = require('../common/tmpdir');
1314
tmpdir.refresh();
@@ -49,6 +50,7 @@ ActionStream.prototype.readable = true;
4950

5051
// Mock keys
5152
const UP = { name: 'up' };
53+
const DOWN = { name: 'down' };
5254
const ENTER = { name: 'enter' };
5355
const CLEAR = { ctrl: true, name: 'u' };
5456

@@ -88,20 +90,40 @@ const tests = [
8890
},
8991
{
9092
env: {},
91-
test: [UP, '\'42\'', ENTER],
92-
expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
93+
test: [UP, '21', ENTER, "'42'", ENTER],
94+
expected: [
95+
prompt,
96+
// TODO(BridgeAR): The line is refreshed too many times. The double prompt
97+
// is redundant and can be optimized away.
98+
'2', '1', '21\n', prompt, prompt,
99+
"'", '4', '2', "'", "'42'\n", prompt, prompt
100+
],
93101
clean: false
94102
},
95103
{ // Requires the above test case
96104
env: {},
97-
test: [UP, UP, ENTER],
98-
expected: [prompt, `${prompt}'42'`, '\'42\'\n', prompt]
105+
test: [UP, UP, UP, DOWN, ENTER],
106+
expected: [
107+
prompt,
108+
`${prompt}'42'`,
109+
`${prompt}21`,
110+
prompt,
111+
`${prompt}21`,
112+
'21\n',
113+
prompt
114+
]
99115
},
100116
{
101117
env: { NODE_REPL_HISTORY: historyPath,
102118
NODE_REPL_HISTORY_SIZE: 1 },
103-
test: [UP, UP, CLEAR],
104-
expected: [prompt, `${prompt}'you look fabulous today'`, prompt]
119+
test: [UP, UP, DOWN, CLEAR],
120+
expected: [
121+
prompt,
122+
`${prompt}'you look fabulous today'`,
123+
prompt,
124+
`${prompt}'you look fabulous today'`,
125+
prompt
126+
]
105127
},
106128
{
107129
env: { NODE_REPL_HISTORY: historyPathFail,
@@ -167,6 +189,8 @@ function runTest(assertCleaned) {
167189
const opts = tests.shift();
168190
if (!opts) return; // All done
169191

192+
console.log('NEW');
193+
170194
if (assertCleaned) {
171195
try {
172196
assert.strictEqual(fs.readFileSync(defaultHistoryPath, 'utf8'), '');
@@ -192,6 +216,7 @@ function runTest(assertCleaned) {
192216
output: new stream.Writable({
193217
write(chunk, _, next) {
194218
const output = chunk.toString();
219+
console.log('INPUT', util.inspect(output));
195220

196221
// Ignore escapes and blank lines
197222
if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output))

0 commit comments

Comments
 (0)