Skip to content
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.

Sign up for GitHub

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: refactor repl.js #6071

Closed
wants to merge 1 commit into from
Closed

repl: refactor repl.js #6071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ function REPLServer(prompt,
self.on('line', function(cmd) {
debug('line %j', cmd);
sawSIGINT = false;
var skipCatchall = false;

// leading whitespaces in template literals should not be trimmed.
if (self._inTemplateLiteral) {
Expand All @@ -417,11 +416,12 @@ function REPLServer(prompt,
return;
} else if (!self.bufferedCommand) {
self.outputStream.write('Invalid REPL keyword\n');
skipCatchall = true;
finish(null);
return;
}
}

if (!skipCatchall && (cmd || (!cmd && self.bufferedCommand))) {
if (cmd || self.bufferedCommand) {
var evalCmd = self.bufferedCommand + cmd;
if (/^\s*\{/.test(evalCmd) && /\}\s*$/.test(evalCmd)) {
// It's confusing for `{ a : 1 }` to be interpreted as a block
Expand Down Expand Up @@ -1022,7 +1022,7 @@ REPLServer.prototype.memory = function memory(cmd) {
// self.lines.level.length === 0
// TODO? keep a log of level so that any syntax breaking lines can
// be cleared on .break and in the case of a syntax error?
// TODO? if a log was kept, then I could clear the bufferedComand and
// TODO? if a log was kept, then I could clear the bufferedCommand and
// eval these lines and throw the syntax error
} else {
self.lines.level = [];
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-repl-null.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';
require('../common');
const repl = require('repl');
const assert = require('assert');

var replserver = new repl.REPLServer();

replserver._inTemplateLiteral = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Trott do you still remember why you added this here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember why I did it, but I do not remember why I chose that over possible alternatives.

I did that because this is going to test that null inside a template string gets treated like an empty string. (See the code comment below.)

It's possible that I was being lazy and/or copying from another test. It may be better to open an actual template string by emiting a backtick in a line. Or it's possible that I tried that and ran into problems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and I was testing this particular corner case because I wanted to make sure that my refactoring the code didn't alter the behavior. And this seemed to me like odd behavior for which we had no tests.


// `null` gets treated like an empty string. (Should it? You have to do some
// strange business to get it into the REPL. Maybe it should really throw?)

assert.doesNotThrow(() => {
replserver.emit('line', null);
});

replserver.emit('line', '.exit');