cfjedimaster / brackets-jshint

Adds JSHint support to Brackets
MIT License
131 stars 41 forks source link

Look for JSHint config info in jshintConfing property of package.json. #26

Open tobie opened 10 years ago

tobie commented 10 years ago

JSHint supports config info set in the jshintConfing property of package.json. Would be nifty if Brackets supported that too.

More details here (scroll down to "Configuration").

cfjedimaster commented 10 years ago

Do you know offhand what takes priority - .jshintrc or package.json?

On Sun, Dec 1, 2013 at 4:28 AM, Tobie Langel notifications@github.comwrote:

JSHint supports config info set in the jshintConfing property of package.json. Would be nifty if Brackets supported that too.

More details here http://www.jshint.com/docs.

— Reply to this email directly or view it on GitHubhttps://github.com/cfjedimaster/brackets-jshint/issues/26 .

Raymond Camden, Adobe Developer Evangelist

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: cfjedimaster

tobie commented 10 years ago

I don't. I was wondering about the precedence issue myself. I'm also suspecting JSHint must have a module that handles that already. Why not tap into it to determine config options?

cfjedimaster commented 10 years ago

I'm just using the core library so I can easily pass in the code string and get results out. I'd rather not bundle more stuff. I think this is a good idea, and if i had to guess, I'd say it should be have lower priority then .jshintrc. If you, or anyone else, wants to implement this and make a pull request, I'll take it in.

On Sun, Dec 1, 2013 at 8:05 AM, Tobie Langel notifications@github.comwrote:

I don't. I was wondering about the precedence issue myself. I'm also suspecting JSHint must have a module that handles that already. Why not tap into it to determine config options?

— Reply to this email directly or view it on GitHubhttps://github.com/cfjedimaster/brackets-jshint/issues/26#issuecomment-29574251 .

Raymond Camden, Adobe Developer Evangelist

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: cfjedimaster

qubyte commented 10 years ago

:+1:

qubyte commented 10 years ago

It's actually the other way around. Config found in package.json takes priority over a .jshintrc file.

cfjedimaster commented 10 years ago

I'd happily accept a pull request for this then. To me it seems a bit complex to have so many different ways to configure it - but if it is the 'standard' for JSHint, it is what it is. :)

On Fri, Jan 24, 2014 at 5:34 PM, Mark S. Everitt notifications@github.comwrote:

It's actually the other way aroundhttps://github.com/jshint/jshint/blob/2.x/src/cli.js#L469. Config found in package.json takes priority over a .jshintrc file.

— Reply to this email directly or view it on GitHubhttps://github.com/cfjedimaster/brackets-jshint/issues/26#issuecomment-33272057 .

Raymond Camden, Web Developer for Adobe

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: cfjedimaster

qubyte commented 10 years ago

Cool. I'll prepare a pull request for you. I took a look the other day, but no access to the Node fs module made me cry. I don't dig promises. ;)

qubyte commented 10 years ago

I've opened a pull request, but I really need to debug this properly. I'm probably being daft, but how can I see the output from console.logs?

cfjedimaster commented 10 years ago

In Brackets, Debug menu, show developer tools.

On Mon, Jan 27, 2014 at 8:21 PM, Mark S. Everitt notifications@github.comwrote:

I've opened a pull request, but I really need to debug this properly. I'm probably being daft, but how can I see the output from console.logs?

— Reply to this email directly or view it on GitHubhttps://github.com/cfjedimaster/brackets-jshint/issues/26#issuecomment-33445602 .

Raymond Camden, Web Developer for Adobe

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: cfjedimaster

qubyte commented 10 years ago

Weird. I didn't have much luck with that. It's late though. I'll sleep on it and return with a fresh mind.

busykai commented 10 years ago

@cfjedimaster @qubyte I believe that "proper" (node-jshint-like) config file handling should be done once Brackets supports async linting (i.e. adobe/brackets#5137 comes through). it will be done along with .jshintrc lookup (possibly using new preferences model which supports config file lookups).

@cfjedimaster could it wait until then?

qubyte commented 10 years ago

Will the feature as implemented here cause problems later on?

busykai commented 10 years ago

i do believe that the implementation is not quite appropriate -- it changes a lot of code where it shouldn't have. looks more like refactoring. i left a comment in #34.

busykai commented 10 years ago

Minor correction: the actual pending PR is adobe/brackets#6530. The other is the link to the original issue (one of the many).

qubyte commented 10 years ago

The refactoring is mainly to avoid code duplication. I could just have copy-pasted-modified the existing promise and chained them together later on, but that would look messy IMO. I will defer to @cfjedimaster though.

qubyte commented 10 years ago

@busykai back to the question... Will this cause problems with respect to adobe/brackets#6530 later on?

busykai commented 10 years ago

let's discuss code next to it.. see #34.