This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Don't use a separate context for the repl.
Fix #1484 Fix #1834 Fix #1482 Fix #771 It's been a while now, and we've seen how this separate context thing works. It constantly confuses people, and no one actually uses '.clear' anyway, so the benefit of that feature does not justify the constant WTFery. This makes repl.context actually be a getter that returns the global object, and prints a deprecation warning. The '.clear' command is gone, and will report that it's an invalid repl keyword. Tests updated to allow the require, module, and exports globals, which are still available in the repl just like they were before, by making them global.
- Loading branch information
Showing
3 changed files
with
40 additions
and
103 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b70fed4
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 commits brokes debugger's
repl
command: now every command will end up with crash,.scope
request was removed and everything.b70fed4
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.
Review? https://github.com/isaacs/node/tree/repl-revert
b70fed4
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.
Is there a way to fix the debugger without adding this back?
b70fed4
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.
yes there are one: adding
next
and etc toglobals
object.Are you argree with that?
b70fed4
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.
I'm totally fine with adding the flag to use the global as in https://github.com/isaacs/node/tree/repl-revert. Or we can look at @indutny's suggestion. As long as the normal
node
cli repl runs commands in the global space, I'm totally fine with whatever. Just want to make the n00b wtf's stop.b70fed4
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.
+1 to @isaacs
b70fed4
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.
@ry that was too concise :P
can you be clearer? globals or reverting?
b70fed4
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.
i discussed with isaacs earlier today - his repl-revert sounds good to me.