FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

readline: fix issue with newline-less last line · nodejs/node@114b847 · GitHub

/ node Public

Commit 114b847

Browse files
harrisi
authored and
MoLow
committed
readline: fix issue with newline-less last line
The logic for reading lines was slightly flawed, in that it assumed there would be a final new line. It handled the case where there are no new lines, but this then broke if there were some new lines. The fix in logic is basically removing the special case where there are no new lines by changing it to always read the final line with no new lines. This works because if a file contains no new lines, the final line is the first line, and all is well. There is some subtlety in this functioning, however. If the last line contains no new lines, then `lastIndex` will be the start of the last line, and `kInsertString` will be called from that point. If it does contain a new line, `lastIndex` will be equal to `s.length`, so the slice will be the empty string. Fixes: #47305 PR-URL: #47317 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 3a9c43a commit 114b847

4 files changed

Lines changed: 85 additions & 14 deletions

File tree

‎lib/internal/readline/interface.js‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,21 +1331,19 @@ class Interface extends InterfaceConstructor {
13311331
if (typeof s === 'string' && s) {
13321332
// Erase state of previous searches.
13331333
lineEnding.lastIndex = 0;
1334-
let nextMatch = RegExpPrototypeExec(lineEnding, s);
1335-
// If no line endings are found, just insert the string as is.
1336-
if (nextMatch === null) {
1337-
this[kInsertString](s);
1338-
} else {
1339-
// Keep track of the end of the last match.
1340-
let lastIndex = 0;
1341-
do {
1342-
this[kInsertString](StringPrototypeSlice(s, lastIndex, nextMatch.index));
1343-
({ lastIndex } = lineEnding);
1344-
this[kLine]();
1345-
// Restore lastIndex as the call to kLine could have mutated it.
1346-
lineEnding.lastIndex = lastIndex;
1347-
} while ((nextMatch = RegExpPrototypeExec(lineEnding, s)) !== null);
1334+
let nextMatch;
1335+
// Keep track of the end of the last match.
1336+
let lastIndex = 0;
1337+
while ((nextMatch = RegExpPrototypeExec(lineEnding, s)) !== null) {
1338+
this[kInsertString](StringPrototypeSlice(s, lastIndex, nextMatch.index));
1339+
({ lastIndex } = lineEnding);
1340+
this[kLine]();
1341+
// Restore lastIndex as the call to kLine could have mutated it.
1342+
lineEnding.lastIndex = lastIndex;
13481343
}
1344+
// This ensures that the last line is written if it doesn't end in a newline.
1345+
// Note that the last line may be the first line, in which case this still works.
1346+
this[kInsertString](StringPrototypeSlice(s, lastIndex));
13491347
}
13501348
}
13511349
}

‎test/fixtures/repl-load-multiline-no-trailing-newline.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// The lack of a newline at the end of this file is intentional.
2+
const getLunch = () =>
3+
placeOrder('tacos')
4+
.then(eat);
5+
6+
const placeOrder = (order) => Promise.resolve(order);
7+
const eat = (food) => '<nom nom nom>';

‎test/parallel/test-readline-interface-no-trailing-newline.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
const common = require('../common');
3+
const ArrayStream = require('../common/arraystream');
4+
const assert = require('assert');
5+
6+
common.skipIfDumbTerminal();
7+
8+
const readline = require('readline');
9+
const rli = new readline.Interface({
10+
terminal: true,
11+
input: new ArrayStream(),
12+
output: new ArrayStream(),
13+
});
14+
15+
// Minimal reproduction for #47305
16+
const testInput = '{\n}';
17+
18+
let accum = '';
19+
20+
rli.output.write = (data) => accum += data.replace('\r', '');
21+
22+
rli.write(testInput);
23+
24+
assert.strictEqual(accum, testInput);

‎test/parallel/test-repl-load-multiline-no-trailing-newline.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
const common = require('../common');
3+
const ArrayStream = require('../common/arraystream');
4+
const fixtures = require('../common/fixtures');
5+
const assert = require('assert');
6+
const repl = require('repl');
7+
8+
common.skipIfDumbTerminal();
9+
10+
const command = `.load ${fixtures.path('repl-load-multiline-no-trailing-newline.js')}`;
11+
const terminalCode = '\u001b[1G\u001b[0J \u001b[1G';
12+
const terminalCodeRegex = new RegExp(terminalCode.replace(/\[/g, '\\['), 'g');
13+
14+
const expected = `${command}
15+
// The lack of a newline at the end of this file is intentional.
16+
const getLunch = () =>
17+
placeOrder('tacos')
18+
.then(eat);
19+
20+
const placeOrder = (order) => Promise.resolve(order);
21+
const eat = (food) => '<nom nom nom>';
22+
undefined
23+
`;
24+
25+
let accum = '';
26+
27+
const inputStream = new ArrayStream();
28+
const outputStream = new ArrayStream();
29+
30+
outputStream.write = (data) => accum += data.replace('\r', '');
31+
32+
const r = repl.start({
33+
prompt: '',
34+
input: inputStream,
35+
output: outputStream,
36+
terminal: true,
37+
useColors: false
38+
});
39+
40+
r.write(`${command}\n`);
41+
assert.strictEqual(accum.replace(terminalCodeRegex, ''), expected);
42+
r.close();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL