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

RFC: Comma separated options/arguments for Commands #17588

Open
dereuromark opened this issue Feb 18, 2024 · 2 comments
Open

RFC: Comma separated options/arguments for Commands #17588

dereuromark opened this issue Feb 18, 2024 · 2 comments

Comments

@dereuromark
Copy link
Member

dereuromark commented Feb 18, 2024

Description

So far, one can use multiple options only, and in a very verbose way

bin/cake my_command --option-list=x --option-list=y --option-list=z

and

$args->getMultipleOption('option-list');

What if we were to allow comma-separated input using multiple "comma" option?

// MyCommand
    public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionParser
    {
        $parser = parent::buildOptionParser($parser);

        $parser->addArgument('argument-list', [
            'multiple' => 'comma',
        ]);
        $parser->addOption('option-list', [
            'multiple' => 'comma',
        ]);

        return $parser;
    }

    /**
     * Implement this method with your command's logic.
     *
     * @param \Cake\Console\Arguments $args The command arguments.
     * @param \Cake\Console\ConsoleIo $io The console io
     * @return int|null|void The exit code or null for success
     */
    public function execute(Arguments $args, ConsoleIo $io)
    {
        $x = $args->getArrayArgument('argument-list');

        $y = $args->getArrayOption('option-list');
    }

Then concise

bin/cake my_command bar,baz --option-list=x,y,z

Would result in

// $x
array [
    bar,
    baz
]

and

// $y
array [
    x,
    y,
    z
]

I am open to feedback if this would be viable for core as a more user friendly way of defining multiple values.

We can also use 'multiple' => ',', and allow also other separators, like ;, etc to be used.

Right now I always end up using string inputs and manually making the comma-separated list an array inside.

There is also already $args->getBooleanOption() and $args->getMultipleOption().
One reason why I wouldn't want to overload the meaning of getMultipleOption() is that this only works for options, not arguments. We could of course only add getMultipleArgument() only, as well.

The other reason: The type prefix makes it more clear what it is about. Beside existing "bool", the new "array" makes it more clear that this is still a single argument and not multiple options or arguments, single one with just a list..

@dereuromark dereuromark added this to the 5.1.0 milestone Feb 18, 2024
@markstory
Copy link
Member

I like the idea. I think having comma lists makes sense for a bunch of command line applications.

The other reason: The type prefix makes it more clear what it is about. Beside existing "bool", the new "array" makes it more clear that this is still a single argument and not multiple options or arguments, single one with just a list..

I think we should align on a name for this behavior. multiple => 'comma' introduces new concepts for defining and reading options, but with different names. Could we align on commalist or comma and use it consistently?

$parser->addArgument('argument-list', [
    'commalist' => true,
]);

$args->getCommaListOption('names');
$args->getCommaListArgument('argument-list');

Using multiple and commalist together could be a runtime error. I think we're starting to see a pattern of wanting more ways to have the framework parse command line input. I don't have a well formed idea of how this API could work in a way that typechecks. But I could see this same approach being applied to parsing json input strings, or automatically read file inputs as other scenarios.

@Erwane
Copy link
Contributor

Erwane commented Feb 19, 2024

Personnaly i prefer separated options and know the type of data in the option.

$parser->addOption('option-list', [
            'multiple' => true,
            'separator' => ',', // Could be list_separator or multiple_separator
        ]);

The multiple option stay unchanged.
the separator option is a string, one character and , by default.

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

No branches or pull requests

3 participants