On Mon, Sep 28, 2026, Larry Garfield wrote: > Please remember to bottom-post. :-)
Yep, noted. Thanks. > I am in favor of this RFC in general. It addresses the sort of problem > that does belong in stdlib. > > - getSize()'s error return is null. Not false. False-on-error is an > anti-pattern we should be exterminating with extreme prejudice. Or > possibly an exception, but not false. That makes sense. The original extension used false more broadly, but for the core API the operational failures are already separate and throw TerminalException. I've changed getSize() to return ?TerminalSize, with null meaning that a usable native size couldn't be obtained. > - As I'm not familiar with the underlying OS tools... what is raw mode? > That seems to be just glossed over. It looks like the only useful API > method (readKey() ) requires going into raw mode, so I wonder what its > purpose is. Fair point. The RFC was assuming too much terminal background there. In canonical mode the terminal normally buffers input until a line is complete, may echo it, and handles some control characters itself. Raw mode turns off that line-oriented processing so the application can react to individual key presses and terminal sequences directly. One thing that also wasn't clear is that callers don't need to call enableRawMode() before readKey() or readSecret(). Both handle the temporary mode change internally. enableRawMode() is there for longer-running interactive code that wants to keep the terminal raw across multiple reads/redraws. I've clarified that in the RFC. > - That said, raw mode looks like a textbook case for a context manager. :-) Conceptually, yes. That's what I was trying to model with ModeToken: it represents the active lease, and restoreMode() gives an explicit way to release it in a try/finally. PHP doesn't have a general language-level context-manager construct, and I didn't want to add a terminal-specific callback abstraction just for this, so I kept the primitive explicit. > - Again, readKey() should return null, not false, for all the same reasons. Changed that as well. readKey() now returns Key|string|null, with null meaning the timeout expired before a complete input value was available. Operational failures still throw TerminalException. > - Why does readSecret() not need the same duration/timeout controls as > readKey()? There wasn't a good reason for the overall timeout to be missing, so readSecret() now accepts an optional Time\Duration too. It returns null when that timeout expires, while "" still means the user actually submitted an empty secret. I didn't add sequenceTimeout to readSecret(). Unlike readKey(), it doesn't expose terminal escape sequences to the caller, so that ambiguity handling can stay internal. > - I understand all of the usual arguments for making the Terminal class > final. However, it also has no interface. That means it's basically > impossible to mock for testing purposes. That strikes me as a problem, > because any IO boundary should be mockable. I don't know that multiple > non-testing implementations makes sense (maybe alternatives to the > static constructors?), but we do need some straightforward mechanism to > mock a Terminal object. [...] I agree with the testing concern. I've added TerminalInterface while keeping the native Terminal final, so application and library code can type against something that can be replaced by a userland fake. I also added ModeTokenInterface for fake implementations. The native Terminal still only accepts a native token belonging to the same logical terminal; foreign, stale, or unrelated tokens are rejected with ValueError. I've updated the RFC to match these changes and expanded the raw-mode/testability sections as well. Thanks for the review. Best, Pratik
