feat: add configurable tab bar status - #2586
Conversation
Reserve the right edge of the tab row for a ZOOM pill while the focused pane is zoomed, matching the accent style of the mode bars. The per-tab Z suffix stays; the pill makes the zoomed state visible at a glance like tmux's status-right flag.
Add ui.tab_bar_hostname to display the machine's hostname at the right edge of the tab row, like tmux's #h in status-right. The value resolves where the server renders, so remote sessions show the remote host. Off by default.
kangal-bot
added
the
ai-review
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesTab-bar status system
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Config as UiConfig
participant App
participant Runtime as scheduled task loop
participant Command as tab_bar_status command task
participant Event as AppEvent
participant UI as tab-bar renderer
Config->>App: configure tab-bar status entries
App->>Runtime: schedule refresh work
Runtime->>Command: execute due command
Command->>Event: send TabBarCommandFinished
Event->>App: apply current generation result
App->>UI: update TabBarStatusSegment state
UI->>UI: reserve and render status area
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
scripts/test_config_reference_check.py (1)
35-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the production
Vec<TabBarRightEntryConfig>shape.The sample uses a direct
StatusConfigfield. Production usesui.tab_bar_right: Vec<TabBarRightEntryConfig>. This test does not verify that the reference parser preserves enum values for the configured array field. Change the sample and assertion to usetab_bar_right: Vec<TabBarRightEntryConfig>.Also applies to: 68-80, 147-149
Cargo.toml (1)
41-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
timedependency rationale.Add a short Cargo manifest comment stating that
timeparses and validatesui.tab_bar_rightdatetime formats. No directchronoorjiffdependency provides this capability. Runjust checkbefore commit.Source: Coding guidelines
src/app/tab_bar_status.rs (2)
133-153: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the command environment only when a command is due.
handle_scheduled_tasksruns on every event-loop iteration. This code callsself.custom_command_env()on every pass whenever at least one command entry is configured, even when no runtime is due.custom_command_envcallsstd::env::current_exe()andpane_cwd.is_dir(), so each pass performs filesystem syscalls. Under PTY activity the loop iterates frequently.Resolve the environment lazily on the first due runtime.
♻️ Proposed lazy environment resolution
let generation = self.tab_bar_status_generation; - let (environment, cwd) = self.custom_command_env(); - for runtime in &mut self.tab_bar_commands { + let due = self + .tab_bar_commands + .iter() + .any(|runtime| runtime.task.is_none() && now >= runtime.next_run_at); + if !due { + return changed; + } + let (environment, cwd) = self.custom_command_env(); + for runtime in &mut self.tab_bar_commands { if runtime.task.is_some() || now < runtime.next_run_at { continue; }
468-486: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for the datetime refresh path.
The tests cover command spawning, staleness, abort-on-reload, and deadline behavior. No test exercises the datetime branch of
handle_tab_bar_status_tasks.That branch owns two behaviors worth pinning: it writes the formatted value into the segment identified by
segment_index, and it reportschangedonly when the value differs. A test can configure aDatetimeentry, callhandle_tab_bar_status_taskswith anowpast the deadline, and assert that the segment holds a non-empty value and that a second immediate call reports no change.src/app/input/navigate.rs (1)
861-861: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider relocating
custom_command_envout of the input module.The visibility widening is correct and minimal for the new caller in
src/app/tab_bar_status.rs. The function builds a command environment; it contains no input handling. It now serves two unrelated consumers: key-bound custom commands and tab-bar status commands.Moving it to a shared command-runtime module would keep the input/runtime separation in
app/intact. This is optional and can follow later.As per coding guidelines: "Avoid god objects and preserve the existing separation of application state, actions, and input within
app/."Source: Coding guidelines
src/ui/tabs.rs (1)
466-494: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deriving the rendered layout and the reserved width from one helper.
tab_bar_status_widthcomputes the total reserved width. This loop independently recomputes the same per-segment and per-separator widths to place eachRect. The two must agree exactly, otherwise the status text overflows the tab-bar row or leaves a gap.The invariant currently holds. It is not enforced by the code. A single helper that returns the placed
(Rect, text, accent)tuples, with the reserved width derived from the last rect, would make the agreement structural instead of conventional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 389db0b8-5daf-4dd8-8e2b-9cba923f331c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
Cargo.tomldocs/next/CHANGELOG.mddocs/next/website/src/content/docs/configuration.mdxdocs/next/website/src/data/config-reference.jsonscripts/config_reference_check.pyscripts/test_config_reference_check.pysrc/app/actions.rssrc/app/api.rssrc/app/input/navigate.rssrc/app/mod.rssrc/app/runtime.rssrc/app/state.rssrc/app/tab_bar_status.rssrc/config.rssrc/config/model.rssrc/config/tab_bar.rssrc/events.rssrc/main.rssrc/platform/fallback.rssrc/platform/linux.rssrc/platform/macos.rssrc/platform/unix_common.rssrc/platform/windows.rssrc/server/headless.rssrc/ui.rssrc/ui/tabs.rs
Greptile SummaryThe PR adds configurable server-resolved tab-bar status entries and asynchronous command refreshes, including process-tree cleanup on cancellation.
Confidence Score: 4/5The PR should not merge until Windows status commands can execute when Herdr is hosted inside a job that rejects assignment to the new lifecycle job. Windows commands are created suspended and only resumed after unconditional assignment to a new job; an assignment failure aborts the operation, leaving the configured status entry permanently unable to execute. Files Needing Attention: src/platform/windows.rs
|
| Filename | Overview |
|---|---|
| src/app/tab_bar_status.rs | Implements status-entry scheduling, output sanitization, stale-result rejection, timeout handling, and cancellation. |
| src/platform/windows.rs | Adds suspended launch and kill-on-close job management, but status commands fail when the host job rejects assignment to the new job. |
| src/platform/unix_common.rs | Adds server hostname/time resolution and process-group cleanup for complete command trees. |
| src/platform/fallback.rs | Explicitly disables status commands on unsupported targets while preserving the platform API surface. |
| src/config/tab_bar.rs | Defines and validates the ordered tab-bar status configuration. |
| src/ui/tabs.rs | Renders the right-aligned status area while yielding it when tabs require the available width. |
Sequence Diagram
sequenceDiagram
participant App
participant Scheduler as Status scheduler
participant OS as Platform process setup
participant Command
App->>Scheduler: Configure status entries
Scheduler->>OS: Spawn command suspended/grouped
OS->>OS: Install process-tree lifecycle guard
OS->>Command: Start or resume command
Command-->>Scheduler: Last output line
Scheduler-->>App: TabBarCommandFinished
App->>Scheduler: Reload or timeout
Scheduler->>OS: Drop lifecycle guard
OS->>Command: Terminate process tree
Reviews (5): Last reviewed commit: "fix: keep tab bar status opt-in by defau..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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)
scripts/test_config_reference_check.py (1)
68-79: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSynchronize the sample enum with
TabBarRightEntryConfig.The production enum includes
Zoom,Text,Datetime.formatdefaults, andCommand.timeout_seconds. Add these variants and fields toSAMPLE_MODEL, and assert the complete ordered value list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35df4a77-5e75-4b68-85d6-a5fe1252e243
📒 Files selected for processing (3)
scripts/test_config_reference_check.pysrc/app/tab_bar_status.rssrc/platform/fallback.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/tab_bar_status.rs
- src/platform/fallback.rs
|
@coderabbitai The reduced |
|
Tip For best results, initiate chat on the files or code changes.
ʕᵔᴥᵔʔ ✏️ Learnings added
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20fd5166-9bde-4b9c-a5c0-fdf29ff7c400
📒 Files selected for processing (6)
src/app/tab_bar_status.rssrc/platform/fallback.rssrc/platform/linux.rssrc/platform/macos.rssrc/platform/unix_common.rssrc/platform/windows.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/platform/macos.rs
- src/app/tab_bar_status.rs
| if unsafe { AssignProcessToJobObject(job, process.cast()) } == 0 { | ||
| let error = std::io::Error::last_os_error(); | ||
| unsafe { | ||
| CloseHandle(job); | ||
| } | ||
| return Err(error); | ||
| } |
There was a problem hiding this comment.
If Herdr runs inside a Windows host job that rejects nested job assignment, AssignProcessToJobObject fails and StatusCommandGuard::new aborts before resuming the suspended child, causing every configured status command refresh to fail without executing.
Knowledge Base Used: Platform abstraction and Windows-specific behavior
There was a problem hiding this comment.
This is intentional fail-closed behavior, not a process leak. On supported Windows versions, nested jobs normally allow this assignment; the Windows CI command test also passes in its hosted environment. A host job with incompatible UI restrictions can still reject nesting, and in that case resuming the suspended child would reintroduce the descendant-cleanup bug this guard prevents. The opt-in status entry therefore fails like any other command spawn failure while Herdr remains functional. I do not think weakening lifecycle safety for that restrictive-host edge case is correct.
There was a problem hiding this comment.
That's a sound justification. The fail-closed path is the correct tradeoff here — silently resuming a suspended child with no lifecycle guard would reintroduce the very leak the job object exists to prevent, and losing one opt-in status entry is a far better outcome than unbounded child processes on a restrictive host.
I withdraw the concern.
kangal-bot
removed
the
ai-review
3 participants
Summary
ui.tab_bar_rightstatus area with zoom, hostname, datetime, literal text, and asynchronously refreshed command outputAttribution
This generalizes the focused work from @dhh in #2560 and #2562. All four original commits are preserved in this branch with David's authorship.
Supersedes #2560 and #2562.
Testing
just check