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

Allow nargs=-1 in options with a non-whitespace separator #2771

Open
Tracked by #2834
libklein opened this issue Sep 2, 2024 · 6 comments
Open
Tracked by #2834

Allow nargs=-1 in options with a non-whitespace separator #2771

libklein opened this issue Sep 2, 2024 · 6 comments
Labels
Milestone

Comments

@libklein
Copy link

libklein commented Sep 2, 2024

What should this feature do?

Allow parsing command line options with an arbitrary number of values. This would allow to type

cli arg1 arg2 --option 1, 2, 3

instead of the more tedious and less familiar

cli arg1 arg2 --option 1 --option 2 --option 3

This has been suggested in #2537. This request is different in that options with nargs=-1 would require an additional separator parameter, that has to be set to a non-whitespace character. This would mitigate the ambiguity issues of #2537.

I have contributed this feature to a downstream CLI library (fastapi/typer#800) and would be happy to contribute the changes to click itself if this is something that you would like to see supported. The changes essentially imply adding a few lines to

def process_value(self, ctx: Context, value: t.Any) -> t.Any:

Please comment accordingly.

@Rowlando13
Copy link
Collaborator

@AndreasBackx What do you think? From a style standpoint, I have seen both styles. I prefer the first. It would also be nice to support and arbitrary number of parameters to options. Then we could just say use arguments for subcommands, urls, or files and options for everything else.

@AndreasBackx
Copy link
Collaborator

AndreasBackx commented Nov 28, 2024

I think it should be --option 1 2 3 without commas. It seems like this would have the implication that arguments would have to be put before, if this is the last option:

cli subcommand second-subcommand --multi-option 1 2 3
cli subcommand --multi-option 1 2 3 second-subcommand

I'd say this is a fair trade-off. Except that in the case that --multi-option is an option from subcommand and not second-subcommand, then it would seemingly be impossible to subsequently detect the following subcommand. Clap has the ability to determine an "end value" so you could do the following:

cli subcommand --multi-option 1 2 3 ; second-subcommand

Where ; signifies the end of the "multi option". Though I don't think this is a good idea.

So the limitation would have to be that this cannot be used for command that have subcommands. Unless we also support "global options" on subcommands (#66). I'd be an advocate for it, though the priority right now is getting up to speed with all issues and PRs before we can tackle parser changes I think.

Also interested in hearing how typer tackles those issues.

@AndreasBackx AndreasBackx added this to the 9.0.0 milestone Nov 28, 2024
@libklein
Copy link
Author

The implementation in typer requires a non-whitespace separator to be specified. Hence, cli --option 1 2 3 is forbidden, it has to be e.g. cli --option 1, 2, 3 or cli --option 1| 2 | 3 or whatever the creator of the CLI specified. This avoids the issue of all arguments having to be passed before the multi-value cli option. This is, from my perspective, a reasonable trade-off.

The subcommand case is a good catch. I'd suggest forbidding multi-value CLI options in all but the last subcommand.

@AndreasBackx
Copy link
Collaborator

AndreasBackx commented Nov 30, 2024

I don't see that having much of a benefit compared to the current state. --option 1,2,3 is already something that can be done and I think is better than --option 1, 2, 3. It's confusing to have , as a separator as it's not common.

We should look at supporting --option 1 2 3 in the future. Though the focus right now is to get a grasp on the open issues and PRs. Therefore I don't think it's something that should be expected in the near future.

@davidism
Copy link
Member

davidism commented Nov 30, 2024

I was very confused until I realized you didn't describe your Typer PR accurately here, compared to the actual PR description.

The parser operates on tokens as given to us by the invoking shell. We can't control how the tokens are split incoming to us. So for example, -a b is two tokens ["-a", "b"], and -a b | 'c d' is four tokens ["-a", "b", "|", "c d"]. Note how 'c d' in the shell stays as one token, but requires quotes.

Your examples in this thread suggest that you want to accept -a b | c | d, where there's space between the separator and no quotes. That would add a huge amount of complexity to the parser. But your PR actually doesn't accept that, it would only accept -a b|c|d or -a 'b | c | d'. In fact, the tests in your PR are only working by accident, since float strips whitespace and invoke accepts tokens, it doesn't do further shell splitting.

@davidism
Copy link
Member

This request is similar to #2810, which I just closed. You can already accomplish this type of parsing without any changes to Click, by doing the following:

import click

def separator(ctx: click.Context, param: click.Parameter, value: list[str]) -> list[str]:
    out: list[str] = []

    for item in value:
        out.extend(v.strip() for v in item.split("|"))

    return out

@click.command
@click.option("-a", multiple=True, callback=separator)
def cli(a: list[str]) -> None:
    click.echo(a)

if __name__ == "__main__":
    cli()

Note that options already support a nargs parameter, to accept -a b c d, but it must be bounded to avoid ambiguity and weird interactions with multiple + nargs + tuple values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants