[Console] Make SymfonyQuestionHelper::ask optional by default #22317

ro0NL · 2017-04-06T16:52:04Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes (nothing in core depends on it)
Fixed tickets	#...
License	MIT
Doc PR	symfony/symfony-docs#...

i noticed when writing commands i always keep doing

$io = new SymfonyStyle($input, $output);
$answer = $io->ask('...', null, function ($value) { return $value; });

// instead of just
$answer = $io->ask('...');

only to bypass a built-in validation, of which im not sure why it's there. Note the base question helper doesnt make this assumption...

chalasr · 2017-04-06T16:57:09Z

👍 Already pointed out in #20141 when making it more flexible, I don't know why this default validator has been added, console's Symfony* stuff should only be about output styling, not input filtering.

ro0NL · 2017-04-06T17:32:52Z

src/Symfony/Component/Console/Helper/SymfonyQuestionHelper.php

@@ -29,6 +29,8 @@ class SymfonyQuestionHelper extends QuestionHelper
 {
    /**
     * {@inheritdoc}
+     *
+     * @deprecated since version 3.3, to be removed in 4.0


Not sure about this.. it creates some nasty strikethroughs in IDE's, and the same method call will work in 4.0

Indeed, I think I would talk only about the removed validation. Btw this needs to be documented :)

nicolas-grekas · 2017-04-28T17:46:59Z

Moving to milestone 3.4 as this is not a blocker for 3.3. If you don't agree, please tell :)

ro0NL · 2017-04-28T17:50:03Z

Just annoying :) 👍 for 3.4, im used doing this already.

fabpot

Target should also be changed to 3.4.

fabpot · 2017-07-20T17:27:09Z

UPGRADE-3.3.md

@@ -72,6 +72,8 @@ Console
   have been deprecated in favor of the `console.error` event and the `ConsoleErrorEvent`
   class. The deprecated event and class will be removed in 4.0.

+ * The `SymfonyQuestionHelper::ask` default validation has been deprecated and will be removed in 4.0. Apply validation using `Question::setValidator` instead.


should be moved to 3.4.

fabpot · 2017-07-20T17:27:20Z

src/Symfony/Component/Console/Helper/SymfonyQuestionHelper.php

@@ -39,6 +41,8 @@ public function ask(InputInterface $input, OutputInterface $output, Question $qu
            } else {
                // make required
                if (!is_array($value) && !is_bool($value) && 0 === strlen($value)) {
+                    @trigger_error('The default question validator is deprecated since Symfony 3.3 and will not be used anymore in version 4.0. Set a custom question validator if needed.', E_USER_DEPRECATED);


3.4 instead of 3.3

…default (ro0NL) This PR was merged into the 4.0-dev branch. Discussion ---------- [Console] Make SymfonyQuestionHelper::ask optional by default | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes (nothing in core depends on it) | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  i noticed when writing commands i always keep doing ```php $io = new SymfonyStyle($input, $output); $answer = $io->ask('...', null, function ($value) { return $value; }); // instead of just $answer = $io->ask('...'); ``` only to bypass a built-in validation, of which im not sure why it's there. Note the base question helper doesnt make this assumption... Commits ------- 2da429c [Console] Make SymfonyQuestionHelper::ask optional by default

…nal by default (ro0NL) This PR was merged into the 4.0-dev branch. Discussion ---------- [Console] Make SymfonyQuestionHelper::ask optional by default | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes (nothing in core depends on it) | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  i noticed when writing commands i always keep doing ```php $io = new SymfonyStyle($input, $output); $answer = $io->ask('...', null, function ($value) { return $value; }); // instead of just $answer = $io->ask('...'); ``` only to bypass a built-in validation, of which im not sure why it's there. Note the base question helper doesnt make this assumption... Commits ------- 2da429c [Console] Make SymfonyQuestionHelper::ask optional by default

chalasr · 2017-07-22T18:24:11Z

Thank you @ro0NL

nicolas-grekas · 2017-07-26T06:50:14Z

src/Symfony/Component/Console/Helper/SymfonyQuestionHelper.php

@@ -39,6 +41,8 @@ public function ask(InputInterface $input, OutputInterface $output, Question $qu
            } else {
                // make required
                if (!is_array($value) && !is_bool($value) && 0 === strlen($value)) {
+                    @trigger_error('The default question validator is deprecated since Symfony 3.4 and will not be used anymore in version 4.0. Set a custom question validator instead.', E_USER_DEPRECATED);


@ro0NL is this really in the correct code path? Throwing both a deprecation and an exception is suspicious to me.

We're deprecating the fact this exception is thrown when the value is blank. Meaning in 4.0 ask() is optional by default, whereas now it's required by default.

I actually considered putting the trigger above if (!is_array($value) && !is_bool($value) && 0 === strlen($value)) {, but its weird to get the deprecation for a case that is still valid in 4.0.

Indeed, I think it's the right place for this deprecation.

@deprecated

* 3.4: [DI] use assertStringEqualsFile when possible [VarDumper] Adapt to php 7.2 changes [DI] Fix using private services in expressions [Form][TwigBridge] Don't render _method in form_rest() for a child form [Form] Static call TimezoneType::getTimezones Removed references for non existent validator constraints Suggest using quotes instead of Yaml::PARSE_KEYS_AS_STRINGS [DI] Fix test [Cache] Handle unserialization failures for Memcached Remove unused prop + added @deprecated Remove unused mocks/vars feature #22317 [Console] Make SymfonyQuestionHelper::ask optional by default (ro0NL) [DoctrineBridge][PropertyInfo] Added support for Doctrine Embeddables [Validator] Fix IbanValidator for ukrainian IBANs Router: allow HEAD method to be defined first [WebProfilerBundle] Display trace and context in the logger profiler Fixing a bug where if a core class was autowired, autowiring tried to autowire optional args as if they were required

@deprecated

* 3.4: [DI] use assertStringEqualsFile when possible [VarDumper] Adapt to php 7.2 changes [DI] Fix using private services in expressions [Form][TwigBridge] Don't render _method in form_rest() for a child form [Form] Static call TimezoneType::getTimezones Removed references for non existent validator constraints Suggest using quotes instead of Yaml::PARSE_KEYS_AS_STRINGS [DI] Fix test [Cache] Handle unserialization failures for Memcached Remove unused prop + added @deprecated Remove unused mocks/vars feature symfony#22317 [Console] Make SymfonyQuestionHelper::ask optional by default (ro0NL) [DoctrineBridge][PropertyInfo] Added support for Doctrine Embeddables [Validator] Fix IbanValidator for ukrainian IBANs Router: allow HEAD method to be defined first [WebProfilerBundle] Display trace and context in the logger profiler Fixing a bug where if a core class was autowired, autowiring tried to autowire optional args as if they were required

carsonbot added Status: Needs Review Console Feature Deprecation labels Apr 6, 2017

ro0NL commented Apr 6, 2017

View reviewed changes

nicolas-grekas added this to the 3.3 milestone Apr 7, 2017

nicolas-grekas modified the milestones: 3.3, 3.4 Apr 28, 2017

chalasr approved these changes Jul 20, 2017

View reviewed changes

fabpot approved these changes Jul 20, 2017

View reviewed changes

fabpot requested changes Jul 20, 2017

View reviewed changes

[Console] Make SymfonyQuestionHelper::ask optional by default

a3c07f3

ro0NL changed the base branch from master to 3.4 July 20, 2017 18:57

chalasr closed this Jul 22, 2017

ro0NL deleted the console/sf-question branch July 22, 2017 18:56

nicolas-grekas reviewed Jul 26, 2017

View reviewed changes

fabpot mentioned this pull request Oct 19, 2017

Release v4.0.0-BETA1 #24612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Make SymfonyQuestionHelper::ask optional by default #22317

[Console] Make SymfonyQuestionHelper::ask optional by default #22317

ro0NL commented Apr 6, 2017

chalasr commented Apr 6, 2017

ro0NL Apr 6, 2017

chalasr Apr 6, 2017

nicolas-grekas commented Apr 28, 2017

ro0NL commented Apr 28, 2017

fabpot left a comment

fabpot Jul 20, 2017

fabpot Jul 20, 2017

chalasr commented Jul 22, 2017

nicolas-grekas Jul 26, 2017

chalasr Jul 26, 2017 •

edited

Loading

ro0NL Jul 26, 2017 •

edited

Loading

ogizanagi Jul 26, 2017

[Console] Make SymfonyQuestionHelper::ask optional by default #22317

[Console] Make SymfonyQuestionHelper::ask optional by default #22317

Conversation

ro0NL commented Apr 6, 2017

chalasr commented Apr 6, 2017

ro0NL Apr 6, 2017

Choose a reason for hiding this comment

chalasr Apr 6, 2017

Choose a reason for hiding this comment

nicolas-grekas commented Apr 28, 2017

ro0NL commented Apr 28, 2017

fabpot left a comment

Choose a reason for hiding this comment

fabpot Jul 20, 2017

Choose a reason for hiding this comment

fabpot Jul 20, 2017

Choose a reason for hiding this comment

chalasr commented Jul 22, 2017

nicolas-grekas Jul 26, 2017

Choose a reason for hiding this comment

chalasr Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

ro0NL Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

ogizanagi Jul 26, 2017

Choose a reason for hiding this comment

chalasr Jul 26, 2017 •

edited

Loading

ro0NL Jul 26, 2017 •

edited

Loading