Converged naming, structure, API shape and the comment register across the library. - #154
Conversation
…x) like its siblings.
|
🚀 Deployed on https://6a7bb706d7054593688321cd--tui-docs.netlify.app |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
==========================================
- Coverage 98.14% 98.05% -0.10%
==========================================
Files 150 150
Lines 6033 6054 +21
==========================================
+ Hits 5921 5936 +15
- Misses 112 118 +6 ☔ View full report in Codecov by Harness. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (44)
📝 WalkthroughWalkthroughThis change aligns option, markup, terminal, theme, layout, exception, and rendering APIs. It also updates documentation, tests, examples, and playground themes. ChangesAPI and implementation alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Block/AbstractBlock.php`:
- Around line 14-16: Update the AbstractBlock class documentation describing
elements() so it names \InvalidArgumentException instead of \TypeError when the
theme lacks a declared element interface, while preserving the existing behavior
description.
In `@src/Block/Field.php`:
- Around line 2341-2342: Update the PHPDoc for entryLine() so the $elements
parameter description accurately identifies it as the narrowed field elements,
replacing the incorrect “The theme.” text while preserving the declared
FieldElementsInterface type.
In `@src/Block/Panel.php`:
- Around line 392-395: Update the documentation for Panel::ids() to describe
only the IDs it returns: those from Field, Markup, and Progress blocks.
Explicitly state that IDs belonging to nested Panel blocks are excluded.
In `@src/Block/Tree.php`:
- Around line 14-16: Correct the ownership documentation near Tree so it no
longer claims the headless answer snapshot uses Tree’s flattening traversal.
Keep the implementation unchanged unless deliberately updating
Answers::forTree() to delegate to Tree is required; ensure the comment
accurately describes only the consumers that actually use Tree.
In `@src/Builder/FieldBuilder.php`:
- Around line 29-31: Update the documentation sentence near the builder
declaration to replace the malformed phrase “and one the kind has nowhere to
put” with grammatically complete wording that clearly states declarations
without a valid answer kind are rejected at their original location.
In `@src/Field/Capability/OptionsCapableTrait.php`:
- Around line 15-18: Update the cursor invariant documentation in
OptionsCapableTrait to state that the cursor points to a selectable row when one
exists, and explicitly document that firstSelectable() falls back to index 0
when no row is selectable, which may reference a non-selectable row or no row.
In `@src/Field/Rating.php`:
- Around line 97-98: Update the documentation near applyChar() to clarify that
digit input selects values 0–9 that fall within the configured scale; scales
with min <= 9 and max > 9 remain partially selectable, while values above 9
cannot be selected by digit input. Remove wording that implies every scale
extending past nine is inert.
In `@src/Field/Reorder.php`:
- Around line 95-96: Update the comments in Reorder::handle() around the Accept,
Grab, MoveUp, and MoveDown actions to describe action names and movement
directions rather than fixed Space, Up, or Down keys. Keep the comments accurate
for custom ScopedKeyMap bindings, including the note that accepting a drop
commits no change mid-move.
In `@src/Screen/Capability/ScrollCapableTrait.php`:
- Around line 8-15: Add an `@throws \LogicException` declaration to the
`ScrollCapableInterface::scrollTo()` method documentation, matching the
exception behavior implemented by `ScrollCapableTrait::scrollTo()`.
In `@src/Terminal/Ansi.php`:
- Around line 67-69: Update the comment in Ansi::link() to state that
stripControl() removes C0 control bytes and DEL, rather than claiming every
control byte is dropped; do not change the filter unless the implementation is
explicitly required to reject C1 controls as well.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4d172b2b-bd71-4d52-b23b-6562598b5273
📒 Files selected for processing (82)
src/Answers/Answers.phpsrc/Answers/Provenance.phpsrc/Answers/SummaryFormatter.phpsrc/Answers/ValueFormatter.phpsrc/Block/AbstractBlock.phpsrc/Block/BlockInterface.phpsrc/Block/Breadcrumb.phpsrc/Block/Capability/ActivateCapableInterface.phpsrc/Block/Field.phpsrc/Block/FieldType.phpsrc/Block/Legend.phpsrc/Block/Markup.phpsrc/Block/Option.phpsrc/Block/Panel.phpsrc/Block/Progress.phpsrc/Block/TableSpec.phpsrc/Block/Tree.phpsrc/Builder/FieldBuilder.phpsrc/Builder/Form.phpsrc/Builder/PanelBuilder.phpsrc/CancelException.phpsrc/CollectException.phpsrc/Discovery/Dotenv.phpsrc/Discovery/Scan.phpsrc/Field/AbstractField.phpsrc/Field/Calendar.phpsrc/Field/Capability/CompletionCapableTrait.phpsrc/Field/Capability/FilterCapableInterface.phpsrc/Field/Capability/FilterCapableTrait.phpsrc/Field/Capability/OptionsCapableInterface.phpsrc/Field/Capability/OptionsCapableTrait.phpsrc/Field/Capability/PagingCapableTrait.phpsrc/Field/Capability/PlaceholderCapableTrait.phpsrc/Field/Capability/QueryOptionsCapableInterface.phpsrc/Field/Capability/QueryOptionsCapableTrait.phpsrc/Field/Capability/SelectionBoundedTrait.phpsrc/Field/Capability/SelectionCapableInterface.phpsrc/Field/Capability/SelectionCapableTrait.phpsrc/Field/Capability/TextEditCapableInterface.phpsrc/Field/Capability/TextEditCapableTrait.phpsrc/Field/FieldFactory.phpsrc/Field/FilePicker.phpsrc/Field/Rating.phpsrc/Field/Reorder.phpsrc/Field/Search.phpsrc/Field/Suggest.phpsrc/Field/Template.phpsrc/FormException.phpsrc/Handler/HandlerRegistry.phpsrc/Input/Action.phpsrc/Input/Binding.phpsrc/Input/KeyMap.phpsrc/Input/KeyParser.phpsrc/Resolver/InputResolver.phpsrc/Schema/AgentHelp.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Screen/Capability/BorderCapableInterface.phpsrc/Screen/Capability/BorderCapableTrait.phpsrc/Screen/Capability/ScrollCapableInterface.phpsrc/Screen/Capability/ScrollCapableTrait.phpsrc/Screen/Collector.phpsrc/Screen/ExternalEditor.phpsrc/Screen/KeyRouter.phpsrc/Screen/Layout/AbstractLayout.phpsrc/Screen/Layout/LayoutManager.phpsrc/Screen/Overlay.phpsrc/Screen/Region.phpsrc/Screen/Scroller.phpsrc/Terminal/Ansi.phpsrc/Terminal/Terminal.phpsrc/Testing/TuiTester.phpsrc/Theme/DefaultTheme.phpsrc/Translation/Translator.phpsrc/Tui.phpsrc/Utils/Strings.phptests/phpunit/Unit/Block/EntryTest.phptests/phpunit/Unit/Block/FieldBlockTest.phptests/phpunit/Unit/Block/FieldDeclarationTest.phptests/phpunit/Unit/Schema/AgentHelpTest.phptests/phpunit/Unit/Translation/TranslationRenderTest.phptests/phpunit/Unit/TuiTest.php
| * Holds a surface's offset; scrollTo() throws for one that does not scroll. | ||
| * | ||
| * @package DrevOps\Tui\Screen\Capability | ||
| */ | ||
| trait ScrollCapableTrait { | ||
|
|
||
| /** | ||
| * Whether this surface's contents may outrun it. | ||
| * Whether this surface's contents may overflow it. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the scrollTo() exception on the interface.
ScrollCapableTrait::scrollTo() throws \LogicException when the surface is not scrollable. ScrollCapableInterface::scrollTo() does not declare this exception. Add @throws \LogicException to the interface method so interface callers see the runtime contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Screen/Capability/ScrollCapableTrait.php` around lines 8 - 15, Add an
`@throws \LogicException` declaration to the
`ScrollCapableInterface::scrollTo()` method documentation, matching the
exception behavior implemented by `ScrollCapableTrait::scrollTo()`.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Terminal/Ansi.php (1)
32-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDescribe the scope of
STYLE_SEQUENCEaccurately.
STYLE_SEQUENCEmatches CSI sequences whose final byte is any ASCII letter. That set includes terminal controls other than styling commands. The current text therefore overstates the styling guarantee. Describe the sequence as one recognized byAnsi::strip(), or narrow the pattern if only styling commands are intended.Proposed wording
- * A CSI sequence; CSI sequences carry the styling. + * A CSI sequence recognized by the ANSI escape-sequence stripper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Terminal/Ansi.php` at line 32, Update the documentation for STYLE_SEQUENCE in Ansi::strip() to accurately state that it matches recognized CSI sequences, including non-styling terminal controls; do not describe all matched sequences as styling-only unless the pattern is narrowed accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/Terminal/Ansi.php`:
- Line 32: Update the documentation for STYLE_SEQUENCE in Ansi::strip() to
accurately state that it matches recognized CSI sequences, including non-styling
terminal controls; do not describe all matched sequences as styling-only unless
the pattern is narrowed accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f63d5ba0-1374-4e71-9d93-3563d8fddc2d
📒 Files selected for processing (9)
src/Block/AbstractBlock.phpsrc/Block/Field.phpsrc/Block/Panel.phpsrc/Builder/FieldBuilder.phpsrc/Field/Capability/OptionsCapableTrait.phpsrc/Field/Rating.phpsrc/Field/Reorder.phpsrc/Screen/Capability/ScrollCapableInterface.phpsrc/Terminal/Ansi.php
…he move verb on 'moveCursor'.
…he width/height dialect.
…ome for the indicator rule.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Field/FilePicker.php (1)
289-303: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn the Toggle hint only in multiple mode.
When
$this->multipleisFALSE,hints()still returnsnew Hint('select', Action::Toggle). The hint advertises an action that does not affect the single-selection value. Add it only when$this->multipleisTRUE.The supplied
hints()andliveValue()implementations provide this evidence.Proposed fix
- return [ - new Hint('select', Action::Toggle), + $hints = [ new Hint('move', Action::MoveUp, Action::MoveDown), new Hint('open', Action::MoveRight), new Hint('go up', Action::MoveLeft), new Hint($this->multiple ? 'accept' : 'select', Action::Accept), new Hint('show hidden', Action::Reveal), new Hint('cancel', Action::Cancel), ]; + + if ($this->multiple) { + array_unshift($hints, new Hint('select', Action::Toggle)); + } + + return $hints;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Field/FilePicker.php` around lines 289 - 303, Update FilePicker::hints() so the Action::Toggle “select” hint is included only when $this->multiple is true; preserve the remaining hints and existing single-versus-multiple Accept label behavior.src/Resolver/InputResolver.php (1)
125-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject JSON lists for
--prompts.
is_array($data)accepts decoded JSON lists. For example,["answer"]becomes a PHP array and creates a numeric field ID. Decode without the associative flag and require\stdClassbefore converting its properties to the output map.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Resolver/InputResolver.php` around lines 125 - 138, Update InputResolver::parsePrompts to decode JSON without the associative-array flag, require the decoded value to be an stdClass, and reject all other values including JSON lists with the existing FormException. After validation, convert the object properties into the returned associative map.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/output.mdx`:
- Line 19: Preserve or document all changed public contracts: in
docs/content/output.mdx lines 19-19 and 37-41, update examples to reflect the
supported Output::box($body, $title) contract or provide migration guidance; in
playground/themes/OceanTheme.php lines 243-255 and
src/Primitive/Element/PrimitiveElementsInterface.php, keep renderBanner()
return-type compatibility or document the required theme migration; in
src/Block/Element/FieldElementsInterface.php lines 106-199, restore deprecated
forwarding fieldEntry* methods or document migration; in src/Block/Option.php
lines 55-66 and src/Block/OptionType.php line 15, retain an OptionKind alias or
document the OptionType migration; update stale fieldEntry* references in
docs/content/specification.mdx, docs/content/fields/option-groups.mdx, and
docs/content/fields/anatomy.mdx.
In `@src/Field/FieldFactory.php`:
- Around line 64-72: In FieldFactory::open(), rename the local variable assigned
from $block->options() from $entries to $options, and update every reference in
the method, including optionLabels(), optionDescriptions(), Select, Toggle,
Reorder, and Search construction.
In `@src/Primitive/Output.php`:
- Line 26: Restore the existing positional parameter order in the box() method
so its first argument remains $body and calls such as box('Body text') continue
rendering body content. Update any title handling and related
documentation/examples around box() accordingly; do not reinterpret the first
argument as a title or change existing call behavior.
In `@src/Screen/Scroller.php`:
- Line 23: Preserve named-argument compatibility for the public follow(),
viewport(), and slice() methods by retaining the $height parameter name, or
provide an explicit migration path if the rename to $rows is intentional and
permitted. Ensure existing callers using height: continue to work.
In `@src/Testing/ScreenTester.php`:
- Around line 161-168: Update ScreenTester::theme() to store a named theme
identifier instead of constructing it immediately; preserve direct
ThemeInterface assignment. In controller(), after all tester configuration is
finalized, create the named theme using the final themeOptions(), cols, and
existing ThemeManager::create flow, ensuring fluent options(), border(), and
cols() calls affect the rendered frame.
In `@src/Testing/TuiTester.php`:
- Around line 322-351: Update the ThemeInterface branch in TuiTester so the
ScreenController constructor receives the layout configured by layout() instead
of the hardcoded 'default' value. Reuse the existing configured-layout value
from $this->tui, while leaving the non-ThemeInterface controller path unchanged.
In `@tests/phpunit/Unit/Theme/AbstractThemeTest.php`:
- Around line 36-37: Rename all outdated “entry” terminology to “option” in the
specified PHPUnit test labels, comments, and the affected test method around
AbstractThemeTest. Update AbstractThemeTest.php ranges 36-37, 66-70, 97, and
141-147, plus ElementDelegationTest.php ranges 44-46, 60-66, and 82-85; preserve
the existing test callbacks and assertions while ensuring PHPUnit output and
source wording match the fieldOption* API.
---
Outside diff comments:
In `@src/Field/FilePicker.php`:
- Around line 289-303: Update FilePicker::hints() so the Action::Toggle “select”
hint is included only when $this->multiple is true; preserve the remaining hints
and existing single-versus-multiple Accept label behavior.
In `@src/Resolver/InputResolver.php`:
- Around line 125-138: Update InputResolver::parsePrompts to decode JSON without
the associative-array flag, require the decoded value to be an stdClass, and
reject all other values including JSON lists with the existing FormException.
After validation, convert the object properties into the returned associative
map.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f4aee9fb-f8b5-41d7-a93e-d99e13723f54
📒 Files selected for processing (84)
docs/content/output.mdxplayground/18-output-box.phpplayground/themes/OceanTheme.phpsrc/Block/Actions.phpsrc/Block/Element/FieldElementsInterface.phpsrc/Block/Field.phpsrc/Block/Option.phpsrc/Block/OptionType.phpsrc/Block/Prose.phpsrc/Builder/FieldBuilder.phpsrc/Builder/Form.phpsrc/Builder/PanelBuilder.phpsrc/Field/AbstractField.phpsrc/Field/Calendar.phpsrc/Field/Capability/OptionsCapableTrait.phpsrc/Field/Capability/PagingCapableTrait.phpsrc/Field/Capability/SelectionCapableTrait.phpsrc/Field/FieldFactory.phpsrc/Field/FilePicker.phpsrc/Field/Matcher.phpsrc/Field/Pause.phpsrc/Field/Reorder.phpsrc/Field/Select.phpsrc/Field/Suggest.phpsrc/Field/Template.phpsrc/Input/KeyMap.phpsrc/Primitive/Element/PrimitiveElementsInterface.phpsrc/Primitive/Output.phpsrc/Primitive/Progress.phpsrc/Resolver/InputResolver.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Screen/Collector.phpsrc/Screen/Layout/AbstractLayout.phpsrc/Screen/Layout/GridLayout.phpsrc/Screen/Overlay.phpsrc/Screen/Region.phpsrc/Screen/ScreenController.phpsrc/Screen/ScreenRenderer.phpsrc/Screen/Scroller.phpsrc/Terminal/Markup.phpsrc/Terminal/MarkupSegment.phpsrc/Terminal/MarkupType.phpsrc/Terminal/Terminal.phpsrc/Testing/BufferedTerminal.phpsrc/Testing/ScreenTester.phpsrc/Testing/TuiTester.phpsrc/Theme/AbstractTheme.phpsrc/Theme/DefaultTheme.phpsrc/Theme/DosTheme.phpsrc/Theme/EmberTheme.phpsrc/Theme/FrostTheme.phpsrc/Theme/MidnightTheme.phpsrc/Theme/MonoTheme.phpsrc/Theme/Override/FieldOverrides.phpsrc/Theme/Override/ThemeElement.phpsrc/Translation/Translator.phpsrc/Tui.phptests/phpunit/Fixtures/Theme/CapableTheme.phptests/phpunit/Traits/MixedOptionsTrait.phptests/phpunit/Unit/Block/FieldBlockTest.phptests/phpunit/Unit/Block/FieldDeclarationTest.phptests/phpunit/Unit/Block/OptionTest.phptests/phpunit/Unit/Builder/FormTest.phptests/phpunit/Unit/ControlBytesTest.phptests/phpunit/Unit/Field/CalendarTest.phptests/phpunit/Unit/Field/MatcherTest.phptests/phpunit/Unit/Field/ReorderTest.phptests/phpunit/Unit/Field/SearchTest.phptests/phpunit/Unit/Field/SelectTest.phptests/phpunit/Unit/Field/SuggestTest.phptests/phpunit/Unit/Primitive/OutputTest.phptests/phpunit/Unit/Screen/Layout/GridLayoutTest.phptests/phpunit/Unit/Terminal/MarkupTest.phptests/phpunit/Unit/Terminal/TerminalTest.phptests/phpunit/Unit/Testing/BufferedTerminalTest.phptests/phpunit/Unit/Theme/AbstractThemeTest.phptests/phpunit/Unit/Theme/BuiltinThemesTest.phptests/phpunit/Unit/Theme/ElementDelegationTest.phptests/phpunit/Unit/Theme/OutputRenderTest.phptests/phpunit/Unit/Theme/SupportTest.phptests/phpunit/Unit/Theme/ThemeBuilderTest.phptests/phpunit/Unit/Theme/ThemeRenderTest.phptests/phpunit/Unit/Theme/ThemeTest.php
…d the tester's named-theme build.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Tui.php (1)
558-569: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep
progress()callable-compatible.
progress()andPrimitive\Progress::run()now reject method references, invokable objects, and callable strings that the previouscallableAPI accepted. Preservecallablesupport across both APIs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Tui.php` around lines 558 - 569, Update Tui::progress() and Primitive\Progress::run() to accept the broader callable type instead of requiring \Closure, preserving support for method references, invokable objects, and callable strings while retaining the existing callback invocation and return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/Tui.php`:
- Around line 558-569: Update Tui::progress() and Primitive\Progress::run() to
accept the broader callable type instead of requiring \Closure, preserving
support for method references, invokable objects, and callable strings while
retaining the existing callback invocation and return behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b176cabd-6e28-47b3-9933-76c16ddd661b
📒 Files selected for processing (9)
AGENTS.mdsrc/Answers/Answers.phpsrc/Field/FieldFactory.phpsrc/Field/Suggest.phpsrc/Testing/ScreenTester.phpsrc/Testing/TuiTester.phpsrc/Translation/Translator.phpsrc/Tui.phptests/phpunit/Unit/Theme/AbstractThemeTest.php
|
Summary
A convergence pass that makes the codebase internally consistent with itself. Twelve inconsistency classes landed as behavior-preserving changes, one commit converged source comments onto the project's technical register, and a further seven classes were resolved after the deferred judgment calls were decided. Every commit was gated on the full test suite (2918 tests) plus PHPCS, PHPStan level 9 and Rector, all green throughout.
The package is tagged
0.1.x, so the public-surface changes below are deliberate and were taken while they are still cheap.Behavior-preserving convergence
Naming
Block\Fieldmethods that return why a candidate value is invalid now share the*Violationsuffix already anchored inCollectCapableInterface::requiredViolationandConstrainCapableInterface::boundsViolation.Collector::optionsError(),Template::partError()and the separaterefuses()/refusal()rejection channel are deliberately untouched.is*prefix:Option::selectable(),SelectionCapable*::currentSelectable(),CompletionCapableTrait::completionAvailable()andLayoutManager::nameable(). The existingis*/has*/ third-person-verb system (binds,matches,acceptsValue) is left intact.KeyMapproperty is cased$keyMapand its constructor argument$key_map, per the project rule that properties are camelCase and method arguments snake_case. This includes the string-basedReflectionPropertylookup inTuiTest, which would otherwise have broken silently.Region's head-packed property is renamed$headto pair with the existing$tail;blocks()still returns head and tail merged, so the property and the accessor no longer disagree.$delta, matchingStepCapableInterface::stepBy()andKeyRouter::moveBy().OptionsCapableTrait::stepCursor()keeps$dir, which is a unit scan direction rather than an amount.Field::optionLine()names its narrowed theme argument$elements, matching the seven sibling methods in the same class.DefaultTheme::statusSymbol()becomesstatusGlyph(), the vocabulary used everywhere else in the theme core.Structure
Suggest's 208-characterimplementslist is wrapped one interface per line, mirroringSearch; the single-parameter promoted constructors inKeyRouterandAbstractLayoutcollapse back onto one line.#[\Override]is applied exactly where a concrete inherited method is overridden: removed fromTemplate::liveValue(), which implements an abstract base method, and added toSelectionBoundedTrait::renderConstraint(), which overrides a concreteAbstractFielddefault.API shape
Legend::advertise(),Legend::clear()andBreadcrumb::trail()returnstaticwith@return static, matching the twelve fluent setters already declared that way acrossPanel,Progress,MarkupandActions.AgentHelp's constructor takes(root, context, envPrefix), matchingSchemaGeneratorandSchemaValidator.Comments
Tensions resolved
These were the judgment calls a behavior-preserving pass deliberately would not make on its own. Each was decided explicitly and then applied.
The choice-list row is an
optionThe widest vocabulary split in the codebase: one selectable item was
entryin theFieldAPI and theme elements,optionin the value object and builder, androwin several docblocks. It converges onoption, becauseOption,OptionType,FieldBuilder::option()andOptionsCapable*already carried it.This renames the public
FieldElementsInterfacemethods -fieldEntry(),fieldEntryMarker(),fieldEntrySeparator(),fieldEntrySelector(),fieldEntryNote(),fieldEntryDescription()all becomefieldOption*. A consumer with a custom theme must rename those methods. 43 files changed.Terminal extent is
columnsandrowsThe two dialects were split about evenly, with
ScreenControllermixing both in one file.columns/rowswins because the renderer,Region, the whole layout family and the publicLayoutInterfacealready spoke it.Terminal::width()andTerminal::height()becomeTerminal::columns()andTerminal::rows(), withBufferedTerminalrenamed in lockstep so the double still mirrors its subject.Overlay's$area_widthandScroller's$heightfollow.ScreenRenderer::height()becomesrows()and itspieces()array key changes fromheighttorows.String-measurement helpers (
Ansi::width(),Markup::width()), theme frame widths and content widths inBox/Tableare a different concept and keep their names.FormExceptionfor consumer-declaration mistakesThe original plan reported six raw
\InvalidArgumentExceptionthrows. Auditing the repo found 38, spanning four distinct families, so a blanket conversion would have swept in errors that are not form declarations at all.Sixteen throws in the declaration family now throw
FormException:Actions::select(),Region::fixed()/flex(),AbstractLayoutregion declaration,KeyMapbinding validation,DefaultThemeoption validation,Translatorsource,InputResolverprompts andPagingCapableTraitpage size. BecauseFormException extends \InvalidArgumentException, this is catch-compatible: existingcatch (\InvalidArgumentException)still fires.Twenty-two throws stay raw and internally consistent as their own families: the eight "theme cannot draw X" errors, the nine registry and class-validation errors, and the five stream and programming errors.
Naming ties settled
current*.Template::$active/activeName()become$current/currentName(), andAbstractField::highlightedDescription()becomescurrentDescription().TemplatecomposesTextEditCapableTrait, which already owns$cursorfor the buffer offset, so$currentavoids that collision.-Type.OptionKindbecomesOptionTypeandMarkupKindbecomesMarkupType, both files renamed to match PSR-4.Field::valueKind()becomesvalueType().moveCursor().Reorder::move()andTemplate::move()followFilePicker.OptionsCapableTrait::stepCursor()stays - it is a unit scan with different semantics - andCalendar::move(Key)stays, being key interpretation rather than cursor movement.Public surface aligned
Output::box()takes its title first, matching its siblingOutput::card().Progress::run()narrowscallableto\Closure, the convention everywhere else in the codebase.renderBanner()returnslist<string>likerenderCard(),renderText(),renderTable()andrenderDefinitions(). Rendered output is unchanged - callers join with a newline exactly as the imploded string did.ScreenTester::theme()andTuiTester::theme()both acceptThemeInterface|string, so an instance and a name work on either harness. The three session-abort conventions are out of scope and unchanged.Grid capacity error
PanelBuilder::descend()reporteddeclares %d slot(s) for %d window(s)while feeding it(count(windows()), count(panels)). The counts were in the right order; the nouns were inverted - the grid's declared cells arewindows()in code, and the things needing one are panels. Both copies of the message (inPanelBuilderandGridLayout::assertDeals()) now readdeclares %d window(s) for %d panel(s)., andassertDeals()'s parameter is renamed$panelsto match. This changes observable message text.One scroll subsystem
The renderer re-derived the overflow-indicator rule inline in
moved()andpacked(), whileScrollerplusViewportdid the same work on the controller path - three implementations of one rule.Both renderer paths now resolve their window through
Scroller::viewport()and slice throughScroller::slice(), so the docblock claim thatScrolleris "the one place the scroll-indicator flags are decided" is now true. Verified safe first: the two clamps looked divergent becauseScrollCapableTrait::offset()lacks the outermax(0, ...)thatScroller::clamp()has, butscrollTo()already appliesmax(0, $row), so the difference was unreachable.Documentation claims corrected
Nine comments asserted something the code does not do. The register pass deliberately left them alone, because correcting a claim inside a wording pass hides a behavior-documentation fix in a diff nobody reviews as one. They are corrected in their own commit instead.
src/Block/AbstractBlock.phpdescribed the narrowing failure as a type error.elements()throws\InvalidArgumentException.src/Block/Field.phpstill described the narrowed theme argument as "The theme." after the rename.src/Block/Panel.phpclaimedids()returns every row carrying an id. It collects fromField,MarkupandProgressonly, so nested panel ids are excluded.src/Builder/FieldBuilder.phpcarried a malformed sentence in the class docblock.src/Field/Capability/OptionsCapableTrait.phpclaimed the cursor is only ever placed on a selectable row.firstSelectable()falls back to index 0 when no row is selectable.src/Field/Rating.phpcalled typing inert on any scale running past nine. One digit names one point, so 0 to 9 stay reachable on a 1-15 scale; only the points above nine are not.src/Field/Reorder.phpnamedSpace,UpandDownin two comments. The actions resolve throughScopedKeyMap, so a vim preset or custom map rebinds them.src/Screen/Capability/ScrollCapableInterface.phpdid not declare the\LogicExceptionthatScrollCapableTrait::scrollTo()throws for a surface that does not scroll.src/Terminal/Ansi.phpclaimedlink()drops every control byte.stripControl()matches/[\x00-\x1F\x7F]/, which is the C0 bytes and DEL.Still open
Four comment claims are still flagged and unfixed. Each needs a decision about intended behavior rather than a wording change, so none belongs in this PR.
src/Tui.phpclaims another facade constructed or configured meanwhile may have replaced the shared translator.Translator::setShared()is called only from operations, so only another facade's operation can replace it.src/Answers/Answers.phprefers toitems(). The snapshots live in the public$itemsproperty; per-id access isitem($id).src/Translation/Translator.phpclaims a missing bundled directory would otherwise error on every construction. Catalogs load lazily fromcatalog()on first translate, so the error would surface at first use.src/Field/Suggest.phpdescribes a fixed option set.adoptQueryRows()replaces the values whenever a query source settles.Six convergence classes were considered and deliberately left alone.
Transformnames stay a string const list rather than an enum. They mirror externalStr2Namemethods resolved by string, cross the consumer boundary inderive()declarations and are serialized into the JSON schema, so an enum would have to round-trip back to strings at both boundaries.settlestill carries four distinct operations acrossCollector,Field,KeyRouterandScreenController. RenamingScreenController::settled()tosettle()was evaluated and rejected, becauseresettle()already exists in the same class and calls it.SelectionBoundedTraitstill breaks the<Verb>CapableInterface+<Verb>CapableTraitpattern while living inCapability/. All three candidate fixes coin a name or a contract the codebase never used.AgentHelpandSchemaGeneratorstill emit two key vocabularies (x-asked-whenagainstasked_when) from one per-field pipeline. Both are machine-readable output consumed outside the library.Screen\SourceandAnswers\Provenanceremain two enums for where a value came from, bridged by amatch(TRUE)inCollector::provenance().FilePickerandSuggeststill hand-roll logic their sibling capability traits provide, with deliberate behavioral deltas (append-only editing, string rows, a -1 idle cursor).The
<noun>Text()getter convergence was approved but found inapplicable: both candidates,Panel::preparation()andProgress::workload(), return?\Closurerather than a string, so theTextsuffix cannot apply.Coverage note
The comment-register pass completed fully on 2 of 16 source slices and partially on the rest before the run ended; 55 of 217 source files carry comment changes from it. The largest files -
ScreenController,ScreenRenderer,DefaultThemeandAbstractTheme- are largely untouched by that pass. It is idempotent, so re-running the convergence finishes the remainder without redoing what has landed.Before / After
Summary by CodeRabbit
New Features
API Updates
Documentation