Re: [RFC] Io\Terminal
| From: | Larry Garfield | Date: | Fri, 02 Oct 2026 14:19:25 +0000 |
| Subject: | Re: [RFC] Io\Terminal | ||
| References: | 1 2 3 4 5 6 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-132770@lists.php.net to get a copy of this message | ||
On Thu, Oct 1, 2026, at 10:49 AM, Tim Düsterhus wrote:
> Hi
>
> On 2026-09-29 00:18, Larry Garfield wrote:
>> - 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 assume someone is going to respond with
>> "it's an implementation detail of something else," which is only
>> partially true; I don't want to have to create a pass-through wrapper
>> for something just for testing purposes, and even then, it would be the
>> same API in name only, since it cannot share a type. That hurts
>> interoperability.)
>
> I very strongly disagree with the interface suggestions for the exact
> “implementation detail” reason you mentioned and the introduction of the
> interface has made the API much worse:
>
> The (System)Terminal class *is* an implementation detail of something
> else and it is only exercised as part of the glue code within an
> integration test. It is not the IO boundary, the “TTY stream” you
> provide to the Terminal is. You need the TTY stream separately anyway,
> because the (System)Terminal does not provide any mechanism to write to
> the output (and the underlying stream is not exposed either). In your
> tests you would then attach an appropriate stream to the terminal (e.g..
> using
proc_open() with pty
> descriptors, which is what the RFC’s own
> implementation already uses for testing).
>
> This is very much like the final Random\Randomizer where you
> would
> have unit tests for the “number consumer”, passing hardcoded values. And
> then you have an integration test where you provide a class implementing
> the Random\Engine interface as the IO boundary. How the glue
> code
> turns a “Random\Engine” into a “random number” that is passed to the
> unit-tested “number consumer” is an implementation detail that is not
> worth testing.
>
> There are additional indicators that the SystemTerminal class
> is more
> akin to a “helper” class:
>
> 1. Large parts of the Terminal’s API surface don’t need to
> be instance
> methods, because they don’t really care about the object state (beyond
> the stream). They could also be bare functions and they were in the
> initial pre-RFC implementation. Having them as instance methods makes
> the API a little more convenient to use and enables the RAII reset.
>
> 2. The interface’s contract is “What SystemTerminal
> does”. As you say
> yourself, you are not sure if there can be multiple non-testing
> implementations, and I can't think of any either: The Terminal÷%Šu™tÖÍU
> ¢?h{
> interface does not define an “abstract concept” with multiple equally
> legal implementations.
>
> This is contrary to, say, Time\Clock where the Clock *is* the IO
> boundary and you can have SystemClock, GpsClock (when a GPS receiver is
> attached to your computer), HttpClock (fetching an HTTP service to
> obtain the current time, not useful, but theoretically meaningful), ….
>
> 3. The interface is humongous and not well-defined: Terminals expose a
> broad API surface and this is reflected by the SystemTerminal
> class
> and the RFC even cut out some of the methods that are there in the
> pre-RFC implementation. It is likely that the API surface will grow in
> future PHP versions with additional helper functionality. This also
> means that the interface needs to grow, which is an obvious breaking
> change. Or new non-well-defined interfaces need to be added, which is
> bad API design. In fact v0.3 of the RFC added a readLine()
> method
> which (at least on POSIX) is effectively redundant with
> fgets(), but
> allows consistent access to the “input side” of the terminal. It is not
> unlikely that a future PHP version might want to add
> readUntilEof() or
> similar.
>
> 4. The interface being a direct mapping of the Terminal API surface also
> resulted in the ModeToken interface being added, which is an
> interface
> for something that is an opaque “token” (value) object.. And then due to
> interface constraints / the lack of generics, the Terminal
> interface
> requires any Terminal to accept any
> ModeToken in the signature of
> restoreMode() just to validate that it is the ô
> ‡
¼ç0Ý鍸Êã›ModeToken
> implementation that belongs to the Terminal. The RFC specifies
> a
> ValueError here, but it really is a
> TypeError that cannot be
> expressed by the type system.
>
> 5. This has also resulted in the naming weirdness: The obvious name is
> “Terminal”, the “System” part in “SystemTerminal” carries no additional
> information.
>
> Best regards
> Tim Düsterhus
My main concern is being able to mock the terminal in order to effectively test code that uses the
terminal, without putting a proprietary thin wrapper around it (which largely defeats the purpose of
having a good API in core). Interfaces are the standard way of doing that. If you have a
suggestion for a better way, I'm happy to see it.
--Larry Garfield