Conversation
|
Thanks for pointing this out. I agree this overlaps with #13892 / #13490. I have moved this PR back to draft for now. The intended scope here is a smaller TUI-only subset: make the existing status line segment order configurable and split the context meter into If maintainers prefer #13892 as the canonical implementation, I am happy to close this PR, or alternatively rework this into a smaller follow-up/patch on top of #13892 if the separate context meter segments are still useful. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused configurable-segments implementation. The underlying feature request remains open on current main: ui-tui/src/app/useConfigSync.ts:219-233 only hydrates the status-bar position, not a field list.
Problems
ui-tui/src/components/appChrome.tsx:396appendsSpawnHudeven when it renders nothing. Withsubagentsbefore a visible configured segment,appendSegment()sees a non-empty array and emits a leading│separator.ui-tui/src/theme.ts:561mapsstatus_bar_bg, but the PR's status container atui-tui/src/components/appChrome.tsx:410-421does not render witht.color.statusBg; the advertised background customization has no visible effect.- Current main has a newer width-budgeted status-bar contract (
ui-tui/src/components/appChrome.tsx:455-533, commit2f171743b7ba3f898ab58589dc73a53da06bc19a). A salvage should preserve that behavior rather than restore the older single truncating row.
Suggested changes
- Integrate segment selection into the current pinned-essential/tail-budget renderer.
- Make separator accounting depend on visible segments, with a no-active-subagents test.
- Apply and render-test
statusBg.
Automated hermes-sweeper review.
| } | ||
| break | ||
| case 'subagents': | ||
| statusPieces.push(<SpawnHud key={`${segment}-${statusPieces.length}`} t={t} />) |
There was a problem hiding this comment.
SpawnHud may render null, but this still makes statusPieces.length > 0. If a configured list starts with subagents and no subagent is active, the next visible segment gets a leading │. Only account for this segment after confirming it has visible output, and add that ordering case to the renderer tests.
| statusWarn: c('ui_warn') ?? d.color.statusWarn, | ||
| statusBad: d.color.statusBad, | ||
| statusCritical: d.color.statusCritical, | ||
| statusBg: c('status_bar_bg') ?? d.color.statusBg, |
There was a problem hiding this comment.
This maps the skin key into the theme, but StatusRule does not consume t.color.statusBg as a background (appChrome.tsx:410-421 in this PR). Please apply it to the rendered status-bar container and cover the rendered result, otherwise status_bar_bg remains visually ineffective.
|
Thanks for this — configurable TUI status-bar segments have now landed on main via PR #98282 (building on the classic-CLI field toggles from PR #98250). The Ink TUI status rule filters its segments through Closing as implemented on main. Your PR had the right idea well ahead of the implementation — appreciated. |
Summary
display.tui_statusbar_segmentsfor TUI status line customizationcontext_tokens,context_bar, andcontext_percentso the meter can be hidden independentlyTest Plan
npm test -- --run src/__tests__/useConfigSync.test.ts src/__tests__/appChrome.test.tsnpm run type-checknpm run buildenv -u SSH_CONNECTION -u SSH_CLIENT -u SSH_TTY npm test -- --run