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: Proposal for better repl (implementation) #8504

Closed
wants to merge 2 commits into from

Conversation

princejwesley
Copy link
Contributor

@princejwesley princejwesley commented Sep 12, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

repl

Description of change
  • Welcome message with version and help guide
    • displayWelcomeMessage flag is used to
      turn on/off
  • Differentiate execute & continue actions
    • ^M or enter key to execute the command
    • ^J to continue building multiline expression.
    • executeOnTimeout value is used to determine
      the end of expression when terminal is false.
  • Pretty stack trace.
    • REPL specific stack frames are removed before
      emitting to output stream.
  • Recoverable errors.
    • No more recoverable errors & no false positives.
  • Defined commands(like .exit, .load)
    • Meaningful only at the top level.
    • .break is removed - no more get stuck
    • .clear is available only for local context

Welcome message template

$ node
Welcome to Node.js <<version>> (<<vm name>> VM, <<vm version>>)
Type ^M or enter to execute, ^J to continue, ^C to exit
Or try .help for help, more at https://nodejs.org/dist/<<version>>/docs/api/repl.html

>

Pretty stack trace

$ node -i
> throw new Error('tiny stack')
Error: tiny stack
    at repl:1:7
> var x y;
var x y;
      ^
SyntaxError: Unexpected identifier
>

Refs: #8195

~~~TODO~~~

  • Rebase from master
  • Fix flaky test cases
  • Test in windows machine

@nodejs-github-bot nodejs-github-bot added debugger readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. labels Sep 12, 2016
@addaleax addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 12, 2016
@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@princejwesley princejwesley force-pushed the repl.enhanced branch 5 times, most recently from 4b2370c to dd903bf Compare October 29, 2016 20:27
@princejwesley princejwesley changed the title [WIP] repl: Proposal for better repl (implementation) repl: Proposal for better repl (implementation) Nov 7, 2016
@princejwesley
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/4794/

CC: @nodejs/collaborators

* Welcome message with version and help guide
    * `displayWelcomeMessage` flag is used to
      turn on/off
* Differentiate execute & continue actions
    * ^M or enter key to execute the command
    * ^J to continue building multiline expression.
    * `executeOnTimeout` value is used to determine
      the end of expression when `terminal` is false.
* Pretty stack trace.
    * REPL specific stack frames are removed before
      emitting to output stream.
* Recoverable errors.
    * No more recoverable errors & no false positives.
* Defined commands(like .exit, .load) are meaningful
  only at the top level.
* Remove `.break` command and `.clear`when `useGlobal`
  is false.

Welcome message template
------------------------
```js
$ node
Welcome to Node.js <<version>> (<<vm name>> VM, <<vm version>>)
Type ^M or enter to execute, ^J to continue, ^C to exit
Or try

```

Pretty stack trace
------------------
```js
$ node -i
> throw new Error('tiny stack')
Error: tiny stack
    at repl:1:7
> var x y;
var x y;
      ^
SyntaxError: Unexpected identifier
>
```
@benjamingr
Copy link
Member

The changes here are pretty extensive, are you sure you don't want to split this into several smaller PRs? It's pretty hard to review the different functionality changes this way.

I also think you probably want to elaborate on why magic mode was removed.

I'm generally +1 on the changes but I think this is significant enough to discuss in a CTC meeting.

@princejwesley
Copy link
Contributor Author

@benjamingr 90% of the changes are adding/fixing tests. I don't know how to split it into smaller PRs. Magic mode has no effect and is discussed in #7850

@princejwesley
Copy link
Contributor Author

@addaleax @Fishrock123 @thefourtheye Have a look!

@princejwesley
Copy link
Contributor Author

Closing it. I'll create a new PR so that collaborators can have a look from recent list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants