8000 [Console] Fix validation of empty values using SymfonyQuestionHelper::ask() by chalasr · Pull Request #20141 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Console] Fix validation of empty values using SymfonyQuestionHelper::ask() #20141

< 8000 summary id="button-5f1405f213a1ba14" class="btn btn-sm btn-primary m-0 ml-0 ml-md-2" > 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

Merged
merged 1 commit into from
Oct 5, 2016

Conversation

chalasr
Copy link
Member
@chalasr chalasr commented Oct 3, 2016
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

When using QuestionHelper::ask() it's allowed to return an empty value as answer, e.g:

$helper = new QuestionHelper();
$question = new Question('foo', false);
$question->setValidator(function ($v) { return $v; });
$answer = $helper->ask($input, $output, $question);

Just typing enter for answering this question works, the value of $answer would be false.
But doing the same with SymfonyQuestionHelper::ask():

$helper = new SymfonyQuestionHelper();
$question = new Question('foo', false);
$question->setValidator(function ($v) { return $v; });
$answer = $helper->ask($input, $output, $question);

[ERROR] A value is required.

Same for '' or null.
Here I kept the same check but used as default validator, if a validator is set and allows an empty value to be returned then it's ok.

Also I am not sure about if this default validator should be kept, imho we should be consistent with the default question helper, using the SymfonyQuestionHelper should only impact the output.

Diff best viewed like this
ping @kbond

@chalasr chalasr changed the title [Console] Fix validation of null values using SymfonyStyle::ask() [Console] Fix validation of null values using SymfonyQuestionHelper::ask() Oct 3, 2016
@chalasr chalasr changed the base branch from master to 2.8 October 3, 2016 18:24
@chalasr
Copy link
Member Author
chalasr commented Oct 3, 2016

Failed build on hhvm not related.

@chalasr chalasr force-pushed the fix/sf_question_helper_null_val branch from 70f52de to b170780 Compare October 3, 2016 18:32
@chalasr chalasr force-pushed the fix/sf_question_helper_null_val branch 3 times, most recently from 522871c to 9132ae1 Compare October 3, 2016 19:42
@chalasr chalasr changed the base branch from 2.8 to 2.7 October 3, 2016 22:11
@chalasr chalasr force-pushed the fix/sf_question_helper_null_val branch from 9132ae1 to 8698602 Compare October 3, 2016 22:14
@chalasr chalasr force-pushed the fix/sf_question_helper_null_val branch from 8698602 to a8b910b Compare October 3, 2016 22:20
@chalasr chalasr changed the title [Console] Fix validation of null values using SymfonyQuestionHelper::ask() [Console] Fix validation of empty values using SymfonyQuestionHelper::ask() Oct 3, 2016
@fabpot
Copy link
Member
fabpot commented Oct 5, 2016

Thank you @chalasr.

@fabpot fabpot merged commit a8b910b into symfony:2.7 Oct 5, 2016
fabpot added a commit that referenced this pull request Oct 5, 2016
…tionHelper::ask() (chalasr)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix validation of empty values using SymfonyQuestionHelper::ask()

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  |no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

When using `QuestionHelper::ask()` it's allowed to return an empty value as answer, e.g:

```php
$helper = new QuestionHelper();
$question = new Question('foo', false);
$question->setValidator(function ($v) { return $v; });
$answer = $helper->ask($input, $output, $question);
```

Just typing `enter` for answering this question works, the value of `$answer` would be `false`.
But doing the same with `SymfonyQuestionHelper::ask()`:

```php
$helper = new SymfonyQuestionHelper();
$question = new Question('foo', false);
$question->setValidator(function ($v) { return $v; });
$answer = $helper->ask($input, $output, $question);
```

>  [ERROR] A value is required.

Same for `''` or `null`.
Here I kept the same check but used as default validator, if a validator is set and allows an empty value to be returned then it's ok.

Also I am not sure about if this default validator should be kept, imho we should be consistent with the default question helper, using the `SymfonyQuestionHelper` should only impact the output.

Diff best viewed [like this](https://github.com/symfony/symfony/pull/20141/files?w=1)
ping @kbond

Commits
-------

a8b910b [Console] Fix validation of null values using SymfonyStyle::ask()
@chalasr chalasr deleted the fix/sf_question_helper_null_val A52D branch October 5, 2016 07:10
@sstok
Copy link
Contributor
sstok commented Oct 5, 2016

👍 thank you very much, now I can remove some hacks I used, to make this work 😄

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

Successfully merging this pull request may close these issues.

4 participants
0