Monkey patch node-jslint windows problem and update deps.

73rhodes / Sublime-JSLint

JSLint for Sublime Text 2 and 3.

80 stars 14 forks source link

Monkey patch node-jslint windows problem and update deps. #19

Closed janraasch closed 10 years ago

janraasch commented 10 years ago

By @mayteks comment on https://github.com/darrenderidder/Sublime-JSLint/pull/11 I was reminded of this issue. So I finally went ahead and did what I proposed in https://github.com/darrenderidder/Sublime-JSLint/issues/4#issuecomment-18100021.

With this we have

node-jslint and all its built in options (including e.g. --predef) running on all systems.
pretty ugly monkey patch of node_modules/jslint/lib/reporter.js, but seems ok for the use case.
new jslint.js from 2013-08-26
folder linting under linux and mac os. (no windows due to the problems mentioned in https://github.com/darrenderidder/Sublime-JSLint/issues/4)
The best part. Following updates do not override previously set jslint options.

Still, we definitely need some Windows user to try this, before releasing it to the public again :).

darrenderidder commented 10 years ago

Thanks for this, but for some time jslint has been incorporated directly into the plugin and doesn't depend on node-jslint being installed. This is similar to jshint and linter plugins and it really reduces variability across systems and makes things simpler for end users. We still need to solve the problem of over-writing configuration settings on upgrade, though. Upgrades happen very rarely, but yes the settings should be preserved.

janraasch commented 10 years ago

Okay. I'd still argue that relying on node-jslint for parsing all the available options (e.g. --predef) out of the box is a huge plus compared to writing yet another command line tool calling jslint internally with yet another proprietary API. Esspecially, since node-jslint seems to be used by a lot of people. And, since this pull request includes node_jslint it's not like people have to install it themselves before using this plugin.

No worries though, I wrote up a quick pull request with only the updatable config part (https://github.com/darrenderidder/Sublime-JSLint/pull/21). Take what you like :)

darrenderidder commented 10 years ago

Merged #21. There's a case to be made for including node-jslint but also some reasons not to. The original version required node-jslint but this ended up creating difficulties for users on some systems. Also node-jslint didn't support some of the jslint options I required at the time so ended up monkey-patching it anyway. Perhaps those problems are solved by now. Its a great module for sure. Still, I feel it's a clean and simple option to use linter.js to invoke jslint directly, rather than introducing another module.