Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Letting Group(commands=) take a list of Commands #1339

Closed
energizah opened this issue Jun 12, 2019 · 2 comments · Fixed by #1405
Closed

Letting Group(commands=) take a list of Commands #1339

energizah opened this issue Jun 12, 2019 · 2 comments · Fixed by #1405
Milestone

Comments

@energizah
Copy link
Contributor

energizah commented Jun 12, 2019

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
Copy link
Contributor

jcrotts commented Jun 13, 2019

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
Copy link
Contributor Author

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.

@davidism davidism added this to the 8.0.0 milestone Aug 14, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants