Skip to content

Commit

Permalink
repl: revert 81ea52a to fix newlines in copy-paste
Browse files Browse the repository at this point in the history
When some string is copied and pasted in REPL, if it has a newline (\n)
in it, then it is considered as end of line and REPL throws a
SyntaxError, because a line cannot end with a unclosed string literal.
This behavior is because of the fact that `readline` module breaks at
all the `\n`s in the input string and treats them as a seperate line.
So whenever it encounters a new line character it will emit a `line`
event.

This commit basically reverts
nodejs@81ea52a,
which was an attempt to improve the way string literals were handled
by REPL.

Fixes: nodejs#2749
  • Loading branch information
thefourtheye committed Sep 18, 2015
1 parent e29e470 commit 6742c9b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 102 deletions.
90 changes: 15 additions & 75 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ function REPLServer(prompt,
self._domain.on('error', function(e) {
debug('domain error');
self.outputStream.write((e.stack || e) + '\n');
self._currentStringLiteral = null;
self.bufferedCommand = '';
self.lines.level = [];
self.displayPrompt();
Expand All @@ -217,8 +216,6 @@ function REPLServer(prompt,
self.outputStream = output;

self.resetContext();
// Initialize the current string literal found, to be null
self._currentStringLiteral = null;
self.bufferedCommand = '';
self.lines.level = [];

Expand Down Expand Up @@ -277,86 +274,21 @@ function REPLServer(prompt,
sawSIGINT = false;
}

self._currentStringLiteral = null;
self.bufferedCommand = '';
self.lines.level = [];
self.displayPrompt();
});

function parseLine(line, currentStringLiteral) {
var previous = null, current = null;

for (var i = 0; i < line.length; i += 1) {
if (previous === '\\') {
// if it is a valid escaping, then skip processing
previous = current;
continue;
}

current = line.charAt(i);
if (current === currentStringLiteral) {
currentStringLiteral = null;
} else if (current === '\'' ||
current === '"' &&
currentStringLiteral === null) {
currentStringLiteral = current;
}
previous = current;
}

return currentStringLiteral;
}

function getFinisherFunction(cmd, defaultFn) {
if ((self._currentStringLiteral === null &&
cmd.charAt(cmd.length - 1) === '\\') ||
(self._currentStringLiteral !== null &&
cmd.charAt(cmd.length - 1) !== '\\')) {

// If the line continuation is used outside string literal or if the
// string continuation happens with out line continuation, then fail hard.
// Even if the error is recoverable, get the underlying error and use it.
return function(e, ret) {
var error = e instanceof Recoverable ? e.err : e;

if (arguments.length === 2) {
// using second argument only if it is actually passed. Otherwise
// `undefined` will be printed when invalid REPL commands are used.
return defaultFn(error, ret);
}

return defaultFn(error);
};
}
return defaultFn;
}

self.on('line', function(cmd) {
debug('line %j', cmd);
sawSIGINT = false;
var skipCatchall = false;
var finisherFn = finish;

// leading whitespaces in template literals should not be trimmed.
if (self._inTemplateLiteral) {
self._inTemplateLiteral = false;
} else {
const wasWithinStrLiteral = self._currentStringLiteral !== null;
self._currentStringLiteral = parseLine(cmd, self._currentStringLiteral);
const isWithinStrLiteral = self._currentStringLiteral !== null;

if (!wasWithinStrLiteral && !isWithinStrLiteral) {
// Current line has nothing to do with String literals, trim both ends
cmd = cmd.trim();
} else if (wasWithinStrLiteral && !isWithinStrLiteral) {
// was part of a string literal, but it is over now, trim only the end
cmd = cmd.trimRight();
} else if (isWithinStrLiteral && !wasWithinStrLiteral) {
// was not part of a string literal, but it is now, trim only the start
cmd = cmd.trimLeft();
}

finisherFn = getFinisherFunction(cmd, finish);
cmd = trimWhitespace(cmd);
}

// Check to see if a REPL keyword was used. If it returns true,
Expand Down Expand Up @@ -389,9 +321,9 @@ function REPLServer(prompt,
}

debug('eval %j', evalCmd);
self.eval(evalCmd, self.context, 'repl', finisherFn);
self.eval(evalCmd, self.context, 'repl', finish);
} else {
finisherFn(null);
finish(null);
}

function finish(e, ret) {
Expand All @@ -402,7 +334,6 @@ function REPLServer(prompt,
self.outputStream.write('npm should be run outside of the ' +
'node repl, in your normal shell.\n' +
'(Press Control-D to exit.)\n');
self._currentStringLiteral = null;
self.bufferedCommand = '';
self.displayPrompt();
return;
Expand All @@ -424,7 +355,6 @@ function REPLServer(prompt,
}

// Clear buffer if no SyntaxErrors
self._currentStringLiteral = null;
self.bufferedCommand = '';

// If we got any output - print it (if no error)
Expand Down Expand Up @@ -965,7 +895,6 @@ function defineDefaultCommands(repl) {
repl.defineCommand('break', {
help: 'Sometimes you get stuck, this gets you out',
action: function() {
this._currentStringLiteral = null;
this.bufferedCommand = '';
this.displayPrompt();
}
Expand All @@ -980,7 +909,6 @@ function defineDefaultCommands(repl) {
repl.defineCommand('clear', {
help: clearMessage,
action: function() {
this._currentStringLiteral = null;
this.bufferedCommand = '';
if (!this.useGlobal) {
this.outputStream.write('Clearing context...\n');
Expand Down Expand Up @@ -1046,6 +974,18 @@ function defineDefaultCommands(repl) {
});
}


function trimWhitespace(cmd) {
const trimmer = /^\s*(.+)\s*$/m;
var matches = trimmer.exec(cmd);

if (matches && matches.length === 2) {
return matches[1];
}
return '';
}


function regexpEscape(s) {
return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&');
}
Expand Down
27 changes: 0 additions & 27 deletions test/parallel/test-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,33 +198,6 @@ function error_test() {
// a REPL command
{ client: client_unix, send: '.toString',
expect: 'Invalid REPL keyword\n' + prompt_unix },
// fail when we are not inside a String and a line continuation is used
{ client: client_unix, send: '[] \\',
expect: /^SyntaxError: Unexpected token ILLEGAL/ },
// do not fail when a String is created with line continuation
{ client: client_unix, send: '\'the\\\nfourth\\\neye\'',
expect: prompt_multiline + prompt_multiline +
'\'thefourtheye\'\n' + prompt_unix },
// Don't fail when a partial String is created and line continuation is used
// with whitespace characters at the end of the string. We are to ignore it.
// This test is to make sure that we properly remove the whitespace
// characters at the end of line, unlike the buggy `trimWhitespace` function
{ client: client_unix, send: ' \t .break \t ',
expect: prompt_unix },
// multiline strings preserve whitespace characters in them
{ client: client_unix, send: '\'the \\\n fourth\t\t\\\n eye \'',
expect: prompt_multiline + prompt_multiline +
'\'the fourth\\t\\t eye \'\n' + prompt_unix },
// more than one multiline strings also should preserve whitespace chars
{ client: client_unix, send: '\'the \\\n fourth\' + \'\t\t\\\n eye \'',
expect: prompt_multiline + prompt_multiline +
'\'the fourth\\t\\t eye \'\n' + prompt_unix },
// using REPL commands within a string literal should still work
{ client: client_unix, send: '\'\\\n.break',
expect: prompt_unix },
// using REPL command "help" within a string literal should still work
{ client: client_unix, send: '\'thefourth\\\n.help\neye\'',
expect: /'thefourtheye'/ },
// empty lines in the REPL should be allowed
{ client: client_unix, send: '\n\r\n\r\n',
expect: prompt_unix + prompt_unix + prompt_unix },
Expand Down

0 comments on commit 6742c9b

Please sign in to comment.