Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Fix per-project configuration behaviour #201

Merged

Conversation

rarguelloF
Copy link
Contributor

Per-project settings should take precedence over user settings, just as stated in the docs:

Note that package settings from Settings panel will be ignored if a configuration file was found.

This PR fix this behaviour and ignores user settings if finds a local configuration file.

@Arcanemagus
Copy link
Member

Huh, I was under the impression (from that description) that specifying a config file caused flake8 to ignore those flags, if the flags are overriding the config file (which makes sense) then I agree with you that this should be changed.

@rarguelloF
Copy link
Contributor Author

rarguelloF commented Jun 12, 2016

Yes, flake8 doesn't ignore those flags with the --config flag :(

Btw, I've just realized my solution has a problem. I should check also the config file exists, not only if the parameter is specified (already updated this 😃 ).

@rarguelloF rarguelloF force-pushed the fix-per-project-configuration branch 2 times, most recently from 04f3dd0 to 0725f9f Compare June 12, 2016 20:28
getExistingConfigFiles: (configFiles) ->
return configFiles
.split(/[ ,]+/)
.map (configFile) -> path.join(atom.project.getPaths()[0], configFile)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will fail if the file isn't opened in a project.

@rarguelloF rarguelloF force-pushed the fix-per-project-configuration branch from 0725f9f to a25842f Compare June 14, 2016 09:49
@rarguelloF
Copy link
Contributor Author

I updated my solution. Take it a look and tell me if you see something wrong :D

@@ -188,6 +188,32 @@ module.exports =
return p
return execPath

getProjectPath: (lintedFile) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole function can be replaced with:

atom.project.relativizePath(lintedFile)[0]

See here for the documentation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you need to handle when that returns null (the file not being opened in a project).

@Arcanemagus
Copy link
Member

@rarguelloF Are you interested in addressing the review comments?

@rarguelloF
Copy link
Contributor Author

Yes I'm interested, I'll review your comments this week.

And sorry for disappearing so long, I've had some extremely busy weeks 😢

Thanks for your patience!! 😄

@Arcanemagus
Copy link
Member

No worries, it took me that long to check back 😛.

Override user settings only if per-project config files
exist (using the first encountered one).
Now the associated project path is calculated from the linted file path.
@rarguelloF rarguelloF force-pushed the fix-per-project-configuration branch from 0613e2c to b1eeb5b Compare August 31, 2016 17:44
@rarguelloF
Copy link
Contributor Author

@Arcanemagus I've already addressed the issues. Please tell me if you want me to change something else :D

@Arcanemagus
Copy link
Member

LGTM, merging!

@Arcanemagus Arcanemagus merged commit 2aa70eb into AtomLinter:master Sep 2, 2016
@Arcanemagus
Copy link
Member

Published in v1.13.3 🎉

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

Successfully merging this pull request may close these issues.

2 participants