fix --exclude with ament_flake8 #77
Conversation
| flake8_argv.append('--exclude={0}'.format(excludes)) | ||
| if config_file is not None: | ||
| flake8_argv.append('--config={0}'.format(config_file)) | ||
| if excludes is not None: |
There was a problem hiding this comment.
Changing the check to if not excludes: will give consistent behaviour with the other linters (including flake8 v2) in the event the list is empty. Unless there's a reason this check is more appropriate, could you update it?
There was a problem hiding this comment.
I am not sure I understand. The variable excludes is either None or a not empty list. How would it be an empty list?
There was a problem hiding this comment.
From my understanding it could be an empty list if a user specifies the --exclude option, but with no filenames
There was a problem hiding this comment.
That is not possible. argparse won't allow you to do that with the current settings.
Also in that case I think the line you commented on is doing exactly the right thing: excludes is not None and therefore calls the script with --excluded=. (which would be what the user asked for).
There was a problem hiding this comment.
Which setting makes it not possible? From what I've seen '--foo --bar 1 2 will make foo an empty list if it's an nargs='*' option.
My concern was consistency with the v2.x implementation, since it doesn't use the argument if it's []. but after re-looking at it, the end result will still turn out the same as 2.x with this implementation, so I'm fine to leave it as is
There was a problem hiding this comment.
If you try to run ament_flake8 --exclude the result will be:
Usage: flake8 [options] file file ...
flake8: error: --exclude option requires 1 argument
There was a problem hiding this comment.
oh, here's an example of how it can be an empty list:
In [1]: import argparse
In [2]: parser = argparse.ArgumentParser()
In [3]: parser.add_argument('--foo', nargs='*')
Out[3]: _StoreAction(option_strings=['--foo'], dest='foo', nargs='*', const=None, default=None, type=None, choices=None, help=None, metavar=None)
In [4]: parser.add_argument('--bar', nargs='*')
Out[4]: _StoreAction(option_strings=['--bar'], dest='bar', nargs='*', const=None, default=None, type=None, choices=None, help=None, metavar=None)
In [5]: parser.parse_args('--foo --bar 1 2'.split())
Out[5]: Namespace(bar=['1', '2'], foo=[])
it's strange that argparse has different behaviour base on its position in the list (strange to me, at least!)
No description provided.