bw2 / ConfigArgParse

A drop-in replacement for argparse that allows options to also be set via config files and/or environment variables.
MIT License
727 stars 121 forks source link

Incorrect error message for unknown config file keys + positional choices argument #120

Open macfreek opened 6 years ago

macfreek commented 6 years ago

Steps to reproduce

Consider the following code:

import configargparse

parser = configargparse.ArgumentParser(ignore_unknown_config_file_keys=False)

parser.add_argument('toppings', nargs='*', metavar='COMMAND',
                 choices=['bacon', 'egg', 'sausage', 'spam'])

config_file_contents = """
opt2 = 42
"""

options = parser.parse_args([], config_file_contents=config_file_contents)

I expected this to print error: unrecognized arguments: opt2 = 42. (This is the error given for parser.parse_args(['bacon']))

The actual result is that it prints error: argument COMMAND: invalid choice: '42' (choose from 'bacon', 'egg', 'sausage', 'spam').

Suggested solution

This is caused by a limitation of argparse.ArgumentParser().parse_known_args(), which is used by configargparse: if ignore_unknown_config_file_keys is False, configargparse adds all arguments from the config file to the list of arguments, and feeds it to argparse.ArgumentParser().parse_known_args(), with the key name and value as separate arguments. E.g. parse_known_args('--opt2', '42'). My suggestion is to add this as combined arguments instead. E.g. parse_known_args('--opt2=42') This will result in a much more readable error: unrecognized arguments: --opt0=42.

macfreek commented 6 years ago

I did a quick code review to see what should be changed to make this happen.

At first sight, all it takes is adjusting convert_item_to_command_line_arg, the lines:

        args.append( command_line_key )
        args.append( value )

with

        args.append( command_line_key + '=' + value)

Unfortunately, this will fail: the already_on_command_line function relies on the fact that all keys have their own argument.

I wonder why keys and values are added to the command line arguments at all if there is no associated action.

For me, the only reason to put ignore_unknown_config_file_keys=False instead of ignore_unknown_config_file_keys=True in my code is that I want to print a proper warning and still continue the execution of my program (I overriding the error() method to achieve this).