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.

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 eval() with context ignored in 10.2.0+ #20965

Closed
jdalton opened this issue May 25, 2018 · 3 comments
Closed

Repl eval() with context ignored in 10.2.0+ #20965

jdalton opened this issue May 25, 2018 · 3 comments
Labels
regression Issues related to regressions. repl Issues and PRs related to the REPL subsystem.

Comments

@jdalton
Copy link
Member

jdalton commented May 25, 2018

The commit here 9aa4ec4 has caused default repl#eval to ignore the context parameter passed to it. The issue was introduced in PR #20617 (issue #19021).

@addaleax addaleax added repl Issues and PRs related to the REPL subsystem. regression Issues related to regressions. labels May 25, 2018
@addaleax
Copy link
Member

I’d just revert the patch for now. Thoughts?

@jdalton
Copy link
Member Author

jdalton commented May 25, 2018

IMO since the bit reverted would be just extra hints a revert would be ok.
Then it could be retooled and merged at a later time.

cjihrig added a commit to cjihrig/node that referenced this issue May 28, 2018
This reverts commit 9aa4ec4.

This commit in question introduced a regression in repl.eval(),
as the context argument is no longer passed to runInContext().

PR-URL: nodejs#20972
Fixes: nodejs#20965
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
@jdalton
Copy link
Member Author

jdalton commented May 28, 2018

Thank you @cjihrig!

MylesBorins pushed a commit that referenced this issue May 29, 2018
This reverts commit 9aa4ec4.

This commit in question introduced a regression in repl.eval(),
as the context argument is no longer passed to runInContext().

PR-URL: #20972
Fixes: #20965
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Issues related to regressions. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants