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: Use displayErrors for SyntaxError #7589

Closed
wants to merge 3 commits into from

Conversation

princejwesley
Copy link
Contributor

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

repl

Description of change

Enabling vm's displayErrors option to display code error with caret(^) sign

node 🙈  git:(upstream  display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number
    at Object.exports.createScript (vm.js:47:10)
    at REPLServer.defaultEval (repl.js:255:25)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:495:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
    at REPLServer.Interface._onLine (readline.js:231:10)
    at REPLServer.Interface._line (readline.js:573:8)
    at REPLServer.Interface._ttyWrite (readline.js:850:14)
>

@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Jul 7, 2016
@rvagg
Copy link
Member

rvagg commented Jul 8, 2016

seems good to me, although I'm wondering if there was a good reason we don't do this already

@Fishrock123
Copy link
Contributor

@Fishrock123
Copy link
Contributor

Is the stack-trace ever going to be useful here?

@princejwesley
Copy link
Contributor Author

@Fishrock123 May be needed for .load command or require calls. Have to check it!

@princejwesley
Copy link
Contributor Author

@Fishrock123 Without stack trace

node 🙈  git:(upstream  display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number

> require('/Users/princejohnwesley/Downloads/test.js')
/Users/princejohnwesley/Downloads/test.js:3
var 4;
    ^
SyntaxError: Unexpected number

> .load /Users/princejohnwesley/Downloads/test.js
> var 4;
var 4;
    ^
SyntaxError: Unexpected number

>

```js
node 🙈 ₹ git:(upstream ⚡ display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number
    at Object.exports.createScript (vm.js:47:10)
    at REPLServer.defaultEval (repl.js:255:25)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:495:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
    at REPLServer.Interface._onLine (readline.js:231:10)
    at REPLServer.Interface._line (readline.js:573:8)
    at REPLServer.Interface._ttyWrite (readline.js:850:14)
>
```
@Trott Trott added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 15, 2016
@jasnell
Copy link
Member

jasnell commented Jul 20, 2016

LGTM if CI is green

@princejwesley
Copy link
Contributor Author

@princejwesley
Copy link
Contributor Author

Landed in 68ac0d0d

princejwesley added a commit that referenced this pull request Jul 27, 2016
```js
node 🙈 ₹ git:(upstream ⚡ display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number

>
```

PR-URL: #7589
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@cjihrig cjihrig mentioned this pull request Aug 8, 2016
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
```js
node 🙈 ₹ git:(upstream ⚡ display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number

>
```

PR-URL: #7589
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>

Conflicts:
	test/parallel/test-repl.js
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants