fix(stack diff): Compute stack diff correctly (#159)

This was originally gonna just be running the diff in interactive mode to enable the pager but I figured out the way were computing the diff wasn't quite right. I describe the complexities in comments in `stack_diff.go`.

# Test plan

I have branches like
```
* 2023-06-05-one-b 2d1f535  (2023-06-05-one)
|   
| * 2023-06-05-two-b 001f28f  (HEAD -> 2023-06-05-two)
|/  
| 
* 2023-06-05-one-a aba0167 
```

Before this change, running `av stack diff` on the second branch includes removing the changes from the second commit of branch one.

```diff
$ av stack diff
diff --git a/2023-06-05 b/2023-06-05
index aee8822..6dc6612 100644
--- a/2023-06-05
+++ b/2023-06-05
@@ -1,2 +1,2 @@
 Mon Jun  5 15:02:42 PDT 2023
-Mon Jun  5 15:05:50 PDT 2023
+Mon Jun  5 15:05:04 PDT 2023
```

After this change, it shows just the diff from the second branch commits and also a warning that the branch is not up to date.

```diff
$ av stack diff
diff --git a/2023-06-05 b/2023-06-05
index 5982c55..6dc6612 100644
--- a/2023-06-05
+++ b/2023-06-05
@@ -1 +1,2 @@
 Mon Jun  5 15:02:42 PDT 2023
+Mon Jun  5 15:05:04 PDT 2023

WARNING: Branch 2023-06-05-two is not up to date with parent branch 2023-06-05-one. Run av stack sync to synchronize the branch.
```

cmd/av/stack_diff.go

@@ -5,8 +5,11 @@ import (
 	"os"
 	"strings"
 
+	"github.com/aviator-co/av/internal/actions"
+	"github.com/aviator-co/av/internal/meta"
+	"github.com/aviator-co/av/internal/utils/colors"
+
 	"github.com/aviator-co/av/internal/git"
-	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 )
 
@@ -34,17 +37,90 @@ Generates the diff between the working tree and the parent branch
 		}
 
 		tx := db.ReadTx()
-		branch, _ := tx.Branch(currentBranchName)
+		branch, exists := tx.Branch(currentBranchName)
+		if !exists {
+			defaultBranch, err := repo.DefaultBranch()
+			if err != nil {
+				return err
+			}
+			branch.Parent = meta.BranchState{
+				Name:  defaultBranch,
+				Trunk: true,
+			}
+		}
+
+		diffArgs := []string{"diff"}
+		notUpToDate := false
+
+		if branch.Parent.Trunk {
+			// Compare against the merge-base so that we effectively only see the
+			// diff associated with this branch. Without this, if main has
+			// advanced since this branch was created, we'd also see the (inverse)
+			// diff between main and this branch. For example, if we have:
+			//     main: X -> Y
+			//     one:   \-> 1a -> 1b
+			// without --merge-base, we compute the diff between Y and 1b, which
+			// shows that we're undoing the changes that were introduced in Y
+			// (since 1b doesn't have those changes). With --merge-base, we
+			// compute the diff relative to X, which is probably what the user
+			// expects.
+			// This roughly matches the diff that GitHub will show in the pull
+			// request files changed view.
+			diffArgs = append(diffArgs, "--merge-base", branch.Parent.Name)
+		} else {
+			// For a non-root branch, we compare against the original branch point.
+			// We don't want to compare exactly against the parent branch since
+			// the parent branch might have been modified but not yet synced.
+			// For example, if we have:
+			//     main: X -> Y
+			//     one:   \-> 1a
+			//     two:         \-> 2a
+			// and then one is updated (either by adding a new commit or amending),
+			// we still only want to show the diff associated with two as {2a}.
+			// Concretely, if we have
+			//     main: X -> Y
+			//     one:   \-> 1a -> 1b
+			//     two:         \-> 2a
+			// we don't want to compute the diff between 1b and 2a since that
+			// will show the diff between 1b and 2a, which shows that we're
+			// undoing the changes that were introduced in 1b (since 2a doesn't
+			// have those changes). Instead, we want to compute the diff between
+			// 1a and 2a. We can't just use merge-base here to account for the
+			// fact that one might be amended (not just advanced).
+			diffArgs = append(diffArgs, branch.Parent.Head)
 
-		diff, err := repo.Diff(&git.DiffOpts{
-			Color:  !color.NoColor,
-			Commit: branch.Parent.Name,
+			// Determine if the branch is up-to-date with the parent and warn if
+			// not.
+			currentParentHead, err := repo.RevParse(&git.RevParse{Rev: branch.Parent.Name})
+			if err != nil {
+				return err
+			}
+			notUpToDate = currentParentHead != branch.Parent.Head
+		}
+
+		// NOTE:
+		// We don't use repo.Diff here since that sets the --exit-error flag
+		// which in turn disables the output pager. We want this command to
+		// behave similarly to default `git diff` for the user.
+		_, err = repo.Run(&git.RunOpts{
+			Args:        diffArgs,
+			Interactive: true,
 		})
 		if err != nil {
 			return err
 		}
 
-		_, _ = fmt.Fprint(os.Stderr, diff.Contents)
+		// We need to display this **after** the diff to ensure that the diff
+		// output pager doesn't eat this message.
+		if notUpToDate {
+			_, _ = fmt.Fprint(os.Stderr,
+				colors.Warning("\nWARNING: Branch "), colors.UserInput(currentBranchName),
+				colors.Warning(" is not up to date with parent branch "),
+				colors.UserInput(branch.Parent.Name), colors.Warning(". Run "),
+				colors.CliCmd("av stack sync"), colors.Warning(" to synchronize the branch.\n"),
+			)
+			return actions.ErrExitSilently{ExitCode: 1}
+		}
 
 		return nil
 	},

cmd/av/stack_tree.go

@@ -150,7 +150,7 @@ func getUpstreamStatus(repo *git.Repo, branch meta.Branch) (string, error) {
 
 	upstreamBranch := fmt.Sprintf("remotes/origin/%s", branch.Name)
 	upstreamDiff, err := repo.Diff(
-		&git.DiffOpts{Quiet: true, Branch1: branch.Name, Branch2: upstreamBranch},
+		&git.DiffOpts{Quiet: true, Specifiers: []string{branch.Name, upstreamBranch}},
 	)
 	if err != nil {
 		return "", err

internal/actions/reparent.go

@@ -61,7 +61,7 @@ func Reparent(
 		"\n",
 	)
 
-	diff, err := repo.Diff(&git.DiffOpts{Commit: "HEAD", Quiet: true})
+	diff, err := repo.Diff(&git.DiffOpts{Specifiers: []string{"HEAD"}, Quiet: true})
 	if err != nil {
 		return nil, err
 	}

internal/git/diff.go

@@ -1,25 +1,23 @@
 package git
 
 import (
-	"os/exec"
-
 	"emperror.dev/errors"
 )
 
 type DiffOpts struct {
-	// If specified, generate the diff between the working tree and this commit.
-	// If empty (default), generates the diff between the working tree and the
-	// current index (i.e., the diff containing all unstaged changes).
-	Commit string
+	// The revisions to compare.
+	// The behavior of the diff changes depending on how these are specified.
+	//   - If empty, the generated diff is relative to the current staging area.
+	//   - If one commit is given, the diff is calculated between the working tree
+	//     and the given commit.
+	//   - If two commits are given (or one string representing a commit range
+	//     like `<a>..<b>`), the diff is calculated between the two commits.
+	Specifiers []string
 	// If true, don't actually generate the diff, just return whether or not its
 	// empty. If set, Diff.Contents will always be an empty string.
 	Quiet bool
 	// If true, shows the colored diff.
 	Color bool
-	// Both branches need to be specified in order to find the diff between the two branches.
-	// If a Commit is specified, the branches will not be used.
-	Branch1 string
-	Branch2 string
 	// If specified, compare only the specified paths.
 	Paths []string
 }
@@ -35,29 +33,27 @@ func (r *Repo) Diff(d *DiffOpts) (*Diff, error) {
 	if d.Quiet {
 		args = append(args, "--quiet")
 	}
-	if d.Commit != "" {
-		args = append(args, d.Commit)
-	} else if d.Branch1 != "" && d.Branch2 != "" {
-		args = append(args, d.Branch1)
-		args = append(args, d.Branch2)
-	}
-
 	if d.Color {
 		args = append(args, "--color=always")
 	}
 
+	args = append(args, d.Specifiers...)
 	// This needs to be last because everything after the `--` is interpreted
 	// as a path, not a flag.
 	if len(d.Paths) > 0 {
 		args = append(args, "--")
 		args = append(args, d.Paths...)
 	}
-	contents, err := r.Git(args...)
-	var exitError *exec.ExitError
-	if errors.As(err, &exitError) && exitError.ExitCode() == 1 {
-		return &Diff{Empty: false, Contents: contents}, nil
-	} else if err != nil {
+	output, err := r.Run(&RunOpts{
+		Args: args,
+	})
+	if err != nil {
 		return nil, err
 	}
-	return &Diff{Empty: true, Contents: contents}, nil
+	if output.ExitCode == 1 {
+		return &Diff{Empty: false, Contents: string(output.Stdout)}, nil
+	} else if output.ExitCode != 0 {
+		return nil, errors.Errorf("git diff failed: %s", string(output.Stderr))
+	}
+	return &Diff{Empty: true, Contents: string(output.Stdout)}, nil
 }