repl: Empty command should be sent to eval function - #11871
Conversation
Sorry, something went wrong.
This fixes a regression introduced in nodejs#6171
|
Verified against node-inspect: > ../node/node cli.js examples/empty.js
< Debugger listening on port 9229.
< Warning: This is an experimental feature and could change at any time.
< To start debugging, open the following URL in Chrome:
< chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=127.0.0.1:9229/2f87b9e8-e570-4878-b33e-4858c0d7f7a0
< Debugger attached.
break in examples/empty.js:2
1 (function (exports, require, module, __filename, __dirname) {
> 2 });
debug> ["hello", "world"].join(" ")
'hello world'
debug>
'hello world'
debug>
On current master, the 2nd 'hello world' isn't printed. |
Sorry, something went wrong.
Sorry, something went wrong.
|
🍏 |
Sorry, something went wrong.
addaleax
left a comment
There was a problem hiding this comment.
LGTM with a suggestion
Sorry, something went wrong.
|
|
||
| process.on('exit', () => { | ||
| assert(evalCalledWithExpectedArgs); | ||
| }); |
There was a problem hiding this comment.
I think you don’t need for the process to exit, the eval callback should be called synchronousl
Sorry, something went wrong.
There was a problem hiding this comment.
I can simplify the test file, just wrote it this way to match the existing ones (cp test/parallel/test-repl-{eval,empty}.js).
Sorry, something went wrong.
|
Depends on a prior semver-major commit. Marking do-not-land on lower versions. |
Sorry, something went wrong.
The `process.on("exit")` event handlers are unnecessary, so it’s okay
to drop them.
Ref: nodejs#11871
This fixes a regression introduced in nodejs#6171 PR-URL: nodejs#11871 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The `process.on("exit")` event handlers are unnecessary, so it’s okay
to drop them.
PR-URL: #11946
Ref: #11871
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This fixes a regression introduced in #6171
The command line debugger relies on the previous behavior. This adds a regression test and restores it.
Checklist
Affected core subsystem(s)
repl