[Console] added a better way to ask questions to the user #10606
Conversation
|
rather than "not yet", maybe add a checkbox list with the TODO items |
|
@stof I'm working on your comment #9811 (comment) Do you have an idea about doing something different than what's been done in #8452? |
|
given that it is a new class (and so no BC concern), it would make sense to pass the input as an argument along the output IMO. We are already passing the output because we need to display the question. It would make sense to pass the input as well given that we are dealing with asking for input |
|
Alright, I've passed InputInterface and OutputInterface in the const 8000 ructor (as we do in the latest Table helper) |
|
The Table helper is different. This QuestionHelper is designed as a stateless service object (registered in the HelperSet) while the Table object is a stateful one-time object (you create a new Table instance for each table you render). This is why passing them as arguments might be better. I think that the concept of HelperSet could be deprecated in 3.0 if we push toward using DI in commands (the HelperSet is just a service locator); However, for this to be useful, we will probably need to change a bit the architecture: the output implementation is currently a service able to output text, not a value object representing the output like the Response is in Symfony. However, it is a scoped service as it is passed to the |
|
related to my previous comment, if you look at PhpSpec and Behat, which are both complex command-line apps requiring to be able to push output in different places (for instance in event listeners), they are both injecting the output as a synthetic service, which is what it really is (the input is not a service in them either) |
|
Hello, I've addressed comments and added unit tests. |
|
I just added a fix to the question implementation : When using a ChoiceQuestion, the result is not the index of the choice anymore, but its value. |
…ser (fabpot, romainneutron) This PR was merged into the 2.5-dev branch. Discussion ---------- [Console] added a better way to ask questions to the user | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | not yet This PR replaces #9811 - [x] add missing phpdocs - [x] add unit tests - [ ] submit doc PR Commits ------- 336bba2 [Console] Add docblocks and unit tests to QuestionHelper c413f89 [Console] added a better way to ask questions to the user
This PR was merged into the 2.5-dev branch. Discussion ---------- Fix travis build | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT Since #10287 and #10606 have been merged, errors occur in FrameworkBundle and Console component. This patch solves this. After merging 2.4 in master (that would fix Process suite), everything should be green. Commits ------- 099e480 Fix travis build
| * @param OutputInterface $output | ||
| * @param Question $question | ||
| * | ||
| * @return bool|mixed|null|string |
There was a problem hiding this comment.
Doesn't mixed imply the others?
…eutron) This PR was merged into the master branch. Discussion ---------- [Console] Add documentation for QuestionHelper | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#10606) | Applies to | 2.5+ This is the documentation for the new Question Helper Commits ------- f4ee337 [Console] Add documentation for QuestionHelper
This PR replaces #9811