Allow more control over ArgumentParser initialization in management commands

[Dmitry]

Hello everyone.
Noticed today that there is no way to customize CommandParser (ArgumentParser subclass), initialized in management commands (​https://github.com/django/django/blob/master/django/core/management/base.py#L227). There is no option (or i was unable to find it - please correct me in this case) to pass any kwargs in CommandParser constructor. The public method create_parser contains some django-related hard-coded add_arguments calls, so overriding it directly leads to some copy paste.

I needed to use some custom formatter for ArgumentParser (​https://docs.python.org/3/library/argparse.html?highlight=argparse#argparse.ArgumentDefaultsHelpFormatter) to add default values to help page, but was unable to do this properly. There is also some other usefull keyword arguments available in ArgumentParser.

As a proposal a new method could be added to BaseCommand (let's say BaseCommand.get_parser_instance), for example like this:

def get_parser_instance(self):
        return CommandParser(
            self, prog="%s %s" % (os.path.basename(prog_name), subcommand),
            description=self.help or None,
        )

def create_parser(self, prog_name, subcommand)::
       parser = self.get_parser_instance()
       ... etc

This should not break anything in BaseCommand, but will allow have more control on parser's creation overriding just get_parser_instance method.
Please sorry for mistakes.

[Tim Graham]

Would a hook to provide some additional keyword arguments be sufficient?

[Dmitry]

Replying to Tim Graham:

Would a hook to provide some additional keyword arguments be sufficient?

Yeah, this will be great, maybe the better way. I was thinking about way to implement this, but was unable to find beautiful enough way to add such hook. Maybe you have some good ideas.

As a reference my idea was something like this:

1. Add parser_kwargs argument in BaseCommand.init, defaults to {}

           def __init__(self, stdout=None, stderr=None, no_color=False, parser_kwargs=None):
           self.stdout = OutputWrapper(stdout or sys.stdout)
           self.stderr = OutputWrapper(stderr or sys.stderr)
           if no_color:
               self.style = no_style()
           else:
               self.style = color_style()
               self.stderr.style_func = self.style.ERROR
          if not parser_kwargs:
              parser_kwargs = {}
   

2. use parser_kwargs in create_parser method

       def create_parser(self, prog_name, subcommand):
           """
           Create and return the ``ArgumentParser`` which will be used to
           parse the arguments to this command.
           """
           if not 'prog' in self.parser_kwargs:
               self.parser_kwargs['prog'] = "%s %s" % (os.path.basename(prog_name), subcommand)
           if not 'description' in self.parser_kwargs:
               self.parser_kwargs['description'] = self.help or None
           parser = CommandParser(
               self, **self.parser_kwargs
           )
   

The problem is that in this case you will have two conflicting way to define description (using parser_kwargs or help), but possibly it's not a big issue.

[Tom Forbes]

The create_parser method could just pass in all kwargs to CommandParser, rather than having a parser_kwargs instance variable?

def create_parser(self, **kwargs):
    return super().create_parser(my_custom_arg=123)

# In BaseCommand
def create_parser(self, **kwargs):
    kwargs.setdefault('description', ...)
    return CommandParser(self, **kwargs)

Might be a bit more flexible

[Dmitry]

Replying to Tom Forbes:

The create_parser method could just pass in all kwargs to CommandParser, rather than having a parser_kwargs instance variable?

def create_parser(self, **kwargs):
    return super().create_parser(my_custom_arg=123)

# In BaseCommand
def create_parser(self, **kwargs):
    kwargs.setdefault('description', ...)
    return CommandParser(self, **kwargs)

Might be a bit more flexible

I agree, this looks better.

[humbertotm]

Hi, guys.

I would like to work on this.
There's been no activity in this thread lately, so I'm hoping this is still a valid issue. If not, please let me know.
Thanks!

[humbertotm]

Replying to Dmitry:

Replying to Tom Forbes:

The create_parser method could just pass in all kwargs to CommandParser, rather than having a parser_kwargs instance variable?

def create_parser(self, **kwargs):
    return super().create_parser(my_custom_arg=123)

# In BaseCommand
def create_parser(self, **kwargs):
    kwargs.setdefault('description', ...)
    return CommandParser(self, **kwargs)

Might be a bit more flexible

I agree, this looks better.

By the looks of it, I am guessing that every call to this method in the code base would have to change in order to fit the proposed method signature, right?

[humbertotm]

This is the fix I'm proposing based on my understanding of the issue. As far as I understand, the requested enhancement requires that CommandParser can ultimately receive any additional args that it can pass to its parent class ArgumentParser. What I did was adding an additional positional argument in the form of kwargs to the create_parser() method in BaseCommand as follows:

def create_parser(self, prog_name, subcommand, **kwargs):
        """
        Create and return the ``ArgumentParser`` which will be used to
        parse the arguments to this command. Hi.
        """
        default_kwargs = {
            'prog': '%s %s' % (os.path.basename(prog_name), subcommand),
            'description': self.help or None,
            'formatter_class': DjangoHelpFormatter,
            'missing_args_message': getattr(self, 'missing_args_message', None),
            'called_from_command_line': getattr(self, '_called_from_command_line', None)
        }
        kwargs.update(default_kwargs)
        parser = CommandParser(**kwargs)
        ...

This way, the kwargs provided to the method call are merged with the default kwargs into a single dictionary to be passed on to CommandParser. These changes do not break the test suite, but additional regression tests are still pending.
Let me know if I'm failing to see or understand something by following this approach.
Thanks!

[humbertotm]

Replying to Claude Paroz:

Hi, Claude!

I was going through the tests dir looking for the appropriate place for this fix's test, but I am a bit lost.
The /tests/admin_scripts/tests.py file seems like a good candidate, but I have my doubts.
Any guidance on this would be greatly appreciated!

[Carlton Gibson]

Replying to humbertotm:

The /tests/admin_scripts/tests.py file seems like a good candidate

Either that or tests/user_commands.

[Claude Paroz]

+1 for user_commands.

[Tim Graham <timograham@…>]

In e95008f2:

Fixed #29152 -- Allowed passing kwargs to ArgumentParser initialization in management commands.