Skip to content

refactor(diff): normalize git argument ordering in workspaceTrackedDiff #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chethanuk wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −1
Open

refactor(diff): normalize git argument ordering in workspaceTrackedDiff #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion internal/diff/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,18 @@ func (p *Provider) computeMergeBase(ctx context.Context, from, to string) string
}

func (p *Provider) workspaceTrackedDiff(ctx context.Context) (string, error) {
out, err := p.runGit(ctx, "-c", "core.quotepath=false", "diff", "--no-ext-diff", "--no-textconv", "--find-renames", "--src-prefix=a/", "--dst-prefix=b/", "--no-color", "-U"+fmt.Sprint(DiffContextLines), "HEAD", "--")
out, err := p.runGit(ctx, "-c", "core.quotepath=false", "diff", "--no-ext-diff", "--no-textconv", "--find-renames", "--src-prefix=a/", "--dst-prefix=b/", "--no-color", "-U"+fmt.Sprint(DiffContextLines), "--end-of-options", "HEAD", "--")
if err == nil && out != "" {
return out, nil
}
if ctx.Err() != nil {
return "", ctx.Err()
}
// Fall back to the staged diff when `git diff HEAD` errored or was empty. This is
// not redundant with the call above: in a repository with no commits yet there is no
// HEAD, so `git diff HEAD` fails with "bad revision 'HEAD'", but `git diff --staged`
// still surfaces staged changes by diffing the index against the empty tree — the only
// way to review a workspace before its first commit.
return p.runGit(ctx, "-c", "core.quotepath=false", "diff", "--no-ext-diff", "--no-textconv", "--find-renames", "--src-prefix=a/", "--dst-prefix=b/", "--no-color", "-U"+fmt.Sprint(DiffContextLines), "--staged", "--")
}

Expand Down
30 changes: 30 additions & 0 deletions internal/diff/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,36 @@ func TestWorkspaceDiffSurvivesExternalDiffTool(t *testing.T) {
}
}

// TestWorkspaceDiffNoCommitsUsesStagedFallback pins the second runGit call in
// workspaceTrackedDiff: in a repository with no commits there is no HEAD, so
// `git diff HEAD` fails, and the staged diff is the only way to review the
// workspace. Removing the `--staged` fallback (as an over-eager "simplification"
// might) makes this return zero diffs.
func TestWorkspaceDiffNoCommitsUsesStagedFallback(t *testing.T) {
repo := t.TempDir()
runGitTest(t, repo, "init", "-q")

// Staged file, no commit yet -> no HEAD. No commit also means no need for
// the user.email/user.name/commit.gpgsign config the committing tests set.
file := filepath.Join(repo, "staged.txt")
if err := os.WriteFile(file, []byte("alpha\nbeta\n"), 0o644); err != nil {
t.Fatalf("write staged.txt: %v", err)
}
runGitTest(t, repo, "add", "staged.txt")

runner := gitcmd.New(0)
provider := NewWorkspaceProvider(repo, runner)

diffs, err := provider.GetDiff(context.Background())
if err != nil {
t.Fatalf("GetDiff returned error: %v", err)
}
if len(diffs) == 0 {
t.Fatalf("expected staged changes to be surfaced in a repo with no commits " +
"(the --staged fallback in workspaceTrackedDiff), got 0 diffs")
}
}

// TestCommitDiffSurvivesExternalDiffTool covers the ModeCommit call site
// (git show <commit>), which likewise must pass --no-ext-diff so that a
// user's external diff tool does not break single-commit analysis.
Expand Down