-
Notifications
You must be signed in to change notification settings - Fork 285
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 support for pylint config files #673
base: develop
Are you sure you want to change the base?
Conversation
@goanpeca, please review this one. |
@goanpeca, any news here? |
Sorry I missed the initial ping. Will take a look at it now! |
|
||
log = logging.getLogger(__name__) | ||
|
||
PROJECT_CONFIGS = ['.pylintrc', 'pylintrc'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we are only using the first 2 options?
https://docs.pylint.org/en/1.6.0/run.html
pylintrc in the current working directory
.pylintrc in the current working directory
If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a pylintrc file. This allows you to specify coding standards on a module-by-module basis. Of course, a directory is judged to be a Python module if it contains an __init__.py file.
The file named by environment variable PYLINTRC
if you have a home directory which isn’t /root:
.pylintrc in your home directory
.config/pylintrc in your home directory
/etc/pylintrc
return self.parse_config_multi_keys(config, CONFIG_KEYS, OPTIONS) | ||
|
||
def _user_config_file(self): | ||
if self.is_windows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to make this special case for windows?
I was expecting this os.path.expanduser('~/.pylintrc')
to work on all osses
class PylintConfig(ConfigSource): | ||
"""Parse pylint configurations.""" | ||
|
||
def user_config(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to add a docstring with some explanation. Also maybe saying what the returned object is.
return os.path.expanduser('~\\.pylintrc') | ||
return os.path.expanduser('~/.pylintrc') | ||
|
||
def project_config(self, document_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to add a docstring with some explanation. Also maybe saying what the returned object is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition @youben11 ! Left a couple of questions and we now need to add tests for this functionality
'disable': 'MESSAGES CONTROL', | ||
'ignore': 'MASTER', | ||
'max-line-length': 'FORMAT', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still active? It would be a much appreciated feature! 🚀 |
@gatesn This PR fixes #616 by supporting pylintrc config files, this will make sure pylint do the linting the same way on all files. To also make sure I don't break working clients that are configuring pylint through "pyls.plugins.pylint.args", I made it the first value to use as argument if it was found, otherwise, we build arguments based either on the conf files or the other values specified in the plugin conf.