-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DialogHelper] Multiselect : added an option to the "select" function that makes the possibility of multiselect options. #7602
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
Conversation
* | ||
* @throws \InvalidArgumentException | ||
*/ | ||
public function select(OutputInterface $output, $question, $choices, $default = null, $attempts = false, $errorMessage = 'Value "%s" is invalid') | ||
public function select(OutputInterface $output, $question, $choices, $default = null, $attempts = false, $multiselect = false, $errorMessage = 'Value "%s" is invalid') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the parameter in the middle of the signature is a BC break
Can you take @stof comments into account? |
of course, I'm working on it. |
@quiqueporta Please also update the PR description with the PR summary |
OK, thanks ;) |
Fixed the function returns. Fixed the Test
@stof now the multiselect option return an array of key => value. $colors = ['red','blue','orange'];
// Make the select with multiselect option true.
// and select 0,2 options.
// The select returns this ...
Array
(
[0] => 'red'
[2] => 'orange'
) Would it be better to return only the keys? $colors = ['red','blue','orange'];
// Make the select with multiselect option true.
// and select 0,2 options.
// The select returns this ...
Array
(
[0] => 0
[1] => 2
) |
it should indeed return keys to be consistent with the behavior of the non-multiple version |
Ok, i will make the modification and push it ;) |
@stof , ok i make the modificacions ;) |
if (empty($choices[$picked])) { | ||
throw new \InvalidArgumentException(sprintf($errorMessage, $picked)); | ||
$result = $this->askAndValidate($output, '> ', function ($picked) use ($choices, $errorMessage, $multiselect) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this blank line should be removed.
Yes to all |
throw new \InvalidArgumentException(sprintf($errorMessage, $picked)); | ||
$result = $this->askAndValidate($output, '> ', function ($picked) use ($choices, $errorMessage, $multiselect) { | ||
// Collapse all spaces. | ||
$selectedChoices = str_replace(" ","", $picked); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need spaces before ""
for CS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good catch.
When the option list appears, you can select more than one value separated by comma.