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

Reply via email to