feat(stack reorder): Delete branches removed from reorder plan (#174)

# Example

Have a branch structure:
```
* 2023-06-08-two-b ce20b00  (HEAD -> 2023-06-08-two)
| 
* 2023-06-08-two-a 1f8f16b 
| 
* 2023-06-08-one-b 061ed6e  (2023-06-08-one)
| 
* 2023-06-08-one-a a4db90d 
| 
* 2023-05-30-one-a (#957) 87232f0  (origin/main, origin/HEAD, main)
```

Run `av stack reorder` and delete the `stack-branch 2023-06-08-two` line (effectively merging all commits into `2023-06-08-one`):

<img width="817" alt="image" src="https://github.com/aviator-co/av/assets/773453/3887bbce-d805-4910-a84b-2c52b8e766e6">

Choose `e` to edit the plan:
<img width="817" alt="image" src="https://github.com/aviator-co/av/assets/773453/b72f207e-10e3-472a-b14a-01197a4bb725">
(looks like unfortunately we lose the comment information, but this can be fixed in a future change)

Save the same plan, choose `o` to orphan the branches:
<img width="817" alt="image" src="https://github.com/aviator-co/av/assets/773453/18ecec96-3e2e-412d-bb9c-e99841dc832d">

Confirmed that `2023-06-08-two` still exists and hasn't had it's HEAD modified.

Reset to the state above, but this time choose `d` to delete the branches:
<img width="817" alt="image" src="https://github.com/aviator-co/av/assets/773453/b74a5511-726c-48be-adee-c557b0e8d5c3">

cmd/av/stack_reorder.go

@@ -1,10 +1,13 @@
 package main
 
 import (
+	"bufio"
 	"fmt"
 	"os"
 	"strings"
 
+	"github.com/aviator-co/av/internal/git"
+
 	"github.com/aviator-co/av/internal/actions"
 	"github.com/aviator-co/av/internal/config"
 	"github.com/aviator-co/av/internal/meta"
@@ -92,38 +95,15 @@ var stackReorderCmd = &cobra.Command{
 				)
 				return actions.ErrExitSilently{ExitCode: 127}
 			}
-			plan, err := reorder.CreatePlan(repo, db.ReadTx(), root)
+			initialPlan, err := reorder.CreatePlan(repo, db.ReadTx(), root)
 			if err != nil {
 				return err
 			}
 
-			// TODO:
-			// What should we do if the plan removes branches? Currently,
-			// we just don't edit those branches. So if a user edits
-			//     sb one
-			//     pick 1a
-			//     sb two
-			//     pick 2a
-			// and deletes the line for `sb two`, then the reorder will modify
-			// one to contain 1a/2a, but we don't modify two so it'll still be
-			// considered stacked on top of one.
-			// We can probably delete the branch and also emit a message to the
-			// user to the effect of
-			//     Deleting branch `two` because it was removed from the reorder.
-			//     To restore the branch, run `git switch -C two <OLD HEAD>`.
-			// just to make sure they can recover their work. (They already would
-			// be able to using `git reflog` but generally only advanced Git
-			// users think to do that).
-			plan, err = reorder.EditPlan(repo, plan)
+			plan, err := stackReorderEditPlan(repo, initialPlan)
 			if err != nil {
 				return err
 			}
-			if len(plan) == 0 {
-				_, _ = fmt.Fprint(os.Stderr,
-					colors.Failure("ERROR: reorder plan is empty\n"),
-				)
-				return actions.ErrExitSilently{ExitCode: 127}
-			}
 
 			logrus.WithFields(logrus.Fields{
 				"plan":           plan,
@@ -169,3 +149,67 @@ func init() {
 		BoolVar(&stackReorderFlags.Abort, "abort", false, "abort a previous reorder")
 	stackReorderCmd.MarkFlagsMutuallyExclusive("continue", "abort")
 }
+
+func stackReorderEditPlan(repo *git.Repo, initialPlan []reorder.Cmd) ([]reorder.Cmd, error) {
+	plan := initialPlan
+edit:
+	plan, err := reorder.EditPlan(repo, plan)
+	if err != nil {
+		return nil, err
+	}
+	if len(plan) == 0 {
+		_, _ = fmt.Fprint(os.Stderr,
+			colors.Failure("ERROR: reorder plan is empty\n"),
+		)
+		return nil, actions.ErrExitSilently{ExitCode: 127}
+	}
+
+	diff := reorder.Diff(initialPlan, plan)
+	if len(diff.RemovedBranches) > 0 {
+		_, _ = fmt.Fprint(
+			os.Stderr,
+			colors.Warning("\nWARNING: the following branches were removed from the reorder:\n"),
+		)
+		for _, branch := range diff.RemovedBranches {
+			_, _ = fmt.Fprint(os.Stderr, "  - ", colors.UserInput(branch), "\n")
+		}
+
+	promptDeletionBehavior:
+		_, _ = fmt.Fprint(os.Stderr, "\n",
+			`What would you like to do?
+    [a] Abort the reorder
+    [d] Delete the branches
+    [e] Edit the reorder plan
+    [o] Orphan the branches (the Git branch will continue to exist but will not
+        be tracked by av).
+
+[a/d/e/o]: `)
+		choice, err := bufio.NewReader(os.Stdin).ReadString('\n')
+		if err != nil {
+			return nil, err
+		}
+		var deleteRefs bool
+		// ReadString includes the newline in the string, so this should
+		// never panic even if the user just hits enter.
+		switch strings.ToLower(string(choice[0])) {
+		case "a":
+			_, _ = fmt.Fprint(os.Stderr, colors.Failure("\nAborting reorder.\n"))
+			return nil, actions.ErrExitSilently{ExitCode: 127}
+		case "d":
+			deleteRefs = true
+		case "e":
+			goto edit
+		case "o":
+			deleteRefs = false
+		default:
+			_, _ = fmt.Fprint(os.Stderr, colors.Failure("\nInvalid choice.\n"))
+			goto promptDeletionBehavior
+		}
+
+		for _, branch := range diff.RemovedBranches {
+			plan = append(plan, reorder.DeleteBranchCmd{Name: branch, DeleteGitRef: deleteRefs})
+		}
+	}
+
+	return plan, nil
+}

internal/git/git.go

@@ -102,9 +102,10 @@ type RunOpts struct {
 }
 
 type Output struct {
-	ExitCode int
-	Stdout   []byte
-	Stderr   []byte
+	ExitCode  int
+	ExitError *exec.ExitError
+	Stdout    []byte
+	Stderr    []byte
 }
 
 func (o Output) Lines() []string {
@@ -135,12 +136,17 @@ func (r *Repo) Run(opts *RunOpts) (*Output, error) {
 		return nil, errors.Wrapf(err, "git %s", opts.Args)
 	}
 	if err != nil && opts.ExitError && exitError.ExitCode() != 0 {
-		return nil, errors.Errorf("git %s: %s: %s", opts.Args, err, stderr.String())
+		// ExitError.Stderr is only populated if the command was started without
+		// a Stderr pipe, which is not the case here. Just populate it ourselves
+		// to make it easier for callers to access.
+		exitError.Stderr = stderr.Bytes()
+		return nil, errors.WrapIff(err, "git %s (%s)", opts.Args, stderr.String())
 	}
 	return &Output{
-		ExitCode: cmd.ProcessState.ExitCode(),
-		Stdout:   stdout.Bytes(),
-		Stderr:   stderr.Bytes(),
+		ExitCode:  cmd.ProcessState.ExitCode(),
+		ExitError: exitError,
+		Stdout:    stdout.Bytes(),
+		Stderr:    stderr.Bytes(),
 	}, nil
 }
 

internal/reorder/deletebranch.go

@@ -0,0 +1,95 @@
+package reorder
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/spf13/pflag"
+
+	"github.com/aviator-co/av/internal/git"
+	"github.com/aviator-co/av/internal/utils/colors"
+	"github.com/aviator-co/av/internal/utils/errutils"
+)
+
+// DeleteBranchCmd is a command that deletes a branch.
+// This is for internal use only to clean up branches that were removed from a
+// re-order operation.
+type DeleteBranchCmd struct {
+	Name string
+	// If true, delete the branch from Git as well as from the internal database.
+	// If false, only delete the branch metadata from the internal database.
+	DeleteGitRef bool
+}
+
+func (d DeleteBranchCmd) Execute(ctx *Context) error {
+	tx := ctx.DB.WriteTx()
+	tx.DeleteBranch(d.Name)
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	if !d.DeleteGitRef {
+		_, _ = fmt.Fprint(os.Stderr,
+			"Orphaned branch ", colors.UserInput(d.Name), ".\n",
+			"  - Run ", colors.CliCmd("git branch --delete ", d.Name),
+			" to delete the branch from your repository.\n",
+		)
+		return nil
+	}
+
+	_, err := ctx.Repo.Run(&git.RunOpts{
+		Args:      []string{"branch", "--delete", "--force", d.Name},
+		ExitError: true,
+	})
+	if exiterr, ok := errutils.As[*exec.ExitError](err); ok &&
+		strings.Contains(string(exiterr.Stderr), "not found") {
+		_, _ = fmt.Fprint(os.Stderr,
+			colors.Warning("Branch "), colors.UserInput(d.Name),
+			colors.Warning(" was already deleted.\n"),
+		)
+		return nil
+	} else if err != nil {
+		return err
+	}
+
+	_, _ = fmt.Fprint(os.Stderr,
+		"Deleted branch ", colors.UserInput(d.Name), ".\n",
+	)
+	return nil
+}
+
+func (d DeleteBranchCmd) String() string {
+	sb := strings.Builder{}
+	sb.WriteString("delete-branch ")
+	sb.WriteString(d.Name)
+	if d.DeleteGitRef {
+		sb.WriteString(" --delete-git-ref")
+	}
+	return sb.String()
+}
+
+var _ Cmd = DeleteBranchCmd{}
+
+func parseDeleteBranchCmd(args []string) (Cmd, error) {
+	cmd := DeleteBranchCmd{}
+	flags := pflag.NewFlagSet("delete-branch", pflag.ContinueOnError)
+	flags.BoolVar(
+		&cmd.DeleteGitRef,
+		"delete-git-ref",
+		false,
+		"delete the branch from Git as well as from the internal database",
+	)
+	if err := flags.Parse(args); err != nil {
+		return nil, err
+	}
+	if flags.NArg() != 1 {
+		return nil, ErrInvalidCmd{
+			"delete-branch",
+			"exactly one argument is required (the name of the branch to delete)",
+		}
+	}
+	cmd.Name = flags.Arg(0)
+	return cmd, nil
+}

internal/reorder/deletebranch_test.go

@@ -0,0 +1,73 @@
+package reorder
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/aviator-co/av/internal/git"
+	"github.com/aviator-co/av/internal/git/gittest"
+	"github.com/aviator-co/av/internal/meta/jsonfiledb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDeleteBranchCmd_String(t *testing.T) {
+	assert.Equal(t, "delete-branch my-branch", DeleteBranchCmd{Name: "my-branch"}.String())
+	assert.Equal(
+		t,
+		"delete-branch my-branch --delete-git-ref",
+		DeleteBranchCmd{Name: "my-branch", DeleteGitRef: true}.String(),
+	)
+}
+
+func TestDeleteBranchCmd_Execute(t *testing.T) {
+	repo := gittest.NewTempRepo(t)
+	db, err := jsonfiledb.OpenRepo(repo)
+	require.NoError(t, err)
+	out := &bytes.Buffer{}
+	ctx := &Context{repo, db, &State{Branch: "main"}, out}
+
+	start, err := repo.RevParse(&git.RevParse{Rev: "HEAD"})
+	require.NoError(t, err)
+
+	_, err = repo.Git("switch", "-c", "my-branch")
+	require.NoError(t, err)
+	gittest.CommitFile(t, repo, "file", []byte("hello\n"))
+
+	// Need to switch back to main so that the branch ref can be deleted.
+	// This shouldn't be an issue in the actual reorder because we'll never be
+	// checked out on branches that we're about to delete (unless the user does
+	// weird things™).
+	_, err = repo.Git("switch", "main")
+	require.NoError(t, err)
+
+	// delete-branch without --delete-git-ref should preserve the branch ref in Git
+	err = DeleteBranchCmd{Name: "my-branch"}.Execute(ctx)
+	require.NoError(t, err)
+	_, err = repo.RevParse(&git.RevParse{Rev: "my-branch"})
+	require.NoError(t, err, "DeleteBranchCmd.Execute should preserve the branch ref in Git")
+
+	// delete-branch with --delete-git-ref should delete the branch ref in Git
+	err = DeleteBranchCmd{Name: "my-branch", DeleteGitRef: true}.Execute(ctx)
+	require.NoError(t, err)
+	_, err = repo.RevParse(&git.RevParse{Rev: "my-branch"})
+	require.Error(t, err, "DeleteBranchCmd.Execute should delete the branch ref in Git")
+
+	// subsequent delete-branch should be a no-op
+	err = DeleteBranchCmd{Name: "my-branch", DeleteGitRef: true}.Execute(ctx)
+	require.NoError(
+		t,
+		err,
+		"DeleteBranchCmd.Execute should be a no-op if the branch ref is already deleted",
+	)
+
+	// HEAD of original branch should be preserved
+	head, err := repo.RevParse(&git.RevParse{Rev: "HEAD"})
+	require.NoError(t, err)
+	require.Equal(
+		t,
+		start,
+		head,
+		"DeleteBranchCmd.Execute should preserve the HEAD of the original branch",
+	)
+}

internal/reorder/editor.go

@@ -3,6 +3,8 @@ package reorder
 import (
 	"strings"
 
+	"github.com/aviator-co/av/internal/utils/sliceutils"
+
 	"github.com/aviator-co/av/internal/editor"
 	"github.com/aviator-co/av/internal/git"
 	"github.com/aviator-co/av/internal/utils/typeutils"
@@ -47,3 +49,30 @@ func EditPlan(repo *git.Repo, plan []Cmd) ([]Cmd, error) {
 
 	return newPlan, nil
 }
+
+type PlanDiff struct {
+	RemovedBranches []string
+	AddedBranches   []string
+}
+
+func Diff(old []Cmd, new []Cmd) PlanDiff {
+
+	var oldBranches []string
+	for _, cmd := range old {
+		if sb, ok := cmd.(StackBranchCmd); ok {
+			oldBranches = append(oldBranches, sb.Name)
+		}
+	}
+
+	var newBranches []string
+	for _, cmd := range new {
+		if sb, ok := cmd.(StackBranchCmd); ok {
+			newBranches = append(newBranches, sb.Name)
+		}
+	}
+
+	return PlanDiff{
+		RemovedBranches: sliceutils.Subtract(oldBranches, newBranches),
+		AddedBranches:   sliceutils.Subtract(newBranches, oldBranches),
+	}
+}

internal/reorder/parse.go

@@ -18,10 +18,12 @@ func ParseCmd(line string) (Cmd, error) {
 	cmdName := args[0]
 	args = args[1:]
 	switch cmdName {
-	case "stack-branch", "sb":
-		return parseStackBranchCmd(args)
+	case "delete-branch", "db":
+		return parseDeleteBranchCmd(args)
 	case "pick", "p":
 		return parsePickCmd(args)
+	case "stack-branch", "sb":
+		return parseStackBranchCmd(args)
 	default:
 		return nil, errors.Errorf("unknown reorder command %q", cmdName)
 	}

internal/reorder/parse_test.go

@@ -19,6 +19,11 @@ func TestParseCmd(t *testing.T) {
 		{"pick", PickCmd{}, true},
 		{"pick foo", PickCmd{Commit: "foo"}, false},
 		{"pick foo bar", PickCmd{}, true},
+		{"delete-branch", DeleteBranchCmd{}, true},
+		{"delete-branch foo", DeleteBranchCmd{Name: "foo"}, false},
+		{"delete-branch foo bar", DeleteBranchCmd{}, true},
+		{"db foo --delete-git-ref", DeleteBranchCmd{Name: "foo", DeleteGitRef: true}, false},
+		{"blarn", nil, true},
 	} {
 		t.Run(tt.Input, func(t *testing.T) {
 			cmd, err := ParseCmd(tt.Input)