Skip to content

Commit

Permalink
repl,util: insert carriage returns in output
Browse files Browse the repository at this point in the history 
`\n` is not enough for Linux with some custom stream
add carriage returns to ensure that the output is displayed correctly
using `\r\n` should not be a problem, even on non-Windows platforms.

Fixes: nodejs#7954
PR-URL: nodejs#8028
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
@JungMinu
JungMinu committed Aug 13, 2016
1 parent 6a3dbda commit fce4b98
Show file tree
Hide file tree
Showing 19 changed files with 201 additions and 191 deletions.
54 changes: 28 additions & 26 deletions lib/repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ function REPLServer(prompt,
var err, result, retry = false, input = code, wrappedErr;
// first, create the Script object to check the syntax

if (code === '\n')
if (code === '\n' || code === '\r\n')
return cb(null);

while (true) {
Expand All @@ -251,7 +251,7 @@ function REPLServer(prompt,
(self.replMode === exports.REPL_MODE_STRICT || retry)) {
// "void 0" keeps the repl from returning "use strict" as the
// result value for let/const statements.
code = `'use strict'; void 0;\n${code}`;
code = `'use strict'; void 0;\r\n${code}`;
}
var script = vm.createScript(code, {
filename: file,
Expand All @@ -265,7 +265,7 @@ function REPLServer(prompt,
if (self.wrappedCmd) {
self.wrappedCmd = false;
// unwrap and try again
code = `${input.substring(1, input.length - 2)}\n`;
code = `${input.substring(1, input.length - 2)}\r\n`;
wrappedErr = e;
} else {
retry = true;
Expand Down Expand Up @@ -367,7 +367,7 @@ function REPLServer(prompt,
e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/,
(_, pre, line) => pre + (line - 1));
}
top.outputStream.write((e.stack || e) + '\n');
top.outputStream.write((e.stack || e) + '\r\n');
top.lineParser.reset();
top.bufferedCommand = '';
top.lines.level = [];
Expand Down Expand Up @@ -453,7 +453,7 @@ function REPLServer(prompt,
sawSIGINT = false;
return;
}
self.output.write('(To exit, press ^C again or type .exit)\n');
self.output.write('(To exit, press ^C again or type .exit)\r\n');
sawSIGINT = true;
} else {
sawSIGINT = false;
Expand All @@ -470,7 +470,7 @@ function REPLServer(prompt,
sawSIGINT = false;

if (self.editorMode) {
self.bufferedCommand += cmd + '\n';
self.bufferedCommand += cmd + '\r\n';
return;
}

Expand All @@ -490,7 +490,7 @@ function REPLServer(prompt,
if (self.parseREPLKeyword(keyword, rest) === true) {
return;
} else if (!self.bufferedCommand) {
self.outputStream.write('Invalid REPL keyword\n');
self.outputStream.write('Invalid REPL keyword\r\n');
finish(null);
return;
}
Expand All @@ -509,8 +509,8 @@ function REPLServer(prompt,
self.wrappedCmd = false;
if (e && !self.bufferedCommand && cmd.trim().startsWith('npm ')) {
self.outputStream.write('npm should be run outside of the ' +
'node repl, in your normal shell.\n' +
'(Press Control-D to exit.)\n');
'node repl, in your normal shell.\r\n' +
'(Press Control-D to exit.)\r\n');
self.lineParser.reset();
self.bufferedCommand = '';
self.displayPrompt();
Expand All @@ -525,7 +525,7 @@ function REPLServer(prompt,
// {
// ... x: 1
// ... }
self.bufferedCommand += cmd + '\n';
self.bufferedCommand += cmd + '\r\n';
self.displayPrompt();
return;
} else {
Expand All @@ -548,7 +548,7 @@ function REPLServer(prompt,
if (!self.underscoreAssigned) {
self.last = ret;
}
self.outputStream.write(self.writer(ret) + '\n');
self.outputStream.write(self.writer(ret) + '\r\n');
}

// Display prompt again
Expand Down Expand Up @@ -578,10 +578,10 @@ function REPLServer(prompt,

self.on('SIGCONT', function() {
if (self.editorMode) {
self.outputStream.write(`${self._initialPrompt}.editor\n`);
self.outputStream.write(`${self._initialPrompt}.editor\r\n`);
self.outputStream.write(
'// Entering editor mode (^D to finish, ^C to cancel)\n');
self.outputStream.write(`${self.bufferedCommand}\n`);
'// Entering editor mode (^D to finish, ^C to cancel)\r\n');
self.outputStream.write(`${self.bufferedCommand}\r\n`);
self.prompt(true);
} else {
self.displayPrompt(true);
Expand Down Expand Up @@ -713,7 +713,7 @@ REPLServer.prototype.createContext = function() {
this.last = value;
if (!this.underscoreAssigned) {
this.underscoreAssigned = true;
this.outputStream.write('Expression assignment to _ now disabled.\n');
this.outputStream.write('Expression assignment to _ now disabled.\r\n');
}
}
});
Expand Down Expand Up @@ -762,7 +762,7 @@ function ArrayStream() {
this.run = function(data) {
var self = this;
data.forEach(function(line) {
self.emit('data', line + '\n');
self.emit('data', line + '\r\n');
});
};
}
Expand Down Expand Up @@ -1232,7 +1232,7 @@ function defineDefaultCommands(repl) {
this.lineParser.reset();
this.bufferedCommand = '';
if (!this.useGlobal) {
this.outputStream.write('Clearing context...\n');
this.outputStream.write('Clearing context...\r\n');
this.resetContext();
}
this.displayPrompt();
Expand All @@ -1252,7 +1252,7 @@ function defineDefaultCommands(repl) {
var self = this;
Object.keys(this.commands).sort().forEach(function(name) {
var cmd = self.commands[name];
self.outputStream.write(name + '\t' + (cmd.help || '') + '\n');
self.outputStream.write(name + '\t' + (cmd.help || '') + '\r\n');
});
this.displayPrompt();
}
Expand All @@ -1262,10 +1262,10 @@ function defineDefaultCommands(repl) {
help: 'Save all evaluated commands in this REPL session to a file',
action: function(file) {
try {
fs.writeFileSync(file, this.lines.join('\n') + '\n');
this.outputStream.write('Session saved to:' + file + '\n');
fs.writeFileSync(file, this.lines.join('\r\n') + '\r\n');
this.outputStream.write('Session saved to:' + file + '\r\n');
} catch (e) {
this.outputStream.write('Failed to save:' + file + '\n');
this.outputStream.write('Failed to save:' + file + '\r\n');
}
this.displayPrompt();
}
Expand All @@ -1279,19 +1279,21 @@ function defineDefaultCommands(repl) {
if (stats && stats.isFile()) {
var self = this;
var data = fs.readFileSync(file, 'utf8');
var lines = data.split('\n');
// \r\n, \n, or \r followed by something other than \n
const lineEnding = /\r?\n|\r(?!\n)/;
var lines = data.split(lineEnding);
this.displayPrompt();
lines.forEach(function(line) {
if (line) {
self.write(line + '\n');
self.write(line + '\r\n');
}
});
} else {
this.outputStream.write('Failed to load:' + file +
' is not a valid file\n');
' is not a valid file\r\n');
}
} catch (e) {
this.outputStream.write('Failed to load:' + file + '\n');
this.outputStream.write('Failed to load:' + file + '\r\n');
}
this.displayPrompt();
}
Expand All @@ -1304,7 +1306,7 @@ function defineDefaultCommands(repl) {
this.editorMode = true;
REPLServer.super_.prototype.setPrompt.call(this, '');
this.outputStream.write(
'// Entering editor mode (^D to finish, ^C to cancel)\n');
'// Entering editor mode (^D to finish, ^C to cancel)\r\n');
}
});
}
Expand Down
10 changes: 5 additions & 5 deletions lib/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,9 +836,9 @@ function reduceToSingleString(output, base, braces, breakLength) {
// If the opening "brace" is too large, like in the case of "Set {",
// we need to force the first item to be on the next line or the
// items will not line up correctly.
(base === '' && braces[0].length === 1 ? '' : base + '\n ') +
(base === '' && braces[0].length === 1 ? '' : base + '\r\n ') +
' ' +
output.join(',\n ') +
output.join(',\r\n ') +
' ' +
braces[1];
}
Expand Down Expand Up @@ -1001,19 +1001,19 @@ exports.print = internalUtil.deprecate(function() {

exports.puts = internalUtil.deprecate(function() {
for (var i = 0, len = arguments.length; i < len; ++i) {
process.stdout.write(arguments[i] + '\n');
process.stdout.write(arguments[i] + '\r\n');
}
}, 'util.puts is deprecated. Use console.log instead.');


exports.debug = internalUtil.deprecate(function(x) {
process.stderr.write('DEBUG: ' + x + '\n');
process.stderr.write('DEBUG: ' + x + '\r\n');
}, 'util.debug is deprecated. Use console.error instead.');


exports.error = internalUtil.deprecate(function(x) {
for (var i = 0, len = arguments.length; i < len; ++i) {
process.stderr.write(arguments[i] + '\n');
process.stderr.write(arguments[i] + '\r\n');
}
}, 'util.error is deprecated. Use console.error instead.');

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-preload.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const interactive = childProcess.exec(nodeBinary + ' '
+ '-i',
common.mustCall(function(err, stdout, stderr) {
assert.ifError(err);
assert.strictEqual(stdout, `> 'test'\n> `);
assert.strictEqual(stdout, `> 'test'\r\n> `);
}));

interactive.stdin.write('a\n');
Expand Down
9 changes: 5 additions & 4 deletions test/parallel/test-repl-.save.load.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ putIn.run(testFile);
putIn.run(['.save ' + saveFileName]);

// the file should have what I wrote
assert.equal(fs.readFileSync(saveFileName, 'utf8'), testFile.join('\n') + '\n');
assert.equal(fs.readFileSync(saveFileName, 'utf8'), testFile.join('\r\n')
+ '\r\n');

// make sure that the REPL data is "correct"
// so when I load it back I know I'm good
Expand All @@ -54,7 +55,7 @@ var loadFile = join(common.tmpDir, 'file.does.not.exist');
// should not break
putIn.write = function(data) {
// make sure I get a failed to load message and not some crazy error
assert.equal(data, 'Failed to load:' + loadFile + '\n');
assert.equal(data, 'Failed to load:' + loadFile + '\r\n');
// eat me to avoid work
putIn.write = function() {};
};
Expand All @@ -63,7 +64,7 @@ putIn.run(['.load ' + loadFile]);
// throw error on loading directory
loadFile = common.tmpDir;
putIn.write = function(data) {
assert.equal(data, 'Failed to load:' + loadFile + ' is not a valid file\n');
assert.equal(data, 'Failed to load:' + loadFile + ' is not a valid file\r\n');
putIn.write = function() {};
};
putIn.run(['.load ' + loadFile]);
Expand All @@ -78,7 +79,7 @@ const invalidFileName = join(common.tmpDir, '\0\0\0\0\0');
// should not break
putIn.write = function(data) {
// make sure I get a failed to save message and not some other error
assert.equal(data, 'Failed to save:' + invalidFileName + '\n');
assert.equal(data, 'Failed to save:' + invalidFileName + '\r\n');
// reset to no-op
putIn.write = function() {};
};
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-repl-autolibs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function test1() {
if (data.length) {

// inspect output matches repl output
assert.equal(data, util.inspect(require('fs'), null, 2, false) + '\n');
assert.equal(data, util.inspect(require('fs'), null, 2, false) + '\r\n');
// globally added lib matches required lib
assert.equal(global.fs, require('fs'));
test2();
Expand All @@ -36,7 +36,7 @@ function test2() {
gotWrite = true;
if (data.length) {
// repl response error message
assert.equal(data, '{}\n');
assert.equal(data, '{}\r\n');
// original value wasn't overwritten
assert.equal(val, global.url);
}
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-repl-definecommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ r.defineCommand('say2', function() {
this.displayPrompt();
});

inputStream.write('.help\n');
assert(/\nsay1\thelp for say1\n/.test(output), 'help for say1 not present');
assert(/\nsay2\t\n/.test(output), 'help for say2 not present');
inputStream.write('.say1 node developer\n');
inputStream.write('.help\r\n');
assert(/\r\nsay1\thelp for say1\r\n/.test(output), 'help for say1 not present');
assert(/\r\nsay2\t\r\n/.test(output), 'help for say2 not present');
inputStream.write('.say1 node developer\r\n');
assert(/> hello node developer/.test(output), 'say1 outputted incorrectly');
inputStream.write('.say2 node developer\n');
inputStream.write('.say2 node developer\r\n');
assert(/> hello from say2/.test(output), 'say2 outputted incorrectly');
22 changes: 11 additions & 11 deletions test/parallel/test-repl-mode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,45 @@ function testSloppyMode() {

cli.input.emit('data', `
x = 3
`.trim() + '\n');
assert.equal(cli.output.accumulator.join(''), '> 3\n> ');
`.trim() + '\r\n');
assert.equal(cli.output.accumulator.join(''), '> 3\r\n> ');
cli.output.accumulator.length = 0;

cli.input.emit('data', `
let y = 3
`.trim() + '\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\n> ');
`.trim() + '\r\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\r\n> ');
}

function testStrictMode() {
var cli = initRepl(repl.REPL_MODE_STRICT);

cli.input.emit('data', `
x = 3
`.trim() + '\n');
`.trim() + '\r\n');
assert.ok(/ReferenceError: x is not defined/.test(
cli.output.accumulator.join('')));
cli.output.accumulator.length = 0;

cli.input.emit('data', `
let y = 3
`.trim() + '\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\n> ');
`.trim() + '\r\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\r\n> ');
}

function testAutoMode() {
var cli = initRepl(repl.REPL_MODE_MAGIC);

cli.input.emit('data', `
x = 3
`.trim() + '\n');
assert.equal(cli.output.accumulator.join(''), '> 3\n> ');
`.trim() + '\r\n');
assert.equal(cli.output.accumulator.join(''), '> 3\r\n> ');
cli.output.accumulator.length = 0;

cli.input.emit('data', `
let y = 3
`.trim() + '\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\n> ');
`.trim() + '\r\n');
assert.equal(cli.output.accumulator.join(''), 'undefined\r\n> ');
}

function initRepl(mode) {
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-repl-persistent-history.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ActionStream extends stream.Stream {
if (typeof action === 'object') {
self.emit('keypress', '', action);
} else {
self.emit('data', action + '\n');
self.emit('data', action + '\r\n');
}
setImmediate(doAction);
}
Expand Down Expand Up @@ -138,7 +138,7 @@ const tests = [
env: { NODE_REPL_HISTORY_FILE: oldHistoryPath },
test: [UP, CLEAR, '\'42\'', ENTER],
expected: [prompt, convertMsg, prompt, prompt + '\'=^.^=\'', prompt, '\'',
'4', '2', '\'', '\'42\'\n', prompt, prompt],
'4', '2', '\'', '\'42\'\r\n', prompt, prompt],
after: function ensureHistoryFixture() {
// XXX(Fishrock123) Make sure nothing weird happened to our fixture
// or it's temporary copy.
Expand All @@ -154,7 +154,7 @@ const tests = [
{ // Requires the above testcase
env: {},
test: [UP, UP, ENTER],
expected: [prompt, prompt + '\'42\'', prompt + '\'=^.^=\'', '\'=^.^=\'\n',
expected: [prompt, prompt + '\'42\'', prompt + '\'=^.^=\'', '\'=^.^=\'\r\n',
prompt]
},
{
Expand Down
Loading

0 comments on commit fce4b98

Please sign in to comment.