Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

add CLI option to syntax check script, fixes #9426 #9447

Closed
wants to merge 2 commits into from
Closed

add CLI option to syntax check script, fixes #9426 #9447

wants to merge 2 commits into from

Conversation

bahamas10
Copy link

discussion in #9426

This is my first contribution to node core so I may have missed a few files when trying to add this functionality... but in my testing everything worked as expected.

$ cat ./good.js
var a = 'foo';
$ ./out/Release/node -c ./good.js
$ echo $?
0
$ cat ./bad.js
var a foo;
$ ./out/Release/node -c ./bad.js
/Users/dave/dev/node/bad.js:1
var a foo;
      ^^^
SyntaxError: Unexpected identifier
    at startup (node.js:104:54)
    at node.js:826:3
$ echo $?
1

@OrangeDog
Copy link

Someone will want some tests submitted.

@bahamas10
Copy link
Author

@OrangeDog tests added... example output below:

dave - manilla darwin ~/dev/node/test/simple (git:dave-1426873415) $ ../../out/Release/node test-cli-syntax.js 
calling /Users/dave/dev/node/out/Release/node -c /Users/dave/dev/node/test/fixtures/syntax/good_syntax.js
ok
calling /Users/dave/dev/node/out/Release/node --check /Users/dave/dev/node/test/fixtures/syntax/good_syntax.js
ok
calling /Users/dave/dev/node/out/Release/node -c /Users/dave/dev/node/test/fixtures/syntax/good_syntax_shebang.js
ok
calling /Users/dave/dev/node/out/Release/node --check /Users/dave/dev/node/test/fixtures/syntax/good_syntax_shebang.js
ok
calling /Users/dave/dev/node/out/Release/node -c /Users/dave/dev/node/test/fixtures/syntax/bad_syntax.js
ok
calling /Users/dave/dev/node/out/Release/node --check /Users/dave/dev/node/test/fixtures/syntax/bad_syntax.js
ok
calling /Users/dave/dev/node/out/Release/node -c /Users/dave/dev/node/test/fixtures/syntax/bad_syntax_shebang.js
ok
calling /Users/dave/dev/node/out/Release/node --check /Users/dave/dev/node/test/fixtures/syntax/bad_syntax_shebang.js
ok

@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

@bahamas10 .. thank you for this. At this point, because we are working on migrating active development to http://github.com/nodejs/node, this is not something that we would land here. The best thing would be to close this PR and open a new updated one against master on http://github.com/nodejs/node where it would have the best chance of getting a full review.

@bahamas10
Copy link
Author

thanks @jasnell created nodejs/node#2411

@bahamas10 bahamas10 closed this Aug 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants