Skip to content

Local configuration files are ignored if parse_argv is False#246

Closed
kynan wants to merge 1 commit intoPyCQA:masterfrom
kynan:246-enable-local-configuration-file-parsing
Closed

Local configuration files are ignored if parse_argv is False#246
kynan wants to merge 1 commit intoPyCQA:masterfrom
kynan:246-enable-local-configuration-file-parsing

Conversation

@kynan
Copy link
Copy Markdown
Contributor

@kynan kynan commented Dec 3, 2013

When using pep8 as a library, there is currently no way to make the StyleGuide read local configuration files (tox.ini, setup.cfg) other than passing parse_argv=True. This makes little sense in library use where sys.argv is empty, since it breaks for the case where no local configuration files exist with "input not specified".

When used as a library, set args in process_options to current
directory to enable parsing of local configuration files.
@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Dec 3, 2013

For some background: The problem is illustrated by https://bitbucket.org/tarek/flake8/commits/d325cf7e7f4df3efe650dd7b26c8338d452fca53, a change I made to flake8 to allow the Git and Mercurial hooks to parse local configuration files, but which breaks if no configuration files are present. Unfortunately it is not possible to make this work for both cases without this change to pep8.

@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Mar 11, 2014

Any thoughts on this?

@sigmavirus24
Copy link
Copy Markdown
Member

flake8 is dependent on this change @florentx

@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Mar 23, 2014

Some more explanation: Not specifying parse_argv causes pep8 to ignore any local config files: args and arglist are both empty in pep8.py:1843, which means the local config files are ignored because the loop in pep8.py:1773 is never executed.

@florentx
Copy link
Copy Markdown
Contributor

In order to preserve backward compatibility, I would accept an additional keyword argument arglist in the StyleGuide constructor.

diff --git a/pep8.py b/pep8.py
index e38ab26..71aaf19 100755
--- a/pep8.py
+++ b/pep8.py
@@ -1571,11 +1571,12 @@ class StyleGuide(object):
     def __init__(self, *args, **kwargs):
         # build options from the command line
         self.checker_class = kwargs.pop('checker_class', Checker)
+        arglist = kwargs.pop('arglist', None)
         parse_argv = kwargs.pop('parse_argv', False)
         config_file = kwargs.pop('config_file', None)
         parser = kwargs.pop('parser', None)
-        options, self.paths = process_options(
-            parse_argv=parse_argv, config_file=config_file, parser=parser)
+        (options, self.paths) = process_options(
+            arglist, parse_argv, config_file, parser)
         if args or kwargs:
             # build options from dict
             options_dict = dict(*args, **kwargs)

It will be available in the flake8 function get_style_guide without change.

    flake8_style = get_style_guide(
        arglist=['.'],
        config_file=DEFAULT_CONFIG,
        **options)

@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Mar 23, 2014

OK, that would work for flake8. I'll send a pull request.

@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Mar 23, 2014

Done: #259

@florentx
Copy link
Copy Markdown
Contributor

The other PR is merged

@florentx florentx closed this Mar 23, 2014
@florentx
Copy link
Copy Markdown
Contributor

On second thought, I see the argument paths is already documented on the StyleGuide constructor. The arglist argument looks like a duplicate of this.
Maybe I'll revert the last fix and use the paths instead, it makes more sense.

@florentx florentx reopened this Mar 23, 2014
@florentx
Copy link
Copy Markdown
Contributor

@sigmavirus24
Copy link
Copy Markdown
Member

This should probably be better documented then. :)

@kynan
Copy link
Copy Markdown
Contributor Author

kynan commented Mar 23, 2014

@florentx I agree, using paths makes sense given that it's documented but not currently passed into process_options. I'm happy to propose another patch unless you prefer to fix it yourself.

@florentx florentx closed this in 68d72a5 Mar 23, 2014
@florentx florentx added this to the 1.5.0 milestone Mar 25, 2014
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