From 6399dac51b5aef3654427d1c1baf6fbc4a613b30 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:17:32 -0700 Subject: [PATCH] Revert "feat(list): add `[list] sort` for configurable row order (#3933)" This reverts commit e9c839a194538dacffe62e8a847af2e826eef496. --- dev/config.example.toml | 35 -- docs/src/content/docs/config.md | 35 -- .../skills/worktrunk/reference/config.md | 35 -- skills/worktrunk/reference/config.md | 35 -- src/cli/mod.rs | 35 -- src/commands/list/collect/mod.rs | 129 ++---- src/commands/list/mod.rs | 1 - src/commands/list/sort.rs | 366 ------------------ src/commands/picker/mod.rs | 14 +- src/config/user/sections.rs | 23 -- src/config/user/tests.rs | 11 +- tests/integration_tests/list_config.rs | 223 ----------- .../switch_picker_dry_run.rs | 48 --- ...ation_tests__help__help_config_create.snap | 35 -- ...gration_tests__help__help_config_long.snap | 35 -- 15 files changed, 47 insertions(+), 1013 deletions(-) delete mode 100644 src/commands/list/sort.rs diff --git a/dev/config.example.toml b/dev/config.example.toml index a241b8923..95c306457 100644 --- a/dev/config.example.toml +++ b/dev/config.example.toml @@ -103,7 +103,6 @@ # json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning # # columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -# sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order) # # timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables # @@ -143,40 +142,6 @@ # needs an LLM command (`[commit.generation]`), `url` needs a `[list] url` # template — since listing can't supply the data. # -# #### Row order -# -# `sort` orders the rows of the same two surfaces, `columns` orders their -# columns. Keys are column names, most significant first, each optionally -# prefixed with `-` for descending: `["path"]` makes the table mirror the -# directory layout, `["-age"]` puts the oldest commit on top, and -# `["branch", "-age"]` groups by branch name with the newest commit first inside -# each name. -# -# Sortable columns are the ones whose value is known before the table paints: -# `branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -# `upstream`, the diff columns, custom columns) stream in behind the first frame, -# so ordering on them would mean either holding the table for a network round -# trip or reordering rows under the cursor. Naming one is an error rather than a -# silent reinterpretation, as is an unknown key or the same key twice. -# -# `age` counts up from the commit date, so ascending `age` is newest first — the -# same direction as the default — and `-age` puts the oldest commits on top. -# -# Omitting `sort` keeps the default order: the current worktree first, the -# primary worktree second, then newest commit first. Any `sort` value replaces -# that order outright, including the two pinned rows, since pinning them would -# defeat the order being asked for. Newest-first survives as the final tiebreak, -# so rows a spec can't separate keep the order they already had — branch-only -# rows under `sort = ["path"]`, for instance, since only worktrees have a path. -# -# Rows sort within their group and never across it: worktrees first, then -# branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -# group ordered by the spec. -# -# Unlike `columns`, `sort` does reach `--format json`: reordering an array -# neither narrows the payload nor adds a fetch, so the every-field contract -# holds either way. -# # #### Custom columns [experimental] # # Custom columns add per-branch context to the `wt list` table. Each diff --git a/docs/src/content/docs/config.md b/docs/src/content/docs/config.md index 1b90bb065..e9961a34b 100644 --- a/docs/src/content/docs/config.md +++ b/docs/src/content/docs/config.md @@ -195,7 +195,6 @@ remotes = false # Include remote-only branches (--remotes) json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order) timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables ``` @@ -236,40 +235,6 @@ named one. A column whose data source is missing still stays hidden — `summary needs an LLM command (`[commit.generation]`), `url` needs a `[list] url` template — since listing can't supply the data. -#### Row order - -`sort` orders the rows of the same two surfaces, `columns` orders their -columns. Keys are column names, most significant first, each optionally -prefixed with `-` for descending: `["path"]` makes the table mirror the -directory layout, `["-age"]` puts the oldest commit on top, and -`["branch", "-age"]` groups by branch name with the newest commit first inside -each name. - -Sortable columns are the ones whose value is known before the table paints: -`branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -`upstream`, the diff columns, custom columns) stream in behind the first frame, -so ordering on them would mean either holding the table for a network round -trip or reordering rows under the cursor. Naming one is an error rather than a -silent reinterpretation, as is an unknown key or the same key twice. - -`age` counts up from the commit date, so ascending `age` is newest first — the -same direction as the default — and `-age` puts the oldest commits on top. - -Omitting `sort` keeps the default order: the current worktree first, the -primary worktree second, then newest commit first. Any `sort` value replaces -that order outright, including the two pinned rows, since pinning them would -defeat the order being asked for. Newest-first survives as the final tiebreak, -so rows a spec can't separate keep the order they already had — branch-only -rows under `sort = ["path"]`, for instance, since only worktrees have a path. - -Rows sort within their group and never across it: worktrees first, then -branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -group ordered by the spec. - -Unlike `columns`, `sort` does reach `--format json`: reordering an array -neither narrows the payload nor adds a fetch, so the every-field contract -holds either way. - #### Custom columns diff --git a/plugins/worktrunk/skills/worktrunk/reference/config.md b/plugins/worktrunk/skills/worktrunk/reference/config.md index 5c4ce49d4..fb3cd8f62 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/config.md +++ b/plugins/worktrunk/skills/worktrunk/reference/config.md @@ -189,7 +189,6 @@ remotes = false # Include remote-only branches (--remotes) json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order) timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables ``` @@ -230,40 +229,6 @@ named one. A column whose data source is missing still stays hidden — `summary needs an LLM command (`[commit.generation]`), `url` needs a `[list] url` template — since listing can't supply the data. -#### Row order - -`sort` orders the rows of the same two surfaces, `columns` orders their -columns. Keys are column names, most significant first, each optionally -prefixed with `-` for descending: `["path"]` makes the table mirror the -directory layout, `["-age"]` puts the oldest commit on top, and -`["branch", "-age"]` groups by branch name with the newest commit first inside -each name. - -Sortable columns are the ones whose value is known before the table paints: -`branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -`upstream`, the diff columns, custom columns) stream in behind the first frame, -so ordering on them would mean either holding the table for a network round -trip or reordering rows under the cursor. Naming one is an error rather than a -silent reinterpretation, as is an unknown key or the same key twice. - -`age` counts up from the commit date, so ascending `age` is newest first — the -same direction as the default — and `-age` puts the oldest commits on top. - -Omitting `sort` keeps the default order: the current worktree first, the -primary worktree second, then newest commit first. Any `sort` value replaces -that order outright, including the two pinned rows, since pinning them would -defeat the order being asked for. Newest-first survives as the final tiebreak, -so rows a spec can't separate keep the order they already had — branch-only -rows under `sort = ["path"]`, for instance, since only worktrees have a path. - -Rows sort within their group and never across it: worktrees first, then -branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -group ordered by the spec. - -Unlike `columns`, `sort` does reach `--format json`: reordering an array -neither narrows the payload nor adds a fetch, so the every-field contract -holds either way. - #### Custom columns [experimental] Custom columns add per-branch context to the `wt list` table. Each diff --git a/skills/worktrunk/reference/config.md b/skills/worktrunk/reference/config.md index 5c4ce49d4..fb3cd8f62 100644 --- a/skills/worktrunk/reference/config.md +++ b/skills/worktrunk/reference/config.md @@ -189,7 +189,6 @@ remotes = false # Include remote-only branches (--remotes) json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order) timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables ``` @@ -230,40 +229,6 @@ named one. A column whose data source is missing still stays hidden — `summary needs an LLM command (`[commit.generation]`), `url` needs a `[list] url` template — since listing can't supply the data. -#### Row order - -`sort` orders the rows of the same two surfaces, `columns` orders their -columns. Keys are column names, most significant first, each optionally -prefixed with `-` for descending: `["path"]` makes the table mirror the -directory layout, `["-age"]` puts the oldest commit on top, and -`["branch", "-age"]` groups by branch name with the newest commit first inside -each name. - -Sortable columns are the ones whose value is known before the table paints: -`branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -`upstream`, the diff columns, custom columns) stream in behind the first frame, -so ordering on them would mean either holding the table for a network round -trip or reordering rows under the cursor. Naming one is an error rather than a -silent reinterpretation, as is an unknown key or the same key twice. - -`age` counts up from the commit date, so ascending `age` is newest first — the -same direction as the default — and `-age` puts the oldest commits on top. - -Omitting `sort` keeps the default order: the current worktree first, the -primary worktree second, then newest commit first. Any `sort` value replaces -that order outright, including the two pinned rows, since pinning them would -defeat the order being asked for. Newest-first survives as the final tiebreak, -so rows a spec can't separate keep the order they already had — branch-only -rows under `sort = ["path"]`, for instance, since only worktrees have a path. - -Rows sort within their group and never across it: worktrees first, then -branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -group ordered by the spec. - -Unlike `columns`, `sort` does reach `--format json`: reordering an array -neither narrows the payload nor adds a fetch, so the every-field contract -holds either way. - #### Custom columns [experimental] Custom columns add per-branch context to the `wt list` table. Each diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 067fccb0d..511ce034b 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2086,7 +2086,6 @@ remotes = false # Include remote-only branches (--remotes) json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order) timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables ``` @@ -2127,40 +2126,6 @@ named one. A column whose data source is missing still stays hidden — `summary needs an LLM command (`[commit.generation]`), `url` needs a `[list] url` template — since listing can't supply the data. -#### Row order - -`sort` orders the rows of the same two surfaces, `columns` orders their -columns. Keys are column names, most significant first, each optionally -prefixed with `-` for descending: `["path"]` makes the table mirror the -directory layout, `["-age"]` puts the oldest commit on top, and -`["branch", "-age"]` groups by branch name with the newest commit first inside -each name. - -Sortable columns are the ones whose value is known before the table paints: -`branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -`upstream`, the diff columns, custom columns) stream in behind the first frame, -so ordering on them would mean either holding the table for a network round -trip or reordering rows under the cursor. Naming one is an error rather than a -silent reinterpretation, as is an unknown key or the same key twice. - -`age` counts up from the commit date, so ascending `age` is newest first — the -same direction as the default — and `-age` puts the oldest commits on top. - -Omitting `sort` keeps the default order: the current worktree first, the -primary worktree second, then newest commit first. Any `sort` value replaces -that order outright, including the two pinned rows, since pinning them would -defeat the order being asked for. Newest-first survives as the final tiebreak, -so rows a spec can't separate keep the order they already had — branch-only -rows under `sort = ["path"]`, for instance, since only worktrees have a path. - -Rows sort within their group and never across it: worktrees first, then -branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -group ordered by the spec. - -Unlike `columns`, `sort` does reach `--format json`: reordering an array -neither narrows the payload nor adds a fetch, so the every-field contract -holds either way. - #### Custom columns [experimental] Custom columns add per-branch context to the `wt list` table. Each diff --git a/src/commands/list/collect/mod.rs b/src/commands/list/collect/mod.rs index 36bf4efa4..626f75183 100644 --- a/src/commands/list/collect/mod.rs +++ b/src/commands/list/collect/mod.rs @@ -73,10 +73,7 @@ //! //! What each field feeds: //! - `%ct` — sort order on the skeleton. This is why #6 is pre-skeleton; the -//! skeleton can't pick row order without it. A `[list] sort` spec that never -//! consults the date (`["path"]`, `["branch"]`) doesn't need it for order, -//! but the fork still runs — `%h` is a skeleton cell and Age/Message are -//! painted right behind it, so skipping it would only move the cost. +//! skeleton can't pick row order without it. //! - `%ct` again, post-skeleton — Age column ("3 hours ago"). //! - `%s` post-skeleton — Message column. //! - `%h` pre-skeleton — the abbreviated SHA every surface renders: the Commit @@ -1086,36 +1083,24 @@ pub fn collect( std::collections::HashMap::new() }); - // `[list] sort` picks the row order. It resolves here, ahead of the - // `[list] columns` selection below, because order is chosen before the - // skeleton paints — which is also why only skeleton-time columns are - // sortable (see [`super::sort`]). A bad key aborts `wt list` but only - // degrades the picker, which can't surface an abort mid-render: the same - // fork the column selection takes. - let sort_terms = match super::sort::parse_sort_spec(&repo.config().list.sort) { - Ok(terms) => terms, - Err(e) if progressive_handler.is_some() => { - emit_warning(warning_message(format!("Sort order ignored: {e}")).to_string()); - Vec::new() - } - Err(e) => return Err(e), - }; - // Sort worktrees: current first, main second, then by timestamp descending - // — unless `[list] sort` replaces that with its own order. let sorted_worktrees = sort_worktrees_with_cache( worktree_subjects, &main_worktree_id, current_worktree_id.as_ref(), &commit_details_map, - &sort_terms, ); - // Branch-only and remote rows sort as their own groups, by timestamp - // (most recent first) or by the configured spec. - let branches_without_worktrees = - sort_branch_rows(branches_without_worktrees, &commit_details_map, &sort_terms); - let remote_branches = sort_branch_rows(remote_branches, &commit_details_map, &sort_terms); + // Sort branches by timestamp (most recent first) + let branches_without_worktrees = sort_by_timestamp_desc_with_cache( + branches_without_worktrees, + &commit_details_map, + |(_, sha)| sha.as_str(), + ); + let remote_branches = + sort_by_timestamp_desc_with_cache(remote_branches, &commit_details_map, |(_, sha)| { + sha.as_str() + }); // Branches living in more than one worktree. Every row on such a branch // is flagged, including the one `wt` resolves to: that choice is git's @@ -2057,81 +2042,49 @@ pub fn collect( // Sorting Helpers // ============================================================================ -/// Sort `(branch name, SHA)` rows — the branch-only and remote-only groups. -/// -/// With no `[list] sort` spec this is timestamp descending, as before. These -/// rows have no worktree path, so a `path` term leaves them all equal and the -/// newest-first tiebreak inside [`super::sort::compare`] keeps their order. -fn sort_branch_rows( - rows: Vec<(String, String)>, +/// Sort items by timestamp descending using the pre-fetched commit-details map. +fn sort_by_timestamp_desc_with_cache( + items: Vec, commit_details: &std::collections::HashMap, - sort_terms: &[super::sort::SortTerm], -) -> Vec<(String, String)> { - // Facts are precomputed per row rather than derived inside the comparator, - // so the commit-details lookup is one per row instead of one per comparison. - let mut with_facts: Vec<_> = rows - .iter() - .map(|(name, sha)| { - ( - super::sort::SortFacts::new(None, Some(name.as_str()), sha, commit_details), - (name, sha), - ) + get_sha: F, +) -> Vec +where + F: Fn(&T) -> &str, +{ + // Embed timestamp in tuple to avoid parallel Vec and index lookups + let mut with_ts: Vec<_> = items + .into_iter() + .map(|item| { + let ts = commit_details + .get(get_sha(&item)) + .map_or(0, |(_, ts, _)| *ts); + (item, ts) }) .collect(); - with_facts.sort_by(|(a, _), (b, _)| super::sort::compare(sort_terms, a, b)); - with_facts - .into_iter() - .map(|(_, (name, sha))| (name.clone(), sha.clone())) - .collect() + with_ts.sort_by_key(|(_, ts)| std::cmp::Reverse(*ts)); + with_ts.into_iter().map(|(item, _)| item).collect() } /// Sort worktrees: current first, main second, then by timestamp descending. /// Uses the pre-fetched commit-details map for efficiency. -/// -/// A non-empty `[list] sort` spec replaces the whole order, current/primary -/// prefix included: pinning two rows to the top would defeat the ordering the -/// user asked for (`sort = ["path"]` exists to make the table read in path -/// order). Without a spec the prefix and the timestamp fallback are unchanged. fn sort_worktrees_with_cache<'a>( - worktrees: Vec<(&'a WorktreeInfo, WorktreeRef)>, + mut worktrees: Vec<(&'a WorktreeInfo, WorktreeRef)>, main_worktree_id: &WorktreeId, current_worktree_id: Option<&WorktreeId>, commit_details: &std::collections::HashMap, - sort_terms: &[super::sort::SortTerm], ) -> Vec<(&'a WorktreeInfo, WorktreeRef)> { - let pin_current_and_primary = sort_terms.is_empty(); - // Embed priority and facts in the tuple to avoid parallel Vecs and - // per-comparison map lookups. - let mut with_sort_key: Vec<_> = worktrees - .into_iter() - .map(|(wt, worktree_ref)| { - let priority = if pin_current_and_primary { - if current_worktree_id == Some(worktree_ref.id()) { - 0 // Current first - } else if worktree_ref.id() == main_worktree_id { - 1 // Main second - } else { - 2 // Rest by timestamp - } - } else { - 0 // A configured order decides every row; nothing is pinned - }; - let facts = super::sort::SortFacts::new( - Some(wt.path.as_path()), - wt.branch.as_deref(), - &wt.head, - commit_details, - ); - ((wt, worktree_ref), priority, facts) - }) - .collect(); - - with_sort_key.sort_by(|(_, a_priority, a), (_, b_priority, b)| { - a_priority - .cmp(b_priority) - .then_with(|| super::sort::compare(sort_terms, a, b)) + worktrees.sort_by_key(|(wt, worktree_ref)| { + let priority = if current_worktree_id == Some(worktree_ref.id()) { + 0 // Current first + } else if worktree_ref.id() == main_worktree_id { + 1 // Main second + } else { + 2 // Rest by timestamp + }; + let ts = commit_details.get(&wt.head).map_or(0, |(_, ts, _)| *ts); + (priority, std::cmp::Reverse(ts)) }); - with_sort_key.into_iter().map(|(row, _, _)| row).collect() + worktrees } // ============================================================================ diff --git a/src/commands/list/mod.rs b/src/commands/list/mod.rs index 426a9dcba..e536c0e26 100644 --- a/src/commands/list/mod.rs +++ b/src/commands/list/mod.rs @@ -137,7 +137,6 @@ pub mod model; pub mod progressive; mod progressive_table; pub(crate) mod render; -pub(crate) mod sort; // Layout is calculated in collect/mod.rs use anstyle::Style; diff --git a/src/commands/list/sort.rs b/src/commands/list/sort.rs deleted file mode 100644 index 529fa011c..000000000 --- a/src/commands/list/sort.rs +++ /dev/null @@ -1,366 +0,0 @@ -//! Row ordering for the `wt list` table and the `wt switch` picker. -//! -//! `[list] sort` names columns, most significant first, each optionally -//! prefixed with `-` for descending. Only columns whose value is known before -//! the skeleton paints are accepted — exactly those with no background task -//! ([`ColumnKind::required_tasks`] empty). Sorting on a streamed column would -//! mean either holding the skeleton for data that may be a network round-trip -//! away (`ci`) or reordering rows mid-render, which the progressive table -//! doesn't do. -//! -//! An empty spec keeps the historical order: current worktree first, primary -//! worktree second, then newest commit first. A non-empty spec drops that -//! two-row prefix — pinning rows to the top would defeat the ordering the user -//! asked for — but newest-commit-first survives as the final tiebreak. So the -//! empty spec and the tiebreak are the same code path, and rows a spec can't -//! distinguish (every branch-only row under `sort = ["path"]`, since branch -//! rows have no path) keep the order they have today. -//! -//! Rows are ordered within their group, never across: worktrees, then -//! branch-only rows, then remote rows, as before. `sort` reorders each group. - -use std::cmp::Ordering; -use std::path::Path; - -use super::columns::{COLUMN_SPECS, ColumnKind}; - -/// A column `[list] sort` can order rows by. -/// -/// The variants are exactly the built-ins that render without a background -/// task, so a new column must declare itself here or opt out in -/// [`SortKey::from_column`]'s exhaustive match. -/// `test_sortable_keys_are_exactly_the_task_free_columns` pins that -/// correspondence. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum SortKey { - Branch, - Path, - Commit, - /// Time since the head commit — the `age` column, counting *up* from the - /// committer date. - Age, - Message, -} - -/// One `[list] sort` entry: a key and its direction. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct SortTerm { - pub key: SortKey, - pub descending: bool, -} - -impl SortKey { - /// The sort key a built-in column provides, or `None` when its value isn't - /// known before the skeleton paints. - pub fn from_column(kind: ColumnKind) -> Option { - Some(match kind { - ColumnKind::Branch => SortKey::Branch, - ColumnKind::Path => SortKey::Path, - ColumnKind::Commit => SortKey::Commit, - ColumnKind::Time => SortKey::Age, - ColumnKind::Message => SortKey::Message, - // Streamed in after the skeleton, so unavailable when row order is - // chosen. Gutter and custom columns have no `[list] sort` name to - // reach them by (custom values expand after the sort, too). - ColumnKind::Gutter - | ColumnKind::Status - | ColumnKind::WorkingDiff - | ColumnKind::AheadBehind - | ColumnKind::BranchDiff - | ColumnKind::Summary - | ColumnKind::Upstream - | ColumnKind::CiStatus - | ColumnKind::Url - | ColumnKind::Custom(_) => return None, - }) - } -} - -/// Every sortable column name in display order, for error messages and docs. -pub fn sortable_names() -> Vec<&'static str> { - COLUMN_SPECS - .iter() - .filter(|spec| SortKey::from_column(spec.kind).is_some()) - .filter_map(|spec| spec.kind.config_name()) - .collect() -} - -/// Parse the `[list] sort` spec into ordered sort terms. -/// -/// Each entry is a column's kebab [`ColumnKind::config_name`], optionally -/// prefixed with `-` for descending. An empty input yields an empty spec (the -/// caller reads that as "default order"). Unknown names, unsortable columns, -/// and duplicate keys are hard errors so a typo can't silently render a -/// different order; the error lists every sortable name. Validated at the -/// `wt list` edge for the same reason `[list] columns` is — `ColumnKind` lives -/// in the command layer, out of reach of the config crate. -pub fn parse_sort_spec(names: &[String]) -> anyhow::Result> { - let mut terms: Vec = Vec::with_capacity(names.len()); - for name in names { - let (descending, bare) = match name.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, name.as_str()), - }; - let valid = || sortable_names().join(", "); - let key = match ColumnKind::from_config_name(bare) { - Some(kind) => SortKey::from_column(kind).ok_or_else(|| { - anyhow::anyhow!( - "Column {bare:?} in [list] sort is not sortable — its value arrives after the table paints. Sortable columns: {}", - valid() - ) - })?, - None => anyhow::bail!( - "Unknown sort key {name:?} in [list] sort. Sortable columns: {} (prefix with `-` for descending)", - valid() - ), - }; - if terms.iter().any(|term| term.key == key) { - anyhow::bail!("Duplicate sort key {name:?} in [list] sort"); - } - terms.push(SortTerm { key, descending }); - } - Ok(terms) -} - -/// The per-row values a sort term reads, all known before the skeleton paints. -/// -/// A missing value compares as empty: branch-only and remote rows carry no -/// path, and a detached worktree carries no branch. Rows only ever compare -/// within their own group, so the only mixed case is a detached worktree among -/// named ones, which sorts first under ascending `branch`. -pub struct SortFacts<'a> { - pub path: Option<&'a Path>, - pub branch: Option<&'a str>, - pub short_sha: &'a str, - pub timestamp: i64, - pub message: &'a str, -} - -impl<'a> SortFacts<'a> { - /// Build a row's facts, reading the commit fields from the batched - /// commit-details map keyed by `head`. A SHA the batch didn't cover (an - /// unborn branch, a failed batch) compares as empty/epoch, exactly as its - /// cells render. - pub fn new( - path: Option<&'a Path>, - branch: Option<&'a str>, - head: &str, - commit_details: &'a std::collections::HashMap, - ) -> Self { - let detail = commit_details.get(head); - Self { - path, - branch, - short_sha: detail.map_or("", |(short, _, _)| short.as_str()), - timestamp: detail.map_or(0, |(_, ts, _)| *ts), - message: detail.map_or("", |(_, _, subject)| subject.as_str()), - } - } -} - -/// Order two rows by `terms`, falling back to newest commit first. -/// -/// With `terms` empty this *is* newest-first, which is why the default order -/// and the tiebreak need no separate code path. -pub fn compare(terms: &[SortTerm], a: &SortFacts<'_>, b: &SortFacts<'_>) -> Ordering { - for term in terms { - let ordering = match term.key { - SortKey::Branch => a.branch.unwrap_or("").cmp(b.branch.unwrap_or("")), - SortKey::Path => a - .path - .unwrap_or(Path::new("")) - .cmp(b.path.unwrap_or(Path::new(""))), - SortKey::Commit => a.short_sha.cmp(b.short_sha), - // Age counts up from the committer date, so ascending age is - // newest first — the same direction as the default order — and - // `-age` puts the oldest commits on top. - SortKey::Age => b.timestamp.cmp(&a.timestamp), - SortKey::Message => a.message.cmp(b.message), - }; - let ordering = if term.descending { - ordering.reverse() - } else { - ordering - }; - if ordering != Ordering::Equal { - return ordering; - } - } - b.timestamp.cmp(&a.timestamp) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::collections::HashMap; - - fn facts<'a>( - path: Option<&'a str>, - branch: Option<&'a str>, - short_sha: &'a str, - timestamp: i64, - message: &'a str, - ) -> SortFacts<'a> { - SortFacts { - path: path.map(Path::new), - branch, - short_sha, - timestamp, - message, - } - } - - #[test] - fn test_sortable_keys_are_exactly_the_task_free_columns() { - // The promise `[list] sort` makes is "skeleton-time columns only", and - // a column is skeleton-time exactly when it consumes no background - // task. Adding a task-free column without a `SortKey` arm (or giving a - // streamed column one) breaks that promise silently; this catches it. - for spec in COLUMN_SPECS { - let kind = spec.kind; - let sortable = SortKey::from_column(kind).is_some(); - let skeleton_time = kind.required_tasks().is_empty() && kind.config_name().is_some(); - assert_eq!( - sortable, skeleton_time, - "{kind:?}: sortable={sortable} but skeleton-time={skeleton_time}" - ); - } - // Custom columns expand after the sort and are addressed by header, so - // they are unreachable from a sort spec. - assert_eq!(SortKey::from_column(ColumnKind::Custom(0)), None); - } - - #[test] - fn test_parse_sort_spec() { - let terms = parse_sort_spec(&["path".into(), "-age".into()]).unwrap(); - assert_eq!( - terms, - vec![ - SortTerm { - key: SortKey::Path, - descending: false - }, - SortTerm { - key: SortKey::Age, - descending: true - }, - ], - "terms keep configured order; `-` marks descending" - ); - - assert!(parse_sort_spec(&[]).unwrap().is_empty()); - - // A streamed column names a real column but can't order the skeleton. - let streamed = parse_sort_spec(&["ci".into()]).unwrap_err().to_string(); - assert!(streamed.contains("not sortable"), "{streamed}"); - assert!(streamed.contains("path"), "lists valid keys: {streamed}"); - - // A name that isn't a column at all is a different error, and still - // lists the valid keys. - let unknown = parse_sort_spec(&["bogus".into()]).unwrap_err().to_string(); - assert!(unknown.contains("Unknown sort key"), "{unknown}"); - assert!(unknown.contains("bogus"), "{unknown}"); - assert!(unknown.contains("branch"), "{unknown}"); - - // The `-` prefix is stripped before the name is resolved, so a bad name - // reports with its prefix intact rather than as a mystery. - let prefixed = parse_sort_spec(&["-nope".into()]).unwrap_err().to_string(); - assert!(prefixed.contains("\"-nope\""), "{prefixed}"); - - // The same key twice is a contradiction, whichever direction each carries. - let dup = parse_sort_spec(&["age".into(), "-age".into()]) - .unwrap_err() - .to_string(); - assert!(dup.contains("Duplicate sort key"), "{dup}"); - - // Gutter has no name, and matching is exact. - assert!(parse_sort_spec(&["gutter".into()]).is_err()); - assert!(parse_sort_spec(&["Path".into()]).is_err()); - } - - #[test] - fn test_compare_defaults_to_newest_first() { - let older = facts(Some("/a"), Some("a"), "aaa", 100, "a"); - let newer = facts(Some("/b"), Some("b"), "bbb", 200, "b"); - assert_eq!(compare(&[], &newer, &older), Ordering::Less); - assert_eq!(compare(&[], &older, &newer), Ordering::Greater); - } - - #[test] - fn test_compare_orders_by_terms_then_falls_back() { - let path_asc = parse_sort_spec(&["path".into()]).unwrap(); - // `/a` sorts before `/b` even though `/b` is newer. - let a = facts(Some("/a"), Some("z"), "aaa", 100, "z"); - let b = facts(Some("/b"), Some("a"), "bbb", 200, "a"); - assert_eq!(compare(&path_asc, &a, &b), Ordering::Less); - - // Descending flips it. - let path_desc = parse_sort_spec(&["-path".into()]).unwrap(); - assert_eq!(compare(&path_desc, &a, &b), Ordering::Greater); - - // Rows the spec can't tell apart (branch rows have no path) fall back to - // newest first, so they keep the order they have without a spec. - let no_path_old = facts(None, Some("old"), "ccc", 100, "c"); - let no_path_new = facts(None, Some("new"), "ddd", 200, "d"); - assert_eq!( - compare(&path_asc, &no_path_new, &no_path_old), - Ordering::Less - ); - } - - #[test] - fn test_compare_age_direction() { - let old = facts(Some("/a"), Some("a"), "aaa", 100, "a"); - let new = facts(Some("/b"), Some("b"), "bbb", 200, "b"); - // Ascending age = smallest age = newest commit, matching the default. - let age_asc = parse_sort_spec(&["age".into()]).unwrap(); - assert_eq!(compare(&age_asc, &new, &old), Ordering::Less); - // Descending age = oldest commit first. - let age_desc = parse_sort_spec(&["-age".into()]).unwrap(); - assert_eq!(compare(&age_desc, &old, &new), Ordering::Less); - } - - #[test] - fn test_compare_uses_later_terms_to_break_ties() { - let terms = parse_sort_spec(&["branch".into(), "message".into()]).unwrap(); - let a = facts(Some("/a"), Some("same"), "aaa", 200, "aardvark"); - let b = facts(Some("/b"), Some("same"), "bbb", 100, "zebra"); - assert_eq!( - compare(&terms, &a, &b), - Ordering::Less, - "equal branches fall through to the message term, not to the date" - ); - } - - #[test] - fn test_compare_by_commit() { - // The abbreviated SHA orders lexicographically. Not a meaningful - // ranking, but it groups a repeated commit together, and every - // skeleton-time column is offered rather than a hand-picked subset. - let terms = parse_sort_spec(&["commit".into()]).unwrap(); - let a = facts(Some("/a"), Some("a"), "abc1234", 100, "a"); - let b = facts(Some("/b"), Some("b"), "def5678", 200, "b"); - assert_eq!(compare(&terms, &a, &b), Ordering::Less); - assert_eq!(compare(&terms, &b, &a), Ordering::Greater); - } - - #[test] - fn test_facts_from_commit_details() { - let mut details = HashMap::new(); - details.insert( - "full-sha".to_string(), - ("abc1234".to_string(), 42_i64, "subject".to_string()), - ); - let known = SortFacts::new(None, Some("br"), "full-sha", &details); - assert_eq!(known.short_sha, "abc1234"); - assert_eq!(known.timestamp, 42); - assert_eq!(known.message, "subject"); - - // A SHA the batch didn't cover compares as empty/epoch, mirroring the - // placeholder cells its row renders. - let missing = SortFacts::new(None, Some("br"), "unknown", &details); - assert_eq!(missing.short_sha, ""); - assert_eq!(missing.timestamp, 0); - assert_eq!(missing.message, ""); - } -} diff --git a/src/commands/picker/mod.rs b/src/commands/picker/mod.rs index 2dd77449f..0be1bca6f 100644 --- a/src/commands/picker/mod.rs +++ b/src/commands/picker/mod.rs @@ -1720,15 +1720,11 @@ pub fn handle_picker( )); let preview_cache: PreviewCache = Arc::clone(&orchestrator.cache); - // Speculative warm-up: in the default order the picker sorts the current - // worktree first, and the default tab (UnifiedDiff = comparison base - // through the worktree, including untracked files) is what skim will - // render first. Kicking this off before `collect::collect` overlaps - // preview compute with list collection. - // A `[list] sort` spec moves the current worktree off row 0, and the row - // that lands there isn't known until collect has the commit-details batch - // this warm-up is racing — so under a spec this degrades to a plain cache - // prime, paying off only if the user navigates to the current worktree. + // Speculative warm-up: the picker sorts the current worktree first, and + // the default tab (UnifiedDiff = comparison base through the worktree, + // including untracked files) is what skim will render first. Kicking this + // off before `collect::collect` overlaps preview compute with list + // collection. // The real spawn later skips this key via `contains_key`. if let (Ok(Some(branch)), Ok(path), Ok(Some(head))) = ( current_worktree.branch(), diff --git a/src/config/user/sections.rs b/src/config/user/sections.rs index 4131ad5a5..9a2c189d3 100644 --- a/src/config/user/sections.rs +++ b/src/config/user/sections.rs @@ -194,20 +194,6 @@ pub struct ListConfig { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub columns: Vec, - /// Row order for the `wt list` table and the `wt switch` picker: column - /// names, most significant first, each optionally prefixed with `-` for - /// descending (`sort = ["path", "-age"]`). Only columns whose value is - /// known before the table paints are sortable — `branch`, `path`, - /// `commit`, `age`, `message`. Empty means the default order: current - /// worktree first, primary second, then newest commit first. A non-empty - /// value drops that two-row prefix and keeps newest-first only as a - /// tiebreak. Rows are ordered within their group (worktrees, branch-only, - /// remote), never across. Set as a TOML array in config files. - // TODO(list-columns-env): `WORKTRUNK__LIST__SORT` is unwired for the same - // reason `columns` is — see the note there. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub sort: Vec, - /// Custom columns, keyed by header text. See [`ListColumnConfig`]. /// /// *(Experimental — fields may change in future releases.)* @@ -270,14 +256,6 @@ impl Merge for ListConfig { } else { other.columns.clone() }; - // Same wholesale replace, for the same reason: a sort spec is an - // ordering, so merging terms across layers would invent an order - // neither layer asked for. - let sort = if other.sort.is_empty() { - self.sort.clone() - } else { - other.sort.clone() - }; Self { full: other.full.or(self.full), branches: other.branches.or(self.branches), @@ -286,7 +264,6 @@ impl Merge for ListConfig { json_schema: other.json_schema.or(self.json_schema), timeout_ms: other.timeout_ms.or(self.timeout_ms), columns, - sort, custom_columns, } } diff --git a/src/config/user/tests.rs b/src/config/user/tests.rs index 0051f91c0..8e0fc3be1 100644 --- a/src/config/user/tests.rs +++ b/src/config/user/tests.rs @@ -291,7 +291,6 @@ fn test_list_config_serde() { json_schema: None, timeout_ms: None, columns: vec!["branch".into(), "ci".into(), "path".into()], - sort: vec!["path".into()], custom_columns: Default::default(), }; let json = serde_json::to_string(&config).unwrap(); @@ -666,7 +665,6 @@ fn test_merge_list_config() { json_schema: None, timeout_ms: Some(2000), columns: vec!["branch".into(), "ci".into()], - sort: vec!["path".into()], custom_columns: Default::default(), }; let override_config = ListConfig { @@ -677,7 +675,6 @@ fn test_merge_list_config() { json_schema: None, timeout_ms: None, // Should fall back to base columns: Vec::new(), // Empty → fall back to base - sort: Vec::new(), // Empty → fall back to base custom_columns: Default::default(), }; @@ -688,28 +685,23 @@ fn test_merge_list_config() { assert_eq!(merged.summary, Some(true)); // From base assert_eq!(merged.timeout_ms, Some(2000)); // From base assert_eq!(merged.columns, vec!["branch", "ci"]); // From base (override empty) - assert_eq!(merged.sort, vec!["path"]); // From base (override empty) } #[test] fn test_merge_list_config_columns_replace() { // A non-empty override replaces the whole list (it's an ordering, not a - // keyed set), unlike the per-key union used for custom_columns. `sort` is - // an ordering too and follows the same rule. + // keyed set), unlike the per-key union used for custom_columns. let base = ListConfig { columns: vec!["branch".into(), "ci".into(), "path".into()], - sort: vec!["path".into(), "branch".into()], ..Default::default() }; let override_config = ListConfig { columns: vec!["status".into(), "branch".into()], - sort: vec!["-age".into()], ..Default::default() }; let merged = base.merge_with(&override_config); assert_eq!(merged.columns, vec!["status", "branch"]); - assert_eq!(merged.sort, vec!["-age"]); } #[test] @@ -1059,7 +1051,6 @@ fn test_list_config_accessor_methods_with_values() { json_schema: None, timeout_ms: Some(3000), columns: Vec::new(), - sort: Vec::new(), custom_columns: Default::default(), }; assert!(config.full()); diff --git a/tests/integration_tests/list_config.rs b/tests/integration_tests/list_config.rs index 454693332..f5cac7d2b 100644 --- a/tests/integration_tests/list_config.rs +++ b/tests/integration_tests/list_config.rs @@ -1253,226 +1253,3 @@ fn test_list_config_columns_cli_override(repo: TestRepo) { ); }); } - -// ========================================================================= -// [list] sort — row order (#3922) -// ========================================================================= - -/// Row order as `wt list --format json` reports it, which is the order collect -/// produced: the JSON serializer walks `all_items` without re-sorting, so it -/// reads the same order the table paints while being immune to column layout. -fn sorted_branches(repo: &TestRepo, config: &str) -> Vec { - fs::write(repo.test_config_path(), config).unwrap(); - - let mut cmd = wt_command(); - repo.configure_wt_cmd(&mut cmd); - cmd.args(["list", "--format=json"]) - .current_dir(repo.root_path()); - - let output = cmd.output().unwrap(); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!(output.status.success(), "wt list failed: {stderr}"); - let stdout = String::from_utf8_lossy(&output.stdout); - let json: serde_json::Value = serde_json::from_str(&stdout) - .unwrap_or_else(|e| panic!("not JSON ({e}):\n{stdout}\nstderr:\n{stderr}")); - json.as_array() - .unwrap() - .iter() - .map(|item| item["branch"].as_str().unwrap_or_default().to_string()) - .collect() -} - -/// `[list] sort` orders the rows by a skeleton-time column, and a configured -/// order replaces the default prefix outright — `main` is both the current and -/// the primary worktree here, so its move off the top is the whole point: -/// `sort = ["path"]` is asked for so the table reads in path order, which two -/// pinned rows would defeat. -#[rstest] -fn test_list_config_sort_replaces_default_order(repo: TestRepo) { - // The fixture pins one committer date across every commit, so the default - // order's date term is a wash and the stable sort leaves the features in - // `git worktree list` order behind the pinned `main` row. - assert_eq!( - sorted_branches(&repo, "[list]\n"), - vec!["main", "feature-a", "feature-b", "feature-c"], - "default: current/primary first, then newest commit first" - ); - - assert_eq!( - sorted_branches(&repo, "[list]\nsort = [\"branch\"]\n"), - vec!["feature-a", "feature-b", "feature-c", "main"], - "an explicit sort orders every row, current/primary included" - ); - - assert_eq!( - sorted_branches(&repo, "[list]\nsort = [\"-branch\"]\n"), - vec!["main", "feature-c", "feature-b", "feature-a"], - "a `-` prefix reverses the key" - ); -} - -/// Sorting by `path` — the request the feature came from: the listing follows -/// the physical layout (`repo`, `repo.feature-a`, …) rather than commit -/// recency. Descending walks the same layout backwards. -#[rstest] -fn test_list_config_sort_by_path(repo: TestRepo) { - assert_eq!( - sorted_branches(&repo, "[list]\nsort = [\"path\"]\n"), - vec!["main", "feature-a", "feature-b", "feature-c"], - ); - assert_eq!( - sorted_branches(&repo, "[list]\nsort = [\"-path\"]\n"), - vec!["feature-c", "feature-b", "feature-a", "main"], - ); -} - -/// Rows sort inside their group and never across it: with `--branches` on, a -/// branch-only row stays below every worktree however the spec would order the -/// two. Branch rows have no path, so a `path` spec leaves them equal to each -/// other and the newest-first tiebreak keeps the order they have today. -#[rstest] -fn test_list_config_sort_keeps_row_groups(repo: TestRepo) { - // A branch with no worktree, named to sort before every worktree branch. - repo.create_branch("aaa-lonely"); - - let order = sorted_branches(&repo, "[list]\nbranches = true\nsort = [\"branch\"]\n"); - let lonely_at = order.iter().position(|b| b == "aaa-lonely").unwrap(); - assert_eq!( - lonely_at, - order.len() - 1, - "the branch-only group stays below the worktree group: {order:?}" - ); - assert_eq!( - &order[..lonely_at], - ["feature-a", "feature-b", "feature-c", "main"], - "worktrees are still sorted by the spec: {order:?}" - ); -} - -/// A column that streams in after the table paints can't order the skeleton, so -/// naming one is an error that says why and lists the keys that do work — -/// rather than a silently ignored setting. -#[rstest] -fn test_list_config_sort_unsortable_column_errors(repo: TestRepo) { - fs::write( - repo.test_config_path(), - r#"[list] -sort = ["ci"] -"#, - ) - .unwrap(); - - let mut cmd = wt_command(); - repo.configure_wt_cmd(&mut cmd); - cmd.arg("list").current_dir(repo.root_path()); - - let output = cmd.output().unwrap(); - assert!( - !output.status.success(), - "an unsortable column should abort wt list" - ); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("not sortable"), "stderr: {stderr}"); - assert!( - stderr.contains("\"ci\""), - "stderr names the column: {stderr}" - ); - assert!( - stderr.contains("path"), - "stderr lists the sortable keys: {stderr}" - ); -} - -/// A name that isn't a column at all aborts too, with the sortable keys and -/// the `-` prefix spelled out. -#[rstest] -fn test_list_config_sort_unknown_key_errors(repo: TestRepo) { - fs::write( - repo.test_config_path(), - r#"[list] -sort = ["bogus"] -"#, - ) - .unwrap(); - - let mut cmd = wt_command(); - repo.configure_wt_cmd(&mut cmd); - cmd.arg("list").current_dir(repo.root_path()); - - let output = cmd.output().unwrap(); - assert!( - !output.status.success(), - "an unknown key should abort wt list" - ); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!(stderr.contains("Unknown sort key"), "stderr: {stderr}"); - assert!( - stderr.contains("bogus"), - "stderr names the bad key: {stderr}" - ); - assert!( - stderr.contains("branch"), - "stderr lists valid keys: {stderr}" - ); -} - -/// The per-invocation form: `wt --config-set 'list.sort=[…]' list` gives a -/// named view (via an alias) without pinning one order into the config file. -#[rstest] -fn test_list_config_sort_cli_override(repo: TestRepo) { - let mut cmd = wt_command(); - repo.configure_wt_cmd(&mut cmd); - cmd.args([ - "--config-set", - r#"list.sort = ["-branch"]"#, - "list", - "--format=json", - ]) - .current_dir(repo.root_path()); - - let output = cmd.output().unwrap(); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!(output.status.success(), "exit code should be 0: {stderr}"); - let stdout = String::from_utf8_lossy(&output.stdout); - let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); - let branches: Vec<&str> = json - .as_array() - .unwrap() - .iter() - .map(|item| item["branch"].as_str().unwrap_or_default()) - .collect(); - assert_eq!(branches, ["main", "feature-c", "feature-b", "feature-a"]); -} - -/// `age` counts up from the commit date, so ascending `age` is newest first — -/// the same direction as the default order — and `-age` puts the oldest commit -/// on top. The fixture pins one committer date across every commit, so this -/// commits on `feature-a` with an explicitly later one: that row is then the -/// only one the date term can place, and it moves from one end to the other. -#[rstest] -fn test_list_config_sort_by_age(repo: TestRepo) { - let worktree = repo.worktree_path("feature-a").to_path_buf(); - std::fs::write(worktree.join("newer.txt"), "newer\n").unwrap(); - repo.run_git_in(&worktree, &["add", "newer.txt"]); - repo.git_command() - .args(["commit", "-q", "-m", "A newer commit"]) - .current_dir(&worktree) - .env("GIT_AUTHOR_DATE", "2026-06-01T00:00:00Z") - .env("GIT_COMMITTER_DATE", "2026-06-01T00:00:00Z") - .run() - .unwrap(); - - let newest_first = sorted_branches(&repo, "[list]\nsort = [\"age\"]\n"); - assert_eq!( - newest_first.first().map(String::as_str), - Some("feature-a"), - "ascending age is newest first: {newest_first:?}" - ); - - let oldest_first = sorted_branches(&repo, "[list]\nsort = [\"-age\"]\n"); - assert_eq!( - oldest_first.last().map(String::as_str), - Some("feature-a"), - "`-age` puts the newest commit last: {oldest_first:?}" - ); -} diff --git a/tests/integration_tests/switch_picker_dry_run.rs b/tests/integration_tests/switch_picker_dry_run.rs index 9ec79ca2e..9295c3084 100644 --- a/tests/integration_tests/switch_picker_dry_run.rs +++ b/tests/integration_tests/switch_picker_dry_run.rs @@ -682,51 +682,3 @@ template = "{{ branhc }}" "rows still render without the broken column, got: {stdout}" ); } - -/// An unsortable `[list] sort` key aborts `wt list`, but the picker can't -/// surface an abort mid-render: it stashes a warning and falls back to the -/// default row order. Covers the `progressive_handler` degradation arm the sort -/// spec takes in `collect`. -#[rstest] -fn test_picker_dry_run_invalid_sort_key_degrades(mut repo: TestRepo) { - repo.add_worktree("feature-a"); - std::fs::write( - repo.test_config_path(), - r#"[list] -sort = ["ci"] -"#, - ) - .unwrap(); - - let output = repo - .wt_command() - .args(["switch"]) - .env("WORKTRUNK_PICKER_DRY_RUN", "1") - .output() - .unwrap(); - - assert!( - output.status.success(), - "picker degrades on a bad sort key instead of aborting; stderr:\n{}", - String::from_utf8_lossy(&output.stderr) - ); - - let stderr = String::from_utf8(output.stderr).expect("stderr is utf-8"); - assert!( - stderr.contains("Sort order ignored"), - "expected a stashed degradation warning on stderr, got: {stderr}" - ); - assert!( - stderr.contains("not sortable"), - "the warning carries the reason, got: {stderr}" - ); - - let stdout = String::from_utf8(output.stdout).expect("stdout is valid JSON"); - let parsed: serde_json::Value = serde_json::from_str(&stdout).expect("stdout is valid JSON"); - let rows = parsed["rows"].as_array().expect("top-level `rows` array"); - assert!( - rows.iter() - .any(|r| r.as_str().unwrap_or("").contains("feature-a")), - "rows still render in the default order, got: {stdout}" - ); -} diff --git a/tests/snapshots/integration__integration_tests__help__help_config_create.snap b/tests/snapshots/integration__integration_tests__help__help_config_create.snap index 61ba49e53..f8d3df2b8 100644 --- a/tests/snapshots/integration__integration_tests__help__help_config_create.snap +++ b/tests/snapshots/integration__integration_tests__help__help_config_create.snap @@ -179,7 +179,6 @@ Creates ~/.config/worktrunk/config.toml with the following content:   # json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning   #   # columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -  # sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order)   #   # timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables   # @@ -219,40 +218,6 @@ Creates ~/.config/worktrunk/config.toml with the following content:   # needs an LLM command (`[commit.generation]`), `url` needs a `[list] url`   # template — since listing can't supply the data.   # -  # #### Row order -  # -  # `sort` orders the rows of the same two surfaces, `columns` orders their -  # columns. Keys are column names, most significant first, each optionally -  # prefixed with `-` for descending: `["path"]` makes the table mirror the -  # directory layout, `["-age"]` puts the oldest commit on top, and -  # `["branch", "-age"]` groups by branch name with the newest commit first inside -  # each name. -  # -  # Sortable columns are the ones whose value is known before the table paints: -  # `branch`, `path`, `commit`, `age`, `message`. The rest (`status`, `ci`, -  # `upstream`, the diff columns, custom columns) stream in behind the first frame, -  # so ordering on them would mean either holding the table for a network round -  # trip or reordering rows under the cursor. Naming one is an error rather than a -  # silent reinterpretation, as is an unknown key or the same key twice. -  # -  # `age` counts up from the commit date, so ascending `age` is newest first — the -  # same direction as the default — and `-age` puts the oldest commits on top. -  # -  # Omitting `sort` keeps the default order: the current worktree first, the -  # primary worktree second, then newest commit first. Any `sort` value replaces -  # that order outright, including the two pinned rows, since pinning them would -  # defeat the order being asked for. Newest-first survives as the final tiebreak, -  # so rows a spec can't separate keep the order they already had — branch-only -  # rows under `sort = ["path"]`, for instance, since only worktrees have a path. -  # -  # Rows sort within their group and never across it: worktrees first, then -  # branch-only rows (`--branches`), then remote-only rows (`--remotes`), each -  # group ordered by the spec. -  # -  # Unlike `columns`, `sort` does reach `--format json`: reordering an array -  # neither narrows the payload nor adds a fetch, so the every-field contract -  # holds either way. -  #   # #### Custom columns [experimental]   #   # Custom columns add per-branch context to the `wt list` table. Each diff --git a/tests/snapshots/integration__integration_tests__help__help_config_long.snap b/tests/snapshots/integration__integration_tests__help__help_config_long.snap index 2515a80a8..af726a349 100644 --- a/tests/snapshots/integration__integration_tests__help__help_config_long.snap +++ b/tests/snapshots/integration__integration_tests__help__help_config_long.snap @@ -227,7 +227,6 @@ Persistent flag values for wt list. Override on command line as needed.   json-schema = 2 # JSON output schema: 2 (envelope) or 1 (bare array, the current default); unset emits 1 with a warning     columns = ["branch", "status", "ci", "path"] # Columns to show, in order — built-ins or custom headers (omit for the default set) -  sort = ["path"] # Row order, most significant key first; `-` prefixes a descending key (omit for the default order)     timeout-ms = 0 # Wall-clock budget for the entire collect phase; 0 disables @@ -267,40 +266,6 @@ named one. A column whose data source is missing still stays hidden — summ needs an LLM command ([commit.generation]), url needs a [list] url template — since listing can't supply the data. -Row order - -sort orders the rows of the same two surfaces, columns orders their -columns. Keys are column names, most significant first, each optionally -prefixed with - for descending: ["path"] makes the table mirror the -directory layout, ["-age"] puts the oldest commit on top, and -["branch", "-age"] groups by branch name with the newest commit first inside -each name. - -Sortable columns are the ones whose value is known before the table paints: -branch, path, commit, age, message. The rest (status, ci, -upstream, the diff columns, custom columns) stream in behind the first frame, -so ordering on them would mean either holding the table for a network round -trip or reordering rows under the cursor. Naming one is an error rather than a -silent reinterpretation, as is an unknown key or the same key twice. - -age counts up from the commit date, so ascending age is newest first — the -same direction as the default — and -age puts the oldest commits on top. - -Omitting sort keeps the default order: the current worktree first, the -primary worktree second, then newest commit first. Any sort value replaces -that order outright, including the two pinned rows, since pinning them would -defeat the order being asked for. Newest-first survives as the final tiebreak, -so rows a spec can't separate keep the order they already had — branch-only -rows under sort = ["path"], for instance, since only worktrees have a path. - -Rows sort within their group and never across it: worktrees first, then -branch-only rows (--branches), then remote-only rows (--remotes), each -group ordered by the spec. - -Unlike columns, sort does reach --format json: reordering an array -neither narrows the payload nor adds a fetch, so the every-field contract -holds either way. - Custom columns [experimental] Custom columns add per-branch context to the wt list table. Each