Repository navigation
feat: split the CLI output into Output, Terminal, ProgressBar, Table and Color - #10628
paulbalandan wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
ANSI handling, table compatibility, test-state cleanup, static-analysis suppressions, and extension compatibility need resolution.
6 open findings
Final getOutput() method breaks existing command subclasses · New Typed protected output property breaks CLI subclasses · New COLORED_SEGMENT fails to match all SGR escape sequences · New Empty table rows render an extra border marker · New Windows mock state is not restored between tests · New Baseline suppresses invalid is_cli() mock calls · New
What changed in this PR
Splits CLI output responsibilities into injectable instance classes while retaining the static facade. Reviewed under the inferred 4.8 minor-branch policy.
Changes:
- Adds
Output,Terminal,ProgressBar,Table, andColor. - Propagates output instances through modern commands and silent child calls.
- Adds tests, documentation, upgrade notes, and changelog entries.
Validation: Reviewed all diffs and performed a focused WeakMap retention check; project test suites remain for CI.
| File | Description |
|---|---|
utils/phpstan-baseline/loader.neon |
Updates baseline count. |
utils/phpstan-baseline/arguments.count.neon |
Adds Output test suppressions. |
user_guide_src/source/testing/cli/002.php |
Demonstrates output-backed testing. |
user_guide_src/source/testing/cli.rst |
Documents modern command testing. |
user_guide_src/source/installation/upgrade_480.rst |
Adds migration guidance. |
user_guide_src/source/cli/cli_modern_commands/018.php |
Demonstrates modern output usage. |
user_guide_src/source/cli/cli_modern_commands.rst |
Documents command output APIs. |
user_guide_src/source/cli/cli_library/024.php |
Demonstrates new CLI classes. |
user_guide_src/source/cli/cli_library.rst |
Documents facade components. |
user_guide_src/source/changelogs/v4.8.0.rst |
Records API and behavior changes. |
tests/system/Test/TestCaseTest.php |
Updates CLI state reset. |
tests/system/CLI/TerminalTest.php |
Tests terminal detection. |
tests/system/CLI/TableTest.php |
Tests table rendering. |
tests/system/CLI/ProgressBarTest.php |
Tests progress behavior. |
tests/system/CLI/OutputTest.php |
Tests output transport and state. |
tests/system/CLI/NullInputOutputTest.php |
Updates transport restoration. |
tests/system/CLI/CommandsTest.php |
Tests output propagation and events. |
tests/system/CLI/ColorTest.php |
Tests ANSI handling. |
tests/system/CLI/CLITest.php |
Updates facade coverage. |
tests/system/CLI/AbstractCommandTest.php |
Tests command output lifecycle. |
tests/_support/Commands/Modern/SilentCallCommand.php |
Adds silent-call fixture. |
tests/_support/Commands/Modern/OutputProbeCommand.php |
Adds output-routing fixture. |
tests/_support/Commands/Modern/CallsAboutCommand.php |
Adds nested-command fixture. |
tests/_support/CLI/RecordingInputOutput.php |
Adds stream-recording transport. |
system/CLI/Terminal.php |
Encapsulates terminal capabilities. |
system/CLI/Table.php |
Extracts table rendering. |
system/CLI/ProgressBar.php |
Extracts progress rendering. |
system/CLI/Output.php |
Adds instance output writer. |
system/CLI/InputOutput.php |
Clarifies supported handles. |
system/CLI/Console.php |
Passes shared output to commands. |
system/CLI/Commands.php |
Routes output through command execution. |
system/CLI/Color.php |
Extracts ANSI operations. |
system/CLI/CLI.php |
Converts CLI into a facade. |
system/CLI/AbstractCommand.php |
Adds per-run output access. |
rector.php |
Excludes mock-only is_cli() calls. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cefbbff to
9c2dc68
Compare
…, `Table` and `Color`
9c2dc68 to
f7df2c6
Compare
michalsn
left a comment
There was a problem hiding this comment.
Overall, it looks solid - good direction.
| ``$this->getOutput()`` returns the :php:class:`Output <CodeIgniter\\CLI\\Output>` of the | ||
| current run. It is the same instance written to by the static ``CLI`` methods, so the two | ||
| can be mixed and the line breaks stay consistent. ``Output`` carries the writing | ||
| methods: ``write()``, ``print()``, ``error()``, ``newLine()``, ``beep()``, | ||
| ``clearScreen()``, and ``color()``. Tables and progress bars are rendered by | ||
| :php:class:`Table <CodeIgniter\\CLI\\Table>` and :php:class:`ProgressBar <CodeIgniter\\CLI\\ProgressBar>`: |
There was a problem hiding this comment.
We recommend replacing a CLI::color() override with an Output subclass. However, overriding Output::color() does not affect print(), write(), or error(): these methods call the private colorize() method, which delegates directly to Color::apply().
This means the suggested migration does not preserve custom coloring behavior. Not sure if anyone ever used such functionality, but we should at least explain which methods need to be overridden.



Description
The static
CLIclass mixes five jobs: writing to streams, colouring text, laying out tables, drawing a progress bar, and probing the terminal. Its state is process-global, so a command cannot own its output,callSilently()swaps that state by hand, and the planned verbosity levels would add more statics.This PR moves each job into an instance class and turns
CLIinto a facade that delegates to them. EveryCLI::write()keeps working, and the output is byte-identical apart from the behaviour changes listed below.Outputis the writer:print(),write(),error(),newLine(),beep(),clearScreen(),color(). It targets STDOUT, sendserror()to STDERR, detects colour support per stream, and tracks whether the cursor sits at the start of a line.forStderr()returns a copy whose STDOUT writes go to STDERR.Terminalholds the dimensions (read once per instance) and thehasColorSupport()/streamSupports()probes.ProgressBardraws the ten-cell bar and rejects a negative step or a total below one.Tablerenders the bordered table as a string.Colorapplies and strips ANSI codes.CLI::getOutput()andCLI::getTerminal()return the shared instances, andCLI::setOutput()replaces the writer. The facade keeps every released signature and normalises its inputs (?stringcolours,showProgress(false), negative or zero progress values) before delegating. The output-related protected statics stay declared, deprecated and unused.$this->getOutput().AbstractCommand::run()andCommands::runCommand()accept anOutput, whichConsolepasses down, sub-commands inherit, andcallSilently()replaces with one backed byNullInputOutput. The facade is swapped for the duration of the run and restored afterwards, so legacy writers see the same instance.Behaviour changes, documented in the changelog and the upgrade guide:
CLI::beep()goes through theInputOutputtransport instead ofecho.CLI::print()of text ending in a line feed, or a progress bar, no longer causes a blank line before the nextCLI::write().CLI::error()no longer leaves colour support disabled after an invalid colour name, and an empty foreground with a background no longer throws.CLI::showProgress()past the total draws a full bar instead of throwing aValueError.CLIoverridingfwrite()orcolor(), or reading the deprecated statics, no longer affects the output.The framework commands keep using the
CLIfacade. Migrating them to$this->getOutput(), moving the prompts out ofCLI, and modernising the colour detection are follow-up PRs.Checklist: