Hi Sjoerd, I've pushed fixes for the issues you pointed out, along with regression tests.
Zero-timeout reads now handle already-available escape sequences without waiting, and getSize() no longer suppresses a pending resize notification. While testing, I also found that incomplete UTF-8 at EOF could cause readKey() to keep returning null. It now throws TerminalException on EOF, while genuine timeouts still preserve the partial bytes. https://github.com/php/php-src/pull/23941 Thanks again for the careful review. Regards, Pratik On Fri, 23 Oct 2026 16:44:51 -0700, Pratik Bhujel <[email protected]> wrote: > Hi Sjoerd, > > Thanks for taking a look. > Perhaps it would be desirable to distinguish between the user cancelling, and > a genuine error, and perhaps also EOF; for example with subclasses of > TerminalException. > Yes, I see the use case. An application may handle a cancelled password > prompt quietly but report an I/O failure. Currently they all throw > TerminalException, and the messages are not stable identifiers. Separate > subclasses would allow that distinction while keeping TerminalException as > the common catch. I would like to settle this before the vote. > Is this sufficiently long, even when used over SSH or such? Is this > configurable in readSecret as well? > The 25 ms starts after Escape arrives, so it is not a limit on the SSH round > trip. Delayed fragments can still exceed it. readKey() lets you increase > sequenceTimeout, within the overall timeout. > > readSecret() currently uses a fixed 25 ms wait in its byte-stream escape > handling. It is not separately configurable, and increasing the overall > timeout does not increase that wait. That is a limitation worth addressing. > Native Windows console input uses input records, so this escape-sequence > timing does not apply there. > If I resize the terminal twice, do I get two Key::Resize? getSize() seems to > update the stored size of the terminal. Do I still get a Key::Resize after I > call getSize()? > Not necessarily two. On POSIX, resize notifications can be coalesced, and > changes between reads can be detected by comparing dimensions. > > You are right about getSize(). It updates the same cached dimensions, so > calling it first can hide a change from that comparison. Windows can still > have queued resize records. I think getSize() should query the size without > acknowledging a pending notification. That needs fixing and documenting. > Resize should mean "check the current size", rather than promise a count of > every resize. > This would not work with escape sequences, right? Or does it work correctly > if the key is already buffered? > Reading the current byte-stream decoder, the zero deadline makes it return > Escape before checking the remaining bytes, even if the whole sequence is > buffered. The focused zero-timeout test covers a lone Escape. > > Zero should prevent waiting while still allowing a complete available > sequence to be decoded. That needs a focused regression test and a fix. > > I’ll follow up with the test results and changes. These findings are from > checking the code; I haven’t run the new reproducers yet. > > Regards, > Pratik
