Skip to content
This repository has been archived by the owner on Jan 5, 2021. It is now read-only.

Resolve issue #12: .jshintrc is found all the way up the file tree #17

Merged
merged 6 commits into from Mar 12, 2014
Merged

Resolve issue #12: .jshintrc is found all the way up the file tree #17

merged 6 commits into from Mar 12, 2014

Conversation

ghost
Copy link

@ghost ghost commented Mar 8, 2014

Hi,

I resolved Issue #12 (#12).

Best, sharky

@Joezo
Copy link
Owner

Joezo commented Mar 8, 2014

Thank you! I'll take a look through this tomorrow :)

On Saturday, 8 March 2014, sharky3112 notifications@github.com wrote:

Hi,

I resolved Issue #12 #12 (#12#12
).

Best, sharky

You can merge this Pull Request by running

git pull https://github.com/sharky3112/atom-jshint issue#12

Or view, comment on, or merge it at:

#17
Commit Summary

File Changes

Patch Links:

Reply to this email directly or view it on GitHubhttps://github.com//pull/17
.

Kind RegardsJoe holidayextras.com http://holidayextras.com/We take the
hassle, you take the holiday. (R)

@Joezo
Copy link
Owner

Joezo commented Mar 9, 2014

If you open a window with no project the package throws an error
TypeError: Cannot call method 'split' of undefined at AtomJsHint.module.exports.AtomJsHint.loadConfig (packages/atom-jshint/lib/atom-jshint.js:158:34)

It also bombs out completely and stops you opening the editor if it can't find a .jshintrc file anywhere :( Thanks for getting this moving though! I'd like to point out that the original issue #12 specifies it should start using the config file from the file you are currently editing, so that makes it slightly more complicated. It may be worth looking at the source of JSHint to figure out how they do it. I look forward to reviewing this further :)

@ghost
Copy link
Author

ghost commented Mar 10, 2014

Hi! Sorry for crashing your Atom, I immediatelly fixed the bug.

I don't quite understand what you mean by

it should start using the config file from the file you are currently editing

According to the docs there are three ways to use a config file:

  • you can either specify the configuration file manually via the --config flag
  • use a special file .jshintrc or
  • put your config into your projects package.json file under the jshintConfig property.

In case of .jshintrc, JSHint will look for this file in the current working directory and, if not found, will move one level up the directory tree all the way up to the filesystem root.

The first method does not really apply here, the third method is already handled in your code and the second method just got implemented.

What am I missing? I'd gladly do that, too :D

@Joezo
Copy link
Owner

Joezo commented Mar 11, 2014

What I mean is that in your implementation you start looking up the tree from the project root. JSHint begins at the file you are editing, working it's way up from there :) This is not something i'd block the merge on though as it's an improvement on what we have right now :)

@ghost
Copy link
Author

ghost commented Mar 11, 2014

Ah I see! I've changed the path so that this happens. But the config is only loaded once per project (at the beginning), so when you choose another file in another folder in your project, the config gets not reloaded and therefore the path does not get adjusted... I dont know where to change this behavior right now, maybe you now this from the top of your head.

@Joezo
Copy link
Owner

Joezo commented Mar 12, 2014

In that case I think we should stick to what you had before as that it is better than what we've currently got. I'd rather it used the project as a base until we are able to reload the config for each file etc.. are you able to revert that commit? :)

@ghost
Copy link
Author

ghost commented Mar 12, 2014

Sure thing, done 👍

Joezo pushed a commit that referenced this pull request Mar 12, 2014
Resolve issue #12: .jshintrc is found all the way up the file tree
@Joezo Joezo merged commit a12a597 into Joezo:master Mar 12, 2014
@ghost ghost deleted the issue#12 branch March 12, 2014 22:28
@erikdonohoo
Copy link

For projects that have multiple .jshintrc files it would be nice to have it reload the config each time a file is saved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants