-
Notifications
You must be signed in to change notification settings - Fork 32
Conversation
c7235fe
to
f1205cb
Compare
/dev/stty
instead of php://input
if the latter is closedInput
class
@jubianchi, @Metalaka: Please, review it. I will add tests today. |
f1205cb
to
a630f3a
Compare
Ready for a review! |
self::$_old = Processus::execute('stty -g'); | ||
Processus::execute('stty -echo -icanon min 1 time 0'); | ||
self::$_old = Processus::execute('stty -f /dev/tty -g'); | ||
Processus::execute('stty -f /dev/tty -echo -icanon min 1 time 0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still don't know what to do here. -F
on Linux, -f
on BSD. Any idea how to test the OS efficiently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am installing an Ubuntu OS to try finding a solution.
1bf66f9
to
836ad5c
Compare
* | ||
* @param int $length Length. | ||
* @return string | ||
* @throws \Hoa\Exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the underlying method we wrap can throw an exception, so…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this method don't throw any things, @throws
tag is present in Hoa\File\Read
and don't need to be duplicated.
I found a way to solve the issue
|
We are ready for a merge. @Metalaka, @jubianchi: Is it OK for you? |
|
I guess we can merge this patch before, though? This is safe enough for now. |
Ok, let's go. |
By default, the advanced interaction is not enabled if `STDIN` is not direct. We can now bypass this guard in particular cases (for instance if `STDIN` is closed and we would like to read from `/dev/tty`).
In case `STDIN` has been closed, `stty` will not be able to act as expected. So we force it to use the `/dev/tty` file instead of the regular standard input.
Exactly like the `Output` class, the `Input` interface represents a stream of the program, here the input stream. We then create the `Console::setInput` and `Console::getInput` methods. By default, if no `Input` instance is provided, an `Hoa\File\Read` instance will be used representing `php://stdin`. If it has been closed then `/dev/tty` will be used instead. This way, we are ensured methods will behave as expected, whether or not the standard input has been closed or not.
The `getStream` method allows to access to the underlying stream that the `Input` class wraps.
3ea97e2
to
edd3d3a
Compare
We still have an issue with |
On Linux, this is the `stty -F $file` or `stty --file $file` options. On BSD, this is the `stty -f $file` option. There is a conflict here and this is difficult to have something cross-platform. Fortunately, both architectures use the standard input as the `-F`, `--file` or `-f` value. So writing `stty < $file` seems to fix the issue.
As @Metalaka suggests, we can remove the `@throws` clause.
edd3d3a
to
9600ab3
Compare
Fixed with this commit. |
Old title
Imagine this case:
where
foo.php
is a program that reads fromSTDIN
. It works perfectly withHoa\Console
.Now, imagine this other case:
STDIN
will containfoo
andbar
but theHoa\Console\Cursor
—for instance— will no longer work since it reads fromSTDIN
too.The idea is then to close
STDIN
(akaphp://input
) and open/dev/tty
, which is the real standard input in this case.So, to address these needs, we introduce the
Hoa\Console::getStdin
method that is a helper to choose the appopriate standard input. We also change theHoa\Console::advancedInteraction
behavior: 👷stty
uses the-f
option to specify/dev/tty
instead of the default standard input. However, on Linux, this is-F
. I don't know how to address this right now (I mean: Making something cross-platform).Finally, all
Hoa\Console
's classes that were usingSTDIN
directly are now usingHoa\Console::getStdin
.New title
Actually, since #55, this PR has taken a new path. We also introduced an
Input
class. And it considers whether to useHoa\File\Read('php://stdin')
as the default input, orHoa\File\Read('/dev/tty')
in case the former has been closed. This is a basic wrapper.