refactor(list): derive the Status column width from PositionMask (#3845)

Nightly sweep finding in `src/commands/list/`. `build_estimated_widths`
allocated the Status column with a literal `8` plus a comment restating
`PositionMask::FULL`'s arithmetic (`1+1+1+1+1+1+2`), while
`render_with_mask` pads every one of the seven positions to that mask's
widths. The two numbers had to agree and nothing made them — widening a
position (say `USER_MARKER` to 3 for a wider marker) would have left the
column narrower than the cell it renders. This reads the width off the
mask via a new `PositionMask::total_width()`, and pins the invariant
with a test that measures a rendered cell's visible width against it.

The same pass corrects the docs that described a mask nobody builds.
`PositionMask` claimed to track "which status symbol positions are
actually used across all items and the maximum width needed for each
position", with "a width of 0 means the position is unused";
`layout.rs`'s module doc stated it in most detail (`status_width =
max(rendered_width_across_all_items)`, plus a "position mask removes
columns for symbols that appear in zero rows" bullet);
`render_with_mask`'s own doc repeated it. Nothing measures anything:
`::FULL` is the only mask constructed, because the column's geometry is
chosen at skeleton time before any task result exists and progressive
and final renders have to agree on it. All three now say that. The
derived `Default` goes with them — an all-zero mask would silently
collapse the cell, and no caller wants one.

No behavior change — `total_width()` is 8, exactly what the literal was,
so no snapshot moves.

<details><summary>Verification</summary>

```
cargo test --bins -- list::layout list::render list::model   # 116 passed
cargo test --test integration -- integration_tests::list     # 128 passed
cargo clippy --bins --lib                                    # clean
cargo fmt --all -- --check                                   # clean
```

The new test isn't a regression test in the strict sense — there's no
wrong behavior today, since both numbers are 8. It's the guard that
makes them one number: it fails if `FULL`'s widths and what
`render_with_mask` draws ever diverge from `total_width()`. The drift it
prevents is latent, not live.

</details>

---------

Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
This commit is contained in:
Worktrunk Bot
2026-08-18 13:24:05 -07:00
committed by GitHub
parent a6f26e5c6a
commit e07deadf69
2 changed files with 80 additions and 32 deletions
+25 -20
View File
@@ -7,29 +7,33 @@
//!
//! ## Unified Position Grid
//!
//! All status indicators use position-based alignment with selective rendering.
//! All status indicators use position-based alignment.
//! See [`super::model::StatusSymbols`] for the complete symbol list and categories.
//!
//! Only positions used by at least one row are included (position mask):
//! - Within those positions, symbols align vertically for scannability
//! - Empty positions render as single space for grid alignment
//! Every row allocates every position ([`super::model::PositionMask::FULL`] is
//! the only mask anything constructs):
//! - Symbols align vertically at their position for scannability
//! - Empty positions render as whitespace padded to the position's width
//! - No leading spaces before the first symbol
//!
//! Example with working_tree, main_state, and user_marker used:
//! Example with working_tree, main_state, and user_marker carrying data. Every
//! row is the same eight columns wide, one per allocated position:
//! ```text
//! Row 1: " _🤖" (working=space, main=_, user=🤖)
//! Row 2: "?! _ " (working=?!, main=_, user=space)
//! Row 3: " 💬" (working=space, main=space, user=💬)
//! Row 1: " _ 🤖" (working=clean, main=_, user=🤖)
//! Row 2: " !? _ " (working=!?, main=_, user=none)
//! Row 3: " 💬" (working=clean, main=none, user=💬)
//! ```
//!
//! ## Width Calculation
//!
//! ```text
//! status_width = max(rendered_width_across_all_items)
//! status_width = max(header_width, PositionMask::FULL.total_width())
//! ```
//!
//! The width is calculated by rendering each item's status with the position
//! mask and taking the maximum width.
//! The width is not measured across items: the column's geometry is chosen at
//! skeleton time, before any task result exists, and progressive and final
//! renders have to agree on it. So the layout budgets the mask's total width
//! and every render draws exactly that.
//!
//! ## Why This Design?
//!
@@ -37,13 +41,9 @@
//! - One alignment mechanism for all status indicators
//! - User marker treated consistently with git symbols
//!
//! **Eliminates wasted space:**
//! - Position mask removes columns for symbols that appear in zero rows
//! - User marker only takes space when present
//!
//! **Maintains alignment:**
//! - All symbols align vertically at their positions (vertical scannability)
//! - Grid adapts to minimize width based on active positions
//! - The grid never shifts as results arrive
//!
//! # Priority System Design
//!
@@ -754,9 +754,14 @@ fn build_estimated_widths(
// Fixed widths for slow columns (require expensive git operations)
// Values exceeding these widths use compact notation (K suffix)
//
// Status column: Must match PositionMask::FULL width for consistent alignment
// PositionMask::FULL allocates: 1+1+1+1+1+1+2 = 8 chars (7 positions)
let status_fixed = fit_header(ColumnKind::Status.header(), 8);
// Status column: every row renders all seven positions of
// `PositionMask::FULL`, so the column has to be at least that wide or the
// cell overflows it. Read the width off the mask rather than restating its
// arithmetic — the two can't drift.
let status_fixed = fit_header(
ColumnKind::Status.header(),
super::model::PositionMask::FULL.total_width(),
);
let working_diff_fixed = fit_header(ColumnKind::WorkingDiff.header(), 9); // "+999 -999"
let ahead_behind_fixed = fit_header(ColumnKind::AheadBehind.header(), 7); // "↑99 ↓99"
let branch_diff_fixed = fit_header(ColumnKind::BranchDiff.header(), 9); // "+999 -999"
@@ -1155,7 +1160,7 @@ fn allocate_columns_with_priority(
/// - Paths (relative to main worktree)
///
/// Pre-allocated estimates (generous to minimize truncation):
/// - Status: 8 chars (PositionMask::FULL, 7 positions)
/// - Status: `PositionMask::FULL.total_width()` (7 positions, 8 chars today)
/// - Working diff: 9 chars ("+999 -999")
/// - Ahead/behind: 7 chars ("↑99 ↓99")
/// - Branch diff: 9 chars ("+999 -999")
+55 -12
View File
@@ -245,19 +245,22 @@
use super::state::{Divergence, MainState, OperationState, WorktreeState};
/// Tracks which status symbol positions are actually used across all items
/// and the maximum width needed for each position.
/// Per-position character widths for the Status column, used to pad each
/// position so symbols line up vertically across rows.
///
/// This allows the Status column to:
/// 1. Only allocate space for positions that have data
/// 2. Pad each position to a consistent width for vertical alignment
/// The widths are fixed, not measured: [`FULL`](Self::FULL) is the only mask
/// anything constructs. The Status column's geometry is chosen at skeleton
/// time, before any task result exists, and progressive and final renders have
/// to agree on it — a mask narrowed to the positions that happen to carry data
/// would shift columns as results arrived. So every render allocates every
/// position, and [`total_width`](Self::total_width) is what the layout budgets
/// for the cell.
///
/// Stores maximum character width for each of 7 positions (including user marker).
/// A width of 0 means the position is unused.
#[derive(Debug, Clone, Copy, Default)]
/// No `Default`: an all-zero mask would silently collapse the cell to nothing,
/// and there is no caller that wants one.
#[derive(Debug, Clone, Copy)]
pub struct PositionMask {
/// Maximum width for each position: [0, 1, 2, 3, 4, 5, 6]
/// 0 = position unused, >0 = max characters needed
/// Character width allocated to each position: [0, 1, 2, 3, 4, 5, 6]
widths: [usize; 7],
}
@@ -290,6 +293,16 @@ impl PositionMask {
pub(crate) fn width(&self, pos: usize) -> usize {
self.widths[pos]
}
/// Total characters this mask renders — the sum of every position's
/// allocated width.
///
/// The layout budgets the Status column from `FULL.total_width()` rather
/// than a literal, so widening a position here can't leave the column one
/// character short of what [`StatusSymbols::render_with_mask`] emits.
pub(crate) fn total_width(&self) -> usize {
self.widths.iter().sum()
}
}
/// Working tree changes as structured booleans
@@ -437,8 +450,9 @@ impl StatusSymbols {
/// - `Visible(s)` → styled content padded to the slot width.
///
/// CRITICAL: Always use [`PositionMask::FULL`] for consistent spacing
/// between progressive and final rendering. The mask provides the
/// maximum width needed for each position across all rows.
/// between progressive and final rendering. The mask gives each position
/// its allocated width, and the cell the layout budgeted is
/// [`PositionMask::total_width`].
pub fn render_with_mask(&self, mask: &PositionMask, placeholder: &str) -> String {
use anstyle::Style;
use worktrunk::styling::StyledLine;
@@ -786,9 +800,38 @@ mod tests {
assert_snapshot!(rendered, @"· ·↑··");
}
/// The layout budgets the Status column from `FULL.total_width()`, so that
/// number has to be what a rendered cell actually draws — otherwise a
/// widened position overflows the column it was allocated. Both the
/// all-loading cell and a fully-populated one are pinned, since every
/// position pads to its allocated width in either state.
#[test]
fn test_full_mask_total_width_matches_rendered_cell() {
use ansi_str::AnsiStr;
use unicode_width::UnicodeWidthStr;
let visible_width = |rendered: &str| UnicodeWidthStr::width(rendered.ansi_strip().as_ref());
let loading = StatusSymbols::default().render_with_mask(&PositionMask::FULL, "·");
assert_eq!(visible_width(&loading), PositionMask::FULL.total_width());
let populated = StatusSymbols {
working_tree: Some(WorkingTreeStatus::new(true, true, true, false, false)),
operation_state: Some(OperationState::InProgress(InProgressOperation::Rebase)),
worktree_state: Some(WorktreeState::None),
main_state: Some(MainState::Ahead),
upstream_divergence: Some(Divergence::Ahead),
// Two columns wide, matching the USER_MARKER allocation.
user_marker: Some(Some("🔥".to_string())),
}
.render_with_mask(&PositionMask::FULL, "·");
assert_eq!(visible_width(&populated), PositionMask::FULL.total_width());
}
#[test]
fn test_position_mask_width() {
let mask = PositionMask::FULL;
assert_eq!(mask.total_width(), 8);
// Check expected widths for each position
assert_eq!(mask.width(PositionMask::STAGED), 1);
assert_eq!(mask.width(PositionMask::MODIFIED), 1);