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

Use project path as cwd if present #127

Closed
wants to merge 1 commit into from

Conversation

dmnd
Copy link

@dmnd dmnd commented Jul 8, 2014

See AtomLinter/linter-pylint#8 for motivation for this.
It's helpful to use the project root for Python linting at least. I suspect it
would be useful more generally too.

Feel free to close this request if you disagree.

Test Plan:
Crossed fingers

See AtomLinter/linter-pylint#8 for motivation for this.
It's helpful to use the project root for Python linting at least. I suspect it
would be useful more generally too.

Feel free to close this request if you disagree.

Test Plan:
Crossed fingers
@hd-deman
Copy link
Contributor

hd-deman commented Jul 8, 2014

#5 related code

@hd-deman
Copy link
Contributor

hd-deman commented Jul 8, 2014

@park9140 @iam4x let's discuss

@park9140
Copy link

park9140 commented Jul 8, 2014

It seems to me that the a good number of tools search up your file tree for the project config file. Doing it from root doesn't seem correct for single file linting, which is the primary set of supported linters.

However, the cwd is modifiable by individual linters. I have a project I'm working on where I actually update this to be the project path because the "linter" requires full project knowledge in order to operate properly. I assume this is the case in pylint.

Should probably just override in the linters where this matters. I would just pull AtomLinter/linter-pylint#8.

@dmnd
Copy link
Author

dmnd commented Jul 8, 2014

Linters that only care about a single file probably ignore the cwd anyway, so I think this is a better default.

Also, as a linter author the 'current working directory' being a subdirectory of Atom's cwd/project dir is surprising.

@dmnd
Copy link
Author

dmnd commented Aug 15, 2014

Closing this since it seems that others are not convinced it's useful.

@dmnd
Copy link
Author

dmnd commented Aug 18, 2014

Hey @park9140, I'm reopening this since it would have helped out with AtomLinter/linter-csslint#8 for example.

Here's a demonstration for why this is a better default:

~/project $ csslint lib/file.css -> the cwd is ~/project
~/project $ cd lib; csslint lib/file.css -> the cwd is ~/project/lib

Above you can see that on the command line your cwd is determined by your context.

~/project $ atom . # then open lib/file.css -> the linter cwd is ~/project/lib
~/project $ atom lib # then open file.css -> the linter cwd is ~/project/lib

But in AtomLinter, we ignore your context and always set the cwd to the basename of the path being linted.

I think it's beneficial to be consistent with the command line usage. This is what lint executable authors will be designing for. We're doing something different for no good reason. What benefit do we get by overriding the cwd like this?

@dmnd dmnd reopened this Aug 18, 2014
@park9140
Copy link

It seems like the large majority of linters seem to implement this as the default now. There are a few like tslint, and jshint that will need to be modified when this change goes in to maintain the existing functionality.

@dmnd
Copy link
Author

dmnd commented Aug 18, 2014

Yeah, auditing existing linters is going to be a pain. I'll definitely hold off until I can do that.

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

Successfully merging this pull request may close these issues.

3 participants