fix(sync): include git stderr in branch-delete fatal error message#719
Conversation
When branch deletion during prune fails for a reason other than worktree ownership, surface the actual git stderr instead of the opaque 'exit status 1' returned by exec.ExitError. Part of aviator-co#656
Current Aviator status
This PR was merged using Aviator.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
✅ FlexReview StatusCommon Owner:
Review SLO: |
There was a problem hiding this comment.
Code Review
This pull request improves error reporting when deleting merged branches in the Git UI. It now captures and displays the standard error output from the Git command if an exit error occurs, providing more specific context than a generic exit status. I have no feedback to provide.
| errMsg = stderr | ||
| } | ||
| } | ||
| deletionErr = errors.Errorf("cannot delete merged branch %q: %s", branch.branch.Short(), errMsg) |
There was a problem hiding this comment.
we're calling errutils.As[*exec.ExitError](err) twice on the same err here, once above for the worktree check and once here for stderr. can we just extract stderr once at the top of the error block, then branch on strings.Contains(stderr, "used by worktree") for the worktree case and fall through to the fatal path otherwise? also lets us drop the else after continue.
Per tulioz's review on aviator-co#719: avoid calling errutils.As[*exec.ExitError](err) twice on the same err. Extract stderr at the top of the error block, branch on strings.Contains for the worktree case, and drop the else after continue. Signed-off-by: Matt Van Horn <[email protected]>
Summary
Partially addresses #656. When
av sync --prune(or any code path hittingrunDeleteininternal/git/gitui/prune.go) fails to delete a merged branch for a reason other than worktree ownership, the error message now includes the git stderr instead of the opaqueexit status 1.Why this matters
Issue #656 reports the user-facing error:
The "cannot delete merged branch" wrapper text traces to line 235 of
prune.go, where the fatal-error path formatted the underlying error with%v. Since the error is an*exec.ExitError,%vresolves toexit status 1— stripping off the stderr git wrote to explain the failure. PR #660 already handled the specific "used by worktree" phrasing by catching that stderr and producing a recovery-oriented message, but any other failure reason still fell into this opaque path.What changed
internal/git/gitui/prune.go— insiderunDelete(), theelsebranch (non-worktree fatal error) now pullsexiterr.Stderroff the*exec.ExitError(sameerrutils.Ashelper the worktree check above already uses) and passes it to the error message:So the user now sees whatever git actually said rather than the wrapped shell status.
Scope notes
av syncneeds to provide a better error message when branch can't be deleted due to worktrees #656's second paragraph is already handled by the unconditionalCheckoutInitialState()call after the delete loop; no change needed there.Part of #656rather thanClosesbecause the issue's language covers worktree-specific UX that was delivered in Added message for branch deletion error due to worktrees #660, and this PR fills the remaining surface for non-worktree failures.Testing
Part of #656
This contribution was developed with AI assistance (Codex).