Use jshint to check code for problems and errors

[janpieper]

Problems

#80 Drop unused variables #81 Drop duplicate test lib/Client.js#L22 vs. lib/Client.js#L59-L62 (found while making the code jshint compliant)

JSHint Configuration

node

This option defines globals available when your code is running inside of the Node runtime environment. Node.js is a server-side JavaScript environment that uses an asynchronous event-driven model.

Configured value: true

laxbreak

This option suppresses most of the warnings about possibly unsafe line breakings in your code. It doesn't suppress warnings about comma-first coding style. [...]

Configured value: true

This allows us to use the ternary operator with multiple lines:

navdata[optionName] = (exports.OPTION_PARSERS[optionName])
  ? exports.OPTION_PARSERS[optionName](optionReader)
  : new Error('Option not implemented yet: ' + optionName + ' (0x' + optionId.toString(16) + ')');

See lib/navdata/parseNavdata.js#L60-L62.

indent

This option enforces specific tab width for your code. [...]

Configured value: 2

curly

This option requires you to always put curly braces around blocks in loops and conditionals. JavaScript allows you to omit curly braces when the block consists of only one statement, [...]

Configured value: true

trailing

This option makes it an error to leave a trailing whitespace in your code. Trailing whitespaces can be source of nasty bugs with multi-line strings in JavaScript [...]

Configured value: true

[felixge]

-1 on trailing whitespace (why would we want that?)

indent: does this use tabs or spaces?

does jshint allow automatic code formatting?

will we run jshint as part of the test suite on travis?

[rmehner]

On 24.10.2013, at 08:46, Felix Geisendörfer notifications@github.com wrote:

-1 on trailing whitespace (why would we want that?)

They are disallowed (setting the trailing option to true enforces that), all correct. Nobody wants trailing whitespaces.

indent: does this use tabs or spaces?

It's using the tab width, so spaces. By default it doesn't allow mixing spaces and tabs.

does jshint allow automatic code formatting?

Nope, that would be part of a different tool. I know your love for go-fmt ;-)

will we run jshint as part of the test suite on travis?

I'd suggest doing that once the .jshintrc is ready

[felixge]

Alright, LGTM then!

[andrew]

I'm interested in how you hook that into travis, would love to get that on all my js projects, automatic lint checking of all pull requests ftw.

[rmehner]

Merged. Thank you @janpieper :)

Travis + JSHint in a separate PR, I'll take care of that right now.

[rmehner]

Good news, npm test already recognizes the .jshintrc and runs jshint automagically for us. Therefore no extra step needed for Travis, it already works!

On a side note: I think we should add some more options to jshint, totally overlooked that in this PR. Will open up a PR with more options to discuss things.