Added i18n support to ConfirmationQuestion #11129
Conversation
|
there should be some validation that it is a 1-char string (otherwise it will never be true) |
|
@stof done. |
| { | ||
| parent::__construct($question, (bool) $default); | ||
|
|
||
| $this->setNormalizer($this->getDefaultNormalizer()); | ||
|
|
||
| if (1 === strlen($trueAnswer)) { |
There was a problem hiding this comment.
no, because the code compares it to the first byte of the input, not to the first unicode char
|
This needs to be covered by tests |
|
@wouterj why you creates limitation on answer length? Some languages could have more than one character for "yes" word. |
|
@wouterj the ConfirmationQuestion currently handles things the same way than most Unix commands do: anything starting with a |
|
@stof however, we might want to change to logic to check for the length for the given string. This way, languages which have the same start letter for yes and no can be validated nicely too |
|
I've updated it to allow multiple characters to compare |
|
Could be interested to have a RegExp pattern with default value |
| @@ -34,11 +38,12 @@ private function getDefaultNormalizer() | |||
| return $answer; | |||
| } | |||
|
|
|||
| $answerIsTrue = $this->trueAnswer === strtolower(substr($answer, 0, strlen($this->trueAnswer))); | |||
There was a problem hiding this comment.
$answerIsTrue = 0 === substr_compare($answer, $this->trueAnswer, 0, strlen($this->trueAnswer), true)
|
Updated to use a regex, great suggestion @jeremy-derusse ! I've also added a test. |
|
👍 |
|
A small doc PR (http://symfony.com/doc/current/components/console/helpers/questionhelper.html#asking-the-user-for-confirmation) will be great :) |
|
Thank you @wouterj. |
This PR was merged into the 3.0-dev branch. Discussion ---------- Added i18n support to ConfirmationQuestion | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#4879 For instance, when creating a dutch cli app, you want this to be `j` (from "Ja") instead of the english `y`. Commits ------- 3cb280d Added i18n support to ConfirmationQuestion
This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #11129). Discussion ---------- Added i18n support to ConfirmationQuestion | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#4879 For instance, when creating a dutch cli app, you want this to be `j` (from "Ja") instead of the english `y`. Commits ------- c2f3f89 Added i18n support to ConfirmationQuestion
This PR was merged into the 2.7 branch. Discussion ---------- Documented true regex | Q | A | --- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#11129) | Applies to | 2.7+ | Fixed tickets | - Commits ------- 6e6bae8 Small grammar-ish fix 050f7ce Documented true regex
For instance, when creating a dutch cli app, you want this to be
j(from "Ja") instead of the englishy.