fix: Repo.Diff shouldn't fail given ambiguous branch names

cmd/av/stack_tree.go

@@ -149,9 +149,10 @@ 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, Specifiers: []string{branch.Name, upstreamBranch}},
-	)
+	upstreamDiff, err := repo.Diff(&git.DiffOpts{
+		Quiet:      true,
+		Specifiers: []string{branch.Name, upstreamBranch},
+	})
 	if err != nil {
 		return "", err
 	}

e2e_tests/stack_tree_test.go

@@ -0,0 +1,24 @@
+package e2e_tests
+
+import (
+	"testing"
+
+	"github.com/aviator-co/av/internal/git/gittest"
+)
+
+func TestStackTree(t *testing.T) {
+	repo := gittest.NewTempRepo(t)
+	Chdir(t, repo.Dir())
+
+	RequireAv(t, "stack", "branch", "foo")
+	gittest.CommitFile(t, repo, "foo", []byte("foo"))
+
+	RequireAv(t, "stack", "branch", "bar")
+	gittest.CommitFile(t, repo, "bar", []byte("bar"))
+
+	gittest.CheckoutBranch(t, repo, "main")
+	RequireAv(t, "stack", "branch", "spam")
+	gittest.CommitFile(t, repo, "spam", []byte("spam"))
+
+	RequireAv(t, "stack", "tree")
+}

internal/git/diff.go

@@ -38,12 +38,15 @@ func (r *Repo) Diff(d *DiffOpts) (*Diff, error) {
 	}
 
 	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...)
-	}
+	// Note that we still append this `--` even if there are no paths because
+	// otherwise Git might interpret a specifier as ambiguous path and raise an
+	// error.
+	args = append(args, "--")
+	args = append(args, d.Paths...)
+
 	output, err := r.Run(&RunOpts{
 		Args: args,
 	})

internal/git/diff_test.go

@@ -0,0 +1,28 @@
+package git_test
+
+import (
+	"testing"
+
+	"github.com/aviator-co/av/internal/git"
+	"github.com/aviator-co/av/internal/git/gittest"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRepoDiffAmbiguousPathName(t *testing.T) {
+	repo := gittest.NewTempRepo(t)
+
+	_, err := repo.CheckoutBranch(&git.CheckoutBranch{Name: "foo", NewBranch: true})
+	require.NoError(t, err, "repo.CheckoutBranch should not error given a valid branch name")
+
+	gittest.CommitFile(t, repo, "foo", []byte("foo"))
+	diff, err := repo.Diff(&git.DiffOpts{
+		Quiet:      true,
+		Specifiers: []string{"main", "foo"},
+	})
+	require.NoError(t, err, "repo.Diff should not error given an ambiguous branch/path name")
+	require.False(
+		t,
+		diff.Empty,
+		"diff between branches with different trees should return non-empty",
+	)
+}