Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions src/commands/remove.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
//! The `wt remove` command: validate removal targets, approve hooks, and
//! dispatch each removal to the output handler.
//!
//! Target failures are reported independently; later targets still run and
//! the batch exits unsuccessfully. An interrupt cancels the batch immediately.

use std::collections::HashSet;
use std::path::Path;

use anyhow::Context;
use worktrunk::HookType;
use worktrunk::config::UserConfig;
use worktrunk::git::{BranchDeletionMode, ErrorExt, GitError, Repository, ResolvedWorktree};
use worktrunk::git::{
BranchDeletionMode, ErrorExt, GitError, Repository, ResolvedWorktree, WorktrunkError,
};
use worktrunk::styling::{eprintln, info_message};

use crate::cli::{RemoveArgs, SwitchFormat};
Expand Down Expand Up @@ -548,27 +553,26 @@ pub fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()>
announcer.flush()?;
Ok(fate)
};
// Fates in execution order, which is also the JSON order below.
let mut fates = Vec::new();
for result in &plans.others {
fates.push(run(result)?);
}
for result in &plans.branch_only {
fates.push(run(result)?);
}
if let Some(ref result) = plans.current {
fates.push(run(result)?);
let mut failed = !plans.errors.is_empty();
let mut json_items = Vec::new();
for result in all_plans() {
match run(result) {
Ok(fate) => {
if json_mode {
json_items.push(result.to_json(fate));
}
}
Err(e) => {
if let Some(signal) = e.interrupt_signal() {
return Err(WorktrunkError::Interrupted { signal, hint: None }.into());
}
crate::print_command_error(&e);
failed = true;
}
}
}

if json_mode {
let json_items: Vec<serde_json::Value> = plans
.others
.iter()
.chain(&plans.branch_only)
.chain(plans.current.as_ref())
.zip(fates)
.map(|(removal, fate)| removal.to_json(fate))
.collect();
print_json(&json_items)?;
}

Expand All @@ -577,7 +581,7 @@ pub fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()>
// it never delays the user-visible progress/success messages.
super::process::run_internal_sweep(&repo);

if !plans.errors.is_empty() {
if failed {
anyhow::bail!("");
}

Expand Down
32 changes: 21 additions & 11 deletions src/git/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ pub trait ErrorExt {
/// short single-line label.
///
/// Use this when embedding a sub-error's text inside another typed error's
/// message field (e.g., `GitError::WorktreeRemovalFailed::error`,
/// `GitError::PushFailed::error`) so the user sees git's real reason
/// message field (e.g., `GitError::PushFailed::error`) so the user sees git's
/// real reason
/// rather than just the [`CommandError`] single-line summary.
fn display_message(&self) -> String;

Expand Down Expand Up @@ -382,7 +382,7 @@ impl SwitchSuggestionCtx {
/// println!("branch {branch} already exists");
/// }
/// ```
#[derive(Debug, Clone)]
#[derive(Debug)]
pub enum GitError {
// Git state errors
/// A worktree is not on a branch, so a command needing one refuses.
Expand Down Expand Up @@ -539,7 +539,7 @@ pub enum GitError {
WorktreeRemovalFailed {
branch: String,
path: PathBuf,
error: String,
error: anyhow::Error,
/// Top-level entries remaining in the directory (for "Directory not empty" diagnostics)
remaining_entries: Option<Vec<String>>,
},
Expand Down Expand Up @@ -720,7 +720,14 @@ pub enum GitError {
},
}

impl std::error::Error for GitError {}
impl std::error::Error for GitError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::WorktreeRemovalFailed { error, .. } => Some(error.as_ref()),
_ => None,
}
}
}

/// `"1 path with unresolved conflicts"` — the shared tail of every message
/// about an unmerged index. [`GitError::UnmergedPaths`] refuses outright;
Expand Down Expand Up @@ -1335,8 +1342,9 @@ impl GitError {
remaining_entries,
..
} => {
let error = error.display_message();
let title = self.title();
write!(f, "{}", format_error_block(error_message(&title), error))?;
write!(f, "{}", format_error_block(error_message(&title), &error))?;
if let Some(entries) = remaining_entries {
const MAX_SHOWN: usize = 10;
let listing = if entries.len() > MAX_SHOWN {
Expand Down Expand Up @@ -2336,7 +2344,7 @@ mod tests {
GitError::WorktreeRemovalFailed {
branch: "feature".into(),
path: PathBuf::from("/tmp/repo.feature"),
error: "fatal: …".into(),
error: anyhow::anyhow!("fatal: …"),
remaining_entries: None,
}.to_string(),
@"Failed to remove worktree for feature @ /tmp/repo.feature"
Expand All @@ -2361,14 +2369,15 @@ mod tests {
let inner = GitError::BranchAlreadyExists {
branch: "feature".into(),
};
let expected = inner.to_string();
let wrapped = GitError::WithSwitchSuggestion {
source: Box::new(inner.clone()),
source: Box::new(inner),
ctx: SwitchSuggestionCtx {
extra_flags: vec!["--execute=claude".into()],
trailing_args: vec![],
},
};
assert_eq!(inner.to_string(), wrapped.to_string());
assert_eq!(expected, wrapped.to_string());

// WorktrunkError variants
assert_snapshot!(
Expand Down Expand Up @@ -2882,15 +2891,16 @@ mod tests {
action: Some("merge".into()),
worktree: None,
};
let expected = inner.to_string();
let wrapped = GitError::WithSwitchSuggestion {
source: Box::new(inner.clone()),
source: Box::new(inner),
ctx: SwitchSuggestionCtx {
extra_flags: vec!["--execute=claude".into()],
trailing_args: vec!["Check my emails".into()],
},
};
// Errors without switch suggestions should render identically
assert_eq!(inner.to_string(), wrapped.to_string());
assert_eq!(expected, wrapped.to_string());
}

fn sample_command_error() -> CommandError {
Expand Down
4 changes: 2 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1338,8 +1338,8 @@ mod tests {
assert!(out.contains("git fetch failed"));
}

/// Codex P2: typed `GitError` wrappers (e.g., `WorktreeRemovalFailed`,
/// `PushFailed`) embed a stringified sub-error into their `error`
/// Typed `GitError` wrappers (e.g., `PushFailed`) embed a stringified
/// sub-error into their `error`
/// field. With `display_message`, that field carries git's stderr
/// rather than our `CommandError` summary.
#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/output/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1916,7 +1916,7 @@ fn handle_detached_removed_worktree_output(
branch: path_dir_name(ctx.worktree_path).to_string(),
path: ctx.worktree_path.to_path_buf(),
remaining_entries: list_remaining_entries(ctx.worktree_path),
error: err.display_message(),
error: err,
})?;
let (files, bytes) = output
.staged_path
Expand Down Expand Up @@ -1995,7 +1995,7 @@ fn handle_named_removed_worktree_foreground(
branch: branch_name.into(),
path: ctx.worktree_path.to_path_buf(),
remaining_entries: list_remaining_entries(ctx.worktree_path),
error: err.display_message(),
error: err,
})?;
let stats = output
.staged_path
Expand Down
17 changes: 12 additions & 5 deletions tests/integration_tests/git_error_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ fn worktree_errors_render() {
GitError::WorktreeRemovalFailed {
branch: "feature-x".into(),
path: PathBuf::from("/tmp/repo.feature-x"),
error: "fatal: worktree is dirty\nerror: could not remove worktree".into(),
error: anyhow::anyhow!(
"fatal: worktree is dirty\nerror: could not remove worktree"
),
remaining_entries: None,
}
.render(),
Expand All @@ -52,7 +54,9 @@ fn worktree_errors_render() {
GitError::WorktreeRemovalFailed {
branch: "feature-x".into(),
path: PathBuf::from("/tmp/repo.feature-x"),
error: "error: failed to delete '/tmp/repo.feature-x': Directory not empty".into(),
error: anyhow::anyhow!(
"error: failed to delete '/tmp/repo.feature-x': Directory not empty"
),
remaining_entries: Some(vec![
".vite/".into(),
"node_modules/".into(),
Expand All @@ -66,8 +70,9 @@ fn worktree_errors_render() {
GitError::WorktreeRemovalFailed {
branch: "feature-x".into(),
path: PathBuf::from("/tmp/repo.feature-x"),
error: "error: failed to remove '/tmp/repo.feature-x/target': Permission denied"
.into(),
error: anyhow::anyhow!(
"error: failed to remove '/tmp/repo.feature-x/target': Permission denied"
),
remaining_entries: Some(vec!["target/".into()]),
}
.render(),
Expand All @@ -77,7 +82,9 @@ fn worktree_errors_render() {
GitError::WorktreeRemovalFailed {
branch: "feature-x".into(),
path: PathBuf::from("/tmp/repo.feature-x"),
error: "error: failed to delete '/tmp/repo.feature-x': Directory not empty".into(),
error: anyhow::anyhow!(
"error: failed to delete '/tmp/repo.feature-x': Directory not empty"
),
remaining_entries: Some((0..15).map(|i| format!("dir-{i:02}/")).collect()),
}
.render(),
Expand Down
Loading
Loading