feat: unified worktree-delete modal with contextual option#39
Merged
Conversation
Removing a worktree used to cascade through up to three modals: worktree confirm → delete-branch prompt → force-delete prompt when the branch had unmerged commits. The cascade was surprising and easy to muscle-memory through. Collapse to a single modal with keep + exactly one contextual delete option. On open, `git::is_branch_merged` (merge-base check via libgit2, offline) decides whether `git branch -d` will succeed and picks the variant: - merged → `[k] keep` or `[d] delete` (regular `-d`) - unmerged → `[k] keep` or `[D] force-delete` (-D) Keys are absolute rather than arrow-navigable: `k`/Enter for keep, `d` for regular delete, `D` for force delete, `Esc` for cancel. Capital-D is deliberately distinct from lowercase so muscle-memory Enter cannot silently destroy unmerged work. Pressing `d` on an unmerged branch shows an inline "use D to force-delete" hint and keeps the modal open instead of chaining to a third modal. The PR-open warning only renders in the unmerged (force-delete) case; a regular `-d` on a merged branch is safe for an open PR so the warning is suppressed there. State changes: - Drop `Modal::ConfirmDeleteBranch` and `Modal::ForceDeleteBranch` variants entirely — the cascade disappears. - `Modal::ConfirmRemoveWorktree` gains `variant: DeleteVariant`, `pr_number: Option<u32>`, and `error: Option<String>` for the inline hint. - New `DeleteChoice` action enum and `AppMessage::ConfirmWorktreeDeletion(DeleteChoice)`. - Dedicated `remove_worktree_keys` handler in `main.rs`. Six new tests cover both variants (merged / unmerged detection), the three outcomes (keep / delete / force-delete), and the inline-error path for `d` on unmerged. Co-Authored-By: Claude <noreply@anthropic.com>
sebasv
force-pushed
the
feat/v1.3-delete-worktree-modal
branch
from
d24f447 to
43e5cc0
Compare
is_branch_merged previously preferred origin/<base> over local <base> and ignored the local ref entirely. When local main has unpushed commits, a freshly-cut branch off local main shares its tip — git branch -d would happily delete it — but the function reported the branch as unmerged because origin/<base> was behind. Result: every fresh `create worktree → delete worktree` pair triggered the "force-delete will discard work" warning. Check both refs and consider the branch merged when its tip is at, or reachable from, either local <base> or origin/<base>. Co-Authored-By: Claude <noreply@anthropic.com>
Two bugs collided when deleting a fresh worktree pair: the rendered "failed to delete branch" warning, and the failure itself. The warning came from `eprintln!` inside running TUI code. stderr lands on the alt screen and corrupts the rendering, with the message appearing in random cells as the diff repaints. Add `paths::log_warning` that appends to ~/.cache/grove/grove.log and route every TUI-time `eprintln!` in app.rs through it. The deletion itself failed because `git branch -d` re-runs its merged check against the branch's *upstream* (when one is configured via `branch.autoSetupMerge` etc.), not the base we already verified. A fresh branch reachable from local main but ahead of origin/main is exactly the case our `is_branch_merged` approves and `git branch -d` rejects. Switch `delete_branch` (and `force_delete_branch`) to libgit2's unconditional delete — the safety check has already run. Regression test mirrors the upstream-behind setup. Co-Authored-By: Claude <noreply@anthropic.com>
Background warnings now go to ~/.cache/grove/grove.log silently — useful for not corrupting the rendered TUI but invisible to anyone who isn't tailing the file in another window. Add a `Modal::ViewLog` opened with `L` from the sidebar. Reads the last 256 KB of the log so multi-MB files don't blow memory or stall the open; drops the leading (likely truncated) line when we seek past byte 0. j/k scroll one line, PgUp/PgDn move 20, g/G jump to top/bottom, mouse wheel scrolls; Esc/q/L close. Empty-log state shows a placeholder hint rather than a blank box, since users will hit `L` curious on first run when nothing has been logged yet. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of the v1.3 plan (#31). Collapses the cascading worktree-removal flow (up to three modals: remove worktree → delete branch prompt → force-delete prompt) into a single modal with keep + exactly one contextual delete option.
Variant selection. On open, `git::is_branch_merged` (merge-base check via libgit2, no network) picks:
Absolute keys, no arrow navigation. `k`/Enter keep, `d` delete, `D` force-delete, `Esc` cancel. Capital-D is deliberately distinct from lowercase so muscle-memory Enter can't silently destroy unmerged work. `d` on an unmerged branch shows an inline "use D to force-delete" hint and keeps the modal open — no chained force-delete modal.
PR-open warning only renders in the unmerged (force-delete) case, since regular `-d` on a merged branch is safe for an open PR.
Dropped: `Modal::ConfirmDeleteBranch` and `Modal::ForceDeleteBranch` variants entirely — the cascade disappears.
Test plan
Release note
Keybind change: this modal previously accepted `y`/`n`/Enter. The new layout uses `k` / `d` / `D` / `Esc`. Users with `y`-muscle-memory will get a no-op rather than a destructive action — we deliberately don't rebind `y` since it's ambiguous about which of the three options it picks.
🤖 Generated with Claude Code