Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tobie opened this issue Dec 1, 2013 · 17 comments
Open

Comments

@tobie
Copy link

tobie commented Dec 1, 2013

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
Copy link
Owner

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

On Sun, Dec 1, 2013 at 4:28 AM, Tobie Langel notifications@git.luolix.topwrote:

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//issues/26
.

Raymond Camden, Adobe Developer Evangelist

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

@tobie
Copy link
Author

tobie commented Dec 1, 2013

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
Copy link
Owner

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@git.luolix.topwrote:

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//issues/26#issuecomment-29574251
.

Raymond Camden, Adobe Developer Evangelist

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

@qubyte
Copy link

qubyte commented Jan 14, 2014

👍

@qubyte
Copy link

qubyte commented Jan 24, 2014

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

@cfjedimaster
Copy link
Owner

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@git.luolix.topwrote:

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//issues/26#issuecomment-33272057
.

Raymond Camden, Web Developer for Adobe

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

@qubyte
Copy link

qubyte commented Jan 27, 2014

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
Copy link

qubyte commented Jan 28, 2014

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
Copy link
Owner

In Brackets, Debug menu, show developer tools.

On Mon, Jan 27, 2014 at 8:21 PM, Mark S. Everitt
notifications@git.luolix.topwrote:

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//issues/26#issuecomment-33445602
.

Raymond Camden, Web Developer for Adobe

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

@qubyte
Copy link

qubyte commented Jan 28, 2014

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
Copy link
Contributor

busykai commented Jan 28, 2014

@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
Copy link

qubyte commented Jan 28, 2014

Will the feature as implemented here cause problems later on?

@busykai
Copy link
Contributor

busykai commented Jan 28, 2014

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
Copy link
Contributor

busykai commented Jan 28, 2014

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

@qubyte
Copy link

qubyte commented Jan 28, 2014

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
Copy link

qubyte commented Jan 28, 2014

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

@busykai
Copy link
Contributor

busykai commented Jan 28, 2014

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

busykai added a commit to busykai/brackets-jshint that referenced this issue Jan 28, 2014
Add support for reading properties from package.json. It takes priority
over .jshintrc.
busykai added a commit to busykai/brackets-jshint that referenced this issue Jan 28, 2014
Keep track of currently used file and reload it when it changes.
@busykai busykai mentioned this issue Jan 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants