[Console] Use readline for user input when available #DX #15382
Conversation
9b158e2 to
6f175b7
Compare
| */ | ||
| private function hasReadline() | ||
| { | ||
| return function_exists('readline'); |
There was a problem hiding this comment.
I would inline that instead of creating a method for that.
|
👍 |
ea1e0b4 to
aeead4d
Compare
|
@fabpot I inlined the check on the availability of readline. |
| throw new \RuntimeException('Aborted'); | ||
| } | ||
|
|
||
| $ret = trim($ret); |
There was a problem hiding this comment.
What about using stream_get_line() for platforms not supporting readline()? We could then get rid of the trim() call.
There was a problem hiding this comment.
By the way, the behaviour is now inconsistent as the string returned by readline() is not trimmed (for example, when containing extra spaces before or after the actual input).
There was a problem hiding this comment.
Thanks for your review.
I didn't know about stream_get_line() method. Would there be an advantage of using it on platforms not supporting readline()? The string would need to be trimmed as well if we want to keep the same behavior (removing whitespaces around the string) and arrow keys can't be used as well, so it doesn't seem there is any benefit.
I agree with you on the second point I should trim the string both with readline and with the usual fgets functions.
|
Status: Needs work |
This allows to use arrow keys in the terminal instead of having weird characters
aeead4d to
0534899
Compare
|
@xabbuh I made the change according to you second comment. My comments about your first one are above. |
|
@michaelperrin I could imagine that people find Status: Reviewed |
|
Thank you @michaelperrin. |
…DX (michaelperrin) This PR was merged into the 2.8 branch. Discussion ---------- [Console] Use readline for user input when available #DX | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Given I am entering data in an user input When I use left and right keys to make some changes in what I have typed Then the cursor should move accordingly instead of adding characters at the end of the line To make it simple: using the arrow keys (← →) to make changes to what I already typed would be much handier than getting `^[[D` and `^[[C` characters in the terminal and having to delete all chars to type everything again. I could not add any extra tests to this as the STDIN can't be used during tests. But they are not breaking and I tried again all types of questions (text, choices, hidden) by myself. Note that `readline` can't be used for hidden questions, as `stty -echo` is not taken into account. Commits ------- 0534899 [Console] Fix Symfony coding standards violations 8b63d62 [Console] Use readline for user input when available
|
Thank you! |
|
this is missing the handling of |
|
@stof |
|
The comments on the documentation page of the function are saying it does: http://php.net/manual/en/function.readline.php#86719 |
…elperrin) This PR was merged into the 2.8 branch. Discussion ---------- [Console] Handle false return value from readline | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT Following #15382 and as @stof mentioned, the `false` value can be returned by `readline()` and should be handled. This fixes the problem. Commits ------- f5ca270 [Console] Handle false return value from readline
Given I am entering data in an user input
When I use left and right keys to make some changes in what I have typed
Then the cursor should move accordingly instead of adding characters at the end of the line
To make it simple: using the arrow keys (← →) to make changes to what I already typed would be much handier than getting
^[[Dand^[[Ccharacters in the terminal and having to delete all chars to type everything again.I could not add any extra tests to this as the STDIN can't be used during tests. But they are not breaking and I tried again all types of questions (text, choices, hidden) by myself.
Note that
readlinecan't be used for hidden questions, asstty -echois not taken into account.