-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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: fix repl .load #28608
repl: fix repl .load #28608
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 |
---|---|---|
|
@@ -1535,10 +1535,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('\n')); | ||
this.outputStream.write(`Session saved to: ${file}\n`); | ||
} catch { | ||
this.outputStream.write('Failed to save: ' + file + '\n'); | ||
this.outputStream.write(`Failed to save: ${file}\n`); | ||
} | ||
this.displayPrompt(); | ||
} | ||
|
@@ -1548,23 +1548,20 @@ function defineDefaultCommands(repl) { | |
help: 'Load JS from a file into the REPL session', | ||
action: function(file) { | ||
try { | ||
var stats = fs.statSync(file); | ||
const stats = fs.statSync(file); | ||
if (stats && stats.isFile()) { | ||
_turnOnEditorMode(this); | ||
var data = fs.readFileSync(file, 'utf8'); | ||
var lines = data.split('\n'); | ||
for (var n = 0; n < lines.length; n++) { | ||
if (lines[n]) | ||
this.write(`${lines[n]}\n`); | ||
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. The changed behavior here is also a bugfix: in case the saved load would be a template string with multiple line breaks, it would have changed the users data. That is not good and we should always load all data 1-to-1. |
||
} | ||
const data = fs.readFileSync(file, 'utf8'); | ||
this.write(data); | ||
_turnOffEditorMode(this); | ||
this.write('\n'); | ||
} else { | ||
this.outputStream.write('Failed to load: ' + file + | ||
' is not a valid file\n'); | ||
this.outputStream.write( | ||
`Failed to load: ${file} is not a valid file\n` | ||
); | ||
} | ||
} catch { | ||
this.outputStream.write('Failed to load: ' + file + '\n'); | ||
this.outputStream.write(`Failed to load: ${file}\n`); | ||
} | ||
this.displayPrompt(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ const expected = `${command} | |
const getLunch = () => | ||
placeOrder('tacos') | ||
.then(eat); | ||
|
||
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. Superfluous change? 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. No, that's the bug fix mentioned here: #28608 (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. Ah, we're in a string template here, which wasn't obvious from the GitHub interface. I see now. |
||
const placeOrder = (order) => Promise.resolve(order); | ||
const eat = (food) => '<nom nom nom>'; | ||
|
||
|
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.
The originally added line break caused the complete function not to work as expected. Saving should just plainly save exactly the users data and not add the extra line break, since that's not what the user actually did.