Skip to content
This repository

predef option is passed to jslint as string #10

Closed
medikoo opened this Issue · 6 comments

2 participants

Mariusz Nowak
Mariusz Nowak
medikoo commented

predef option when passed to jslint should be instance of Array or Object, but currently it is passed as it was read from command line - as string, which makes it unusable as jslint then just ignores its value.

It'll be great to have it parsed into an array before it goes to jslint.

The option parser that node-jslint uses, nopt, supports arrays through repeating the option that you want to pass in as an array.

So to pass multiple values for --predef you would do:

jslint --predef $ --predef Backbone app.js

I've added this example to the README.md here - #34

Mariusz Nowak

@nikuda thanks for tip, I wasn't aware of that.

What's funny that this issue was "fixed" in node-jslint long while ago: https://github.com/reid/node-jslint/blob/master/lib/linter.js#L24 It already works with comma separated values.

For some reason this issue haven't been closed. I'm doing it now..

Mariusz Nowak medikoo closed this

@medikoo The fix you mention above hasn't worked for me, and I'm not entirely sure it would work for anyone else. It seems that nopt gets to parsing the options before the fix code does. Because nopt parses the options first, anything after the first comma ends up in the parsed.argv.remain array, which gets passed to lintFile() which then fails.

Mariusz Nowak

@nikuda indeed, I looked at the code and such support was broken few months ago with that commit: https://github.com/reid/node-jslint/commit/7fd0321a1358777545eceee2f29650044ff863d3#bin/jslint.js
Before it was fixed to work with comma separated values (thanks to 94d2ca6 ). I'm still using fork that works that way.

Anyway now it's probably more up to nopt convention, so maybe it should stay as you say.

Not a problem, thanks.

I can also remove the dead code as part of my pull request above, if you don't see any other potential problems.

Mariusz Nowak

@nikuda it's no problem. I'm anyway not using that branch anymore (I had some specific needs and I'm now using different version of node-jslint)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.