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

Add ability to use absolute config path #59

Merged
merged 2 commits into from
Mar 21, 2015
Merged

Add ability to use absolute config path #59

merged 2 commits into from
Mar 21, 2015

Conversation

jackbrewer
Copy link
Contributor

Hi, Absolutely loving this linter!

Description

I've come across the need to have an absolute path for my config file, which doesn't currently work as the process.cwd() gets automatically prepended to the config path.

Ideal usage: stylint foo.styl -c /Users/jack/.stylintrc

Solution

This pull request adds a check to see if the specified config path is absolute. If it is, it uses it exactly as supplied. If it's not, it continues to use the code you had in place already.

In future, it might be nice to use node's path.isAbsolute(), but as it was only just released with node 0.12, I went with something more backwards compatible.

Tests

I've not added a test for this, as your existing config tests have an outstanding @TODO comment. Wasn't sure whether you'd like to own that, or if you want me to give it a go. The existing tests still pass as expected.

Justification

I'm putting together a SublimeLinter Stylint plugin which allows for 'live' linting. Like jshint, it traverses up the directory tree to find a config file, then supplies its absolute path to Stylint. Early days, but happy to collaborate once I've progressed a little further if you'd like.

sublime-stylint

@danielhusar
Copy link
Collaborator

Wouldn't be better to use this library?
https://github.com/sindresorhus/path-is-absolute

@jackbrewer
Copy link
Contributor Author

Good suggestion @danielhusar. Updated the pull request to match your suggestion.

To avoid adding another dependency, I'd originally used a variation of the actual node implementation of isAbsolute(), but missed the additional Windows support part. The suggested module has a more robust implementation, and is still nice and lightweight.

@rossPatton rossPatton added this to the 0.9.3 milestone Mar 21, 2015
@rossPatton
Copy link
Collaborator

looks good, gonna merge it, should go out in the next release. i'll take a look at the @todo separately.

since this linter is starting to get a lot of attention lately, i've set a develop branch. in the future, please use that instead of master. would like to keep master in sync with the version on npm

btw, thanks for working on a sublime plugin! I'd love to help if i can

rossPatton pushed a commit that referenced this pull request Mar 21, 2015
Add ability to use absolute config path
@rossPatton rossPatton merged commit 10f3818 into SimenB:master Mar 21, 2015
@jackbrewer jackbrewer deleted the feature/absolute-config-path branch March 21, 2015 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants