-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
repl: handle comments properly #3515
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,88 @@ const BLOCK_SCOPED_ERROR = 'Block-scoped declarations (let, ' + | |
'const, function, class) not yet supported outside strict mode'; | ||
|
||
|
||
class LineParser { | ||
|
||
constructor() { | ||
this.reset(); | ||
} | ||
|
||
reset() { | ||
this._literal = null; | ||
this.shouldFail = false; | ||
this.blockComment = false; | ||
} | ||
|
||
parseLine(line) { | ||
var previous = null; | ||
this.shouldFail = false; | ||
const wasWithinStrLiteral = this._literal !== null; | ||
|
||
for (const current of line) { | ||
if (previous === '\\') { | ||
// valid escaping, skip processing. previous doesn't matter anymore | ||
previous = null; | ||
continue; | ||
} | ||
|
||
if (!this._literal) { | ||
if (previous === '*' && current === '/') { | ||
if (this.blockComment) { | ||
this.blockComment = false; | ||
previous = null; | ||
continue; | ||
} else { | ||
this.shouldFail = true; | ||
break; | ||
} | ||
} | ||
|
||
// ignore rest of the line if `current` and `previous` are `/`s | ||
if (previous === current && previous === '/' && !this.blockComment) { | ||
break; | ||
} | ||
|
||
if (previous === '/' && current === '*') { | ||
this.blockComment = true; | ||
previous = null; | ||
} | ||
} | ||
|
||
if (this.blockComment) continue; | ||
|
||
if (current === this._literal) { | ||
this._literal = null; | ||
} else if (current === '\'' || current === '"') { | ||
this._literal = this._literal || current; | ||
} | ||
|
||
previous = current; | ||
} | ||
|
||
const isWithinStrLiteral = this._literal !== null; | ||
|
||
if (!wasWithinStrLiteral && !isWithinStrLiteral) { | ||
// Current line has nothing to do with String literals, trim both ends | ||
line = line.trim(); | ||
} else if (wasWithinStrLiteral && !isWithinStrLiteral) { | ||
// was part of a string literal, but it is over now, trim only the end | ||
line = line.trimRight(); | ||
} else if (isWithinStrLiteral && !wasWithinStrLiteral) { | ||
// was not part of a string literal, but it is now, trim only the start | ||
line = line.trimLeft(); | ||
} | ||
|
||
const lastChar = line.charAt(line.length - 1); | ||
|
||
this.shouldFail = this.shouldFail || | ||
((!this._literal && lastChar === '\\') || | ||
(this._literal && lastChar !== '\\')); | ||
|
||
return line; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be simpler as a generator ala the readline parser? That way we can pause on comments for more data. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Fishrock123 You mean like yielding tokens one at a time? We don't actually use the tokens as such, right? We just parse the line and find out if it has a string literal or a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eh, sounds fine. |
||
} | ||
|
||
|
||
function REPLServer(prompt, | ||
stream, | ||
eval_, | ||
|
@@ -193,7 +275,7 @@ function REPLServer(prompt, | |
debug('domain error'); | ||
const top = replMap.get(self); | ||
top.outputStream.write((e.stack || e) + '\n'); | ||
top._currentStringLiteral = null; | ||
top.lineParser.reset(); | ||
top.bufferedCommand = ''; | ||
top.lines.level = []; | ||
top.displayPrompt(); | ||
|
@@ -220,8 +302,7 @@ function REPLServer(prompt, | |
self.outputStream = output; | ||
|
||
self.resetContext(); | ||
// Initialize the current string literal found, to be null | ||
self._currentStringLiteral = null; | ||
self.lineParser = new LineParser(); | ||
self.bufferedCommand = ''; | ||
self.lines.level = []; | ||
|
||
|
@@ -280,87 +361,22 @@ function REPLServer(prompt, | |
sawSIGINT = false; | ||
} | ||
|
||
self._currentStringLiteral = null; | ||
self.lineParser.reset(); | ||
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 and the previous | ||
// character doesn't matter anymore. | ||
previous = null; | ||
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 = self.lineParser.parseLine(cmd); | ||
} | ||
|
||
// Check to see if a REPL keyword was used. If it returns true, | ||
|
@@ -393,9 +409,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) { | ||
|
@@ -406,15 +422,15 @@ 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.lineParser.reset(); | ||
self.bufferedCommand = ''; | ||
self.displayPrompt(); | ||
return; | ||
} | ||
|
||
// If error was SyntaxError and not JSON.parse error | ||
if (e) { | ||
if (e instanceof Recoverable) { | ||
if (e instanceof Recoverable && !self.lineParser.shouldFail) { | ||
// Start buffering data like that: | ||
// { | ||
// ... x: 1 | ||
|
@@ -423,12 +439,12 @@ function REPLServer(prompt, | |
self.displayPrompt(); | ||
return; | ||
} else { | ||
self._domain.emit('error', e); | ||
self._domain.emit('error', e.err || e); | ||
} | ||
} | ||
|
||
// Clear buffer if no SyntaxErrors | ||
self._currentStringLiteral = null; | ||
self.lineParser.reset(); | ||
self.bufferedCommand = ''; | ||
|
||
// If we got any output - print it (if no error) | ||
|
@@ -985,7 +1001,7 @@ function defineDefaultCommands(repl) { | |
repl.defineCommand('break', { | ||
help: 'Sometimes you get stuck, this gets you out', | ||
action: function() { | ||
this._currentStringLiteral = null; | ||
this.lineParser.reset(); | ||
this.bufferedCommand = ''; | ||
this.displayPrompt(); | ||
} | ||
|
@@ -1000,7 +1016,7 @@ function defineDefaultCommands(repl) { | |
repl.defineCommand('clear', { | ||
help: clearMessage, | ||
action: function() { | ||
this._currentStringLiteral = null; | ||
this.lineParser.reset(); | ||
this.bufferedCommand = ''; | ||
if (!this.useGlobal) { | ||
this.outputStream.write('Clearing context...\n'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is
for (... of ...)
on a String? I was unaware that worked.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fishrock123 It actually works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works and has the advantage to handle unicode correctly: