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` 
> 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 `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

Reply via email to