Letting Group(commands=) take a list of Commands

[energizah]

I like to use the class-based API for commands. Group(commands=) takes a dict mapping name to command. So I end up writing:

#!/usr/bin env python3

import click


def f(*a, **kw):
    print(a, kw)


commands = [click.Command("cmd1", callback=f), click.Command("cmd2", callback=f)]


cli = click.Group(commands={c.name: c for c in commands})

if __name__ == "__main__":
    cli()

It's a bit annoying to need the dict comprehension outside, so I'm wondering if you might take an addition in __init__, like

if isinstance(commands, collections.abc.Sequence):
    commands = {command.name: command for command in commands}

so that I could write

click.Group(commands=commands)

instead of

click.Group(commands={c.name: c for c in commands})

in all my scripts.

[jcrotts]

I'm not sure adding this to the group init is worth it. The API for group clearly asks for a dictionary of commands. I think making group accept a list or sequence complicates the API, and doesn't provide value outside of the slightly smaller user code.

Happy to review a PR on it though, if you think it does provide value to Click as a whole.

[energizah]

Here's the PR.

I realize this adds a little bit of complexity to the constructor, which isn't optimal. On the other hand, the change makes it easier to use the class-based API, which I think is a good thing and on balance worth the small complexity cost.