Refactor IO #80
Refactor IO #80
Conversation
Codecov Report
@@ Coverage Diff @@
## master #80 +/- ##
============================================
- Coverage 89.13% 88.75% -0.38%
- Complexity 246 267 +21
============================================
Files 18 21 +3
Lines 690 729 +39
============================================
+ Hits 615 647 +32
- Misses 75 82 +7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks good, the only thing I'd say is I agree with the $this->terminal->getOutput()->write($row) not being "right" ... reaching through feels wrong, I think it's probably cleaner just to have methods on the terminal class and remove the getOutput ?
Aside I was thinking that it kind of makes sense to properly pull this into php-school/terminal and use that in our packages instead, what do you think ?
|
|
||
| public function __toString() : string | ||
| { | ||
| return $this->fetch(); |
There was a problem hiding this comment.
looks like this would always return an empty string ?
There was a problem hiding this comment.
Nope - it will return whatever has been written so far and clean the buffer
There was a problem hiding this comment.
oh my dayz... obvs .. I must have been tired not to see that 😂
|
|
||
| public function __construct($stream = null) | ||
| { | ||
| if ($stream === null) { |
There was a problem hiding this comment.
maybe I'm being silly, can't you have STDIN as the default value?
There was a problem hiding this comment.
Let's give it a whirl
|
Makes sense @mikeymike I'll have a think about it - but it sees strange to duplicate the write method from the output stream on to the terminal - and should we also do the same with reading? Gonna take a look at some other packages |
|
Moved to php-school/terminal#1 |
Refactor the input + output to their own stream based handlers - allows to use with virtual terminals and in non-blocking apps. Inspired by https://www.reddit.com/r/PHP/comments/59qpk8/phpschoolclimenu_200/d9asoli/ and code from https://github.com/amphp/byte-stream.
It improves the tests by allowing phpunit output buffering to replaced by our own implementation. This is also helping me with the input tests a lot.
Only thing I'm not sure of is whether the terminal object should be the point of writing eg
$terminal->getOutput()->write('string');Also not sure if I care - we can change it later if we want, I am happy to hear suggestions though.