Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2e54a99

Browse filesBrowse 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
Copy full SHA for 2e54a99

File tree

Expand file treeCollapse file tree

5 files changed

+90
-28
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+90
-28
lines changed
Open diff view settings
Collapse file

‎lib/readline.js‎

Copy file name to clipboardExpand all lines: lib/readline.js
+2-4Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-readline-interface.js
+14-6Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-repl-history-navigation.js
+9-5Lines 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'
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-repl-persistent-history.js
+34-7Lines 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) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-repl-programmatic-history.js
+31-6Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.