From 58d078c7f7b2c530614b6eb0f1e13013ace018da Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Wed, 15 Jul 2026 08:46:50 +1000 Subject: [PATCH] feat(diff): add explicit endpoint modes Add first-class --head and --index comparisons while preserving the existing worktree default and raw Git passthrough forms. --- README.md | 15 ++++++- integration/lifecycle_test.go | 35 ++++++++++++++++ internal/cli/cli.go | 4 ++ internal/cli/cli_test.go | 12 ++++-- internal/cli/schema.go | 3 +- internal/command/diff.go | 29 ++++++++++--- internal/command/diff_test.go | 78 +++++++++++++++++++++++++++++------ internal/command/status.go | 2 +- 8 files changed, 154 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index c2dbc69..27e40e3 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,8 @@ braid diff braid diff --sync-push-only braid diff vendor/rails braid diff :rails +braid diff vendor/rails --head +braid diff vendor/rails --index braid diff vendor/rails -- --stat braid diff vendor/rails -- --cached ``` @@ -315,8 +317,19 @@ every mirror belonging to the source. explicitly selected. The option filters sources only; each selected mirror retains the normal `braid diff` comparison behavior. +By default, Braid compares the recorded upstream base with the working tree, so +the diff includes tracked changes whether they are committed, staged, or +unstaged. As with ordinary `git diff`, untracked files are omitted. Use +`--index` to compare the recorded upstream base with the index, or `--head` to +compare it with downstream `HEAD`. The `--head` endpoint is the committed mirror +content read by `braid push` and sync's push phase, but the diff is not a push +preview and does not check push eligibility or preconditions. + Arguments after `--` are passed to `git diff`. This is useful for generating -patches, limiting output, or checking staged changes only. +patches or limiting output. The existing raw forms `-- --cached` and `-- HEAD` +are equivalent to `--index` and `--head`, respectively. When using a first-class +endpoint option, passthrough arguments must not select another Git revision or +endpoint mode. ### Pulling Mirrors diff --git a/integration/lifecycle_test.go b/integration/lifecycle_test.go index 27ba2ec..277bc97 100644 --- a/integration/lifecycle_test.go +++ b/integration/lifecycle_test.go @@ -148,6 +148,41 @@ func TestExecutableDiffSyncPushOnly(t *testing.T) { assertResult(t, skipped, 0, "", "") } +func TestExecutableDiffEndpoints(t *testing.T) { + root := t.TempDir() + env := newProcessEnv(t, root) + braid := braidBinary(t) + + upstream := filepath.Join(root, "upstream") + initRepo(t, env, upstream) + writeFile(t, upstream, "committed.txt", "base committed\n") + writeFile(t, upstream, "staged.txt", "base staged\n") + writeFile(t, upstream, "unstaged.txt", "base unstaged\n") + commitAll(t, env, upstream, "upstream") + + downstream := filepath.Join(root, "downstream") + initRepo(t, env, downstream) + writeFile(t, downstream, "README.md", "downstream\n") + commitAll(t, env, downstream, "downstream") + assertResult(t, runBraid(t, env, downstream, braid, "--quiet", "add", upstream, "vendor/basic"), 0, "", "") + + writeFile(t, downstream, "vendor/basic/committed.txt", "changed committed\n") + gitOK(t, env, downstream, "add", "vendor/basic/committed.txt") + gitOK(t, env, downstream, "commit", "-m", "committed mirror change") + writeFile(t, downstream, "vendor/basic/staged.txt", "changed staged\n") + gitOK(t, env, downstream, "add", "vendor/basic/staged.txt") + writeFile(t, downstream, "vendor/basic/unstaged.txt", "changed unstaged\n") + + worktree := runBraid(t, env, downstream, braid, "diff", "vendor/basic", "--", "--name-only") + assertResult(t, worktree, 0, "committed.txt\nstaged.txt\nunstaged.txt\n", "") + + index := runBraid(t, env, downstream, braid, "diff", "vendor/basic", "--index", "--", "--name-only") + assertResult(t, index, 0, "committed.txt\nstaged.txt\n", "") + + head := runBraid(t, env, downstream, braid, "diff", "vendor/basic", "--head", "--", "--name-only") + assertResult(t, head, 0, "committed.txt\n", "") +} + func TestUpgradeConfigCommitAndNoCommit(t *testing.T) { for _, noCommit := range []bool{false, true} { t.Run(map[bool]string{false: "commit", true: "no-commit"}[noCommit], func(t *testing.T) { diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 0ac8bf6..6839a4d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -71,6 +71,8 @@ type DiffOptions struct { LocalPath string Keep bool SyncPushOnly bool + Head bool + Index bool GitDiffArgs []string } @@ -425,6 +427,8 @@ func parseDiff(args []string, options *DiffOptions) error { } options.Keep = parsed.has("--keep") options.SyncPushOnly = parsed.has("--sync-push-only") + options.Head = parsed.has("--head") + options.Index = parsed.has("--index") options.GitDiffArgs = parsed.passthrough return nil } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 27d47a7..3b006f7 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -72,13 +72,18 @@ func TestParseCommands(t *testing.T) { }, { name: "diff passthrough", - args: []string{"--no-cache", "--quiet", "diff", "vendor/repo", "--sync-push-only", "--", "--stat", "weird;path"}, + args: []string{"--no-cache", "--quiet", "diff", "vendor/repo", "--sync-push-only", "--head", "--", "--stat", "weird;path"}, want: Invocation{ Global: GlobalOptions{NoCache: true, Quiet: true}, Command: CommandDiff, - Diff: DiffOptions{LocalPath: "vendor/repo", SyncPushOnly: true, GitDiffArgs: []string{"--stat", "weird;path"}}, + Diff: DiffOptions{LocalPath: "vendor/repo", SyncPushOnly: true, Head: true, GitDiffArgs: []string{"--stat", "weird;path"}}, }, }, + { + name: "diff index before selector", + args: []string{"diff", "--index", "vendor/repo"}, + want: Invocation{Command: CommandDiff, Diff: DiffOptions{LocalPath: "vendor/repo", Index: true}}, + }, { name: "diff sync push only before selector", args: []string{"diff", "--sync-push-only", "vendor/repo"}, @@ -207,6 +212,7 @@ func TestParseUsageErrors(t *testing.T) { {name: "existing source sync push", args: []string{"add", ":source", "vendor/new", "--sync-push"}, want: "add to an existing source cannot use --name, --branch, --tag, --revision, --partial-clone, or --sync-push"}, {name: "pull all strategy flag", args: []string{"pull", "--branch", "main"}, want: "pull without local_path cannot use --branch, --tag, or --revision"}, {name: "diff args require separator", args: []string{"diff", "--stat"}, want: "unknown flag for diff: --stat"}, + {name: "diff endpoint conflict", args: []string{"diff", "--head", "--index"}, want: "diff cannot combine --head and --index"}, {name: "sync unknown flag", args: []string{"sync", "--branch", "main"}, want: "unknown flag for sync: --branch"}, {name: "sync no commit unsupported", args: []string{"sync", "--no-commit"}, want: "unknown flag for sync: --no-commit"}, {name: "push whitespace message", args: []string{"push", "vendor/repo", "--message", " \t"}, want: "--message requires a non-empty message value"}, @@ -349,7 +355,7 @@ func TestUsageDocumentsVerboseAsGlobalOnly(t *testing.T) { if got, want := CommandUsage(CommandRemove), "usage: braid remove [--keep] [--no-commit]\n"; got != want { t.Fatalf("CommandUsage(remove) = %q, want %q", got, want) } - if got, want := CommandUsage(CommandDiff), "usage: braid diff [local_path|:source] [--keep] [--sync-push-only] [-- ...]\n"; got != want { + if got, want := CommandUsage(CommandDiff), "usage: braid diff [local_path|:source] [--keep] [--sync-push-only] [--head] [--index] [-- ...]\n"; got != want { t.Fatalf("CommandUsage(diff) = %q, want %q", got, want) } if got, want := CommandUsage(CommandPush), "usage: braid push [--branch|-b ] [--message|-m ] [--keep]\n"; got != want { diff --git a/internal/cli/schema.go b/internal/cli/schema.go index a29adcf..62f4ebb 100644 --- a/internal/cli/schema.go +++ b/internal/cli/schema.go @@ -181,9 +181,10 @@ var commandSpecs = []CommandSpec{ }, { Command: CommandDiff, Name: "diff", Summary: "Show local mirror changes", - Options: []OptionSpec{{Long: "--keep"}, {Long: "--sync-push-only"}}, + Options: []OptionSpec{{Long: "--keep"}, {Long: "--sync-push-only"}, {Long: "--head"}, {Long: "--index"}}, Positionals: []PositionalSpec{{Name: "local_path|:source", Usage: "[local_path|:source]", Completion: CompletionMirror}}, Passthrough: &PassthroughSpec{Usage: "[-- ...]"}, + Conflicts: []ConflictSpec{{Options: []string{"--head", "--index"}, Error: "diff cannot combine --head and --index"}}, }, { Command: CommandPush, Name: "push", Summary: "Push one source's local mirror changes upstream", diff --git a/internal/command/diff.go b/internal/command/diff.go index 99f65e9..2088371 100644 --- a/internal/command/diff.go +++ b/internal/command/diff.go @@ -25,6 +25,11 @@ func (h DiffHandler) Run(inv cli.Invocation, stdout, stderr io.Writer) error { git := h.diffGit(repo, inv, stderr) processGit := h.processDiffGit(repo, inv, stderr) + if inv.Diff.Head { + if _, err := git.RevParse(ctx, "HEAD^{commit}"); err != nil { + return fmt.Errorf("diff --head requires a downstream HEAD commit") + } + } progress := newProgressReporter(stderr, inv.Global.Quiet) cfg, err := config.Load(configRoot(h.Options, repo)) if err != nil { @@ -111,7 +116,7 @@ func (h DiffHandler) diffOne(ctx context.Context, git, processGit DiffGit, cache if err := fetchBaseRevisionIfMissing(ctx, git, cache, m, verbose, progress, trace); err != nil { return err } - args, err := buildDiffArgs(ctx, git, m, options.GitDiffArgs) + args, err := buildDiffArgs(ctx, git, m, options) if err != nil { return err } @@ -135,7 +140,7 @@ func fetchBaseRevisionIfMissing(ctx context.Context, git DiffGit, cache CacheCon return fetchMirror(ctx, git, cache, m, progress) } -func buildDiffArgs(ctx context.Context, git DiffGit, m source.SourceMirror, userArgs []string) ([]string, error) { +func buildDiffArgs(ctx context.Context, git DiffGit, m source.SourceMirror, options cli.DiffOptions) ([]string, error) { item, err := baseDiffItem(ctx, git, m) if err != nil { return nil, err @@ -150,14 +155,26 @@ func buildDiffArgs(ctx context.Context, git DiffGit, m source.SourceMirror, user "--relative=" + m.LocalPath, "--src-prefix=a/" + path.Base(m.UpstreamPath), "--dst-prefix=b/" + path.Base(m.LocalPath), - baseTree, } - args = append(args, userArgs...) + args = appendDiffEndpoint(args, baseTree, options) + args = append(args, options.GitDiffArgs...) return append(args, ":(top)"+m.LocalPath), nil } - args := []string{"--relative=" + m.LocalPath + "/", baseTree} - return append(args, userArgs...), nil + args := []string{"--relative=" + m.LocalPath + "/"} + args = appendDiffEndpoint(args, baseTree, options) + return append(args, options.GitDiffArgs...), nil +} + +func appendDiffEndpoint(args []string, baseTree string, options cli.DiffOptions) []string { + if options.Index { + args = append(args, "--cached") + } + args = append(args, baseTree) + if options.Head { + args = append(args, "HEAD") + } + return args } func baseDiffItem(ctx context.Context, git DiffGit, m source.SourceMirror) (gitexec.TreeItem, error) { diff --git a/internal/command/diff_test.go b/internal/command/diff_test.go index 30a2481..f22519f 100644 --- a/internal/command/diff_test.go +++ b/internal/command/diff_test.go @@ -10,33 +10,73 @@ import ( "braid/internal/testutil" ) -func TestDiffCommandShowsStagedUnstagedReverseAndPathLimited(t *testing.T) { +func TestDiffCommandEndpointModesReverseAndPathLimited(t *testing.T) { upstream := testutil.InitRepo(t) testutil.WriteFile(t, upstream, "a.txt", "base a\n") testutil.WriteFile(t, upstream, "b.txt", "base b\n") + testutil.WriteFile(t, upstream, "c.txt", "base c\n") testutil.CommitAll(t, upstream, "upstream") repo := initDownstream(t) runCommandOK(t, repo, []string{"add", upstream, "vendor/basic"}) - testutil.WriteFile(t, repo, "vendor/basic/a.txt", "staged a\n") + testutil.WriteFile(t, repo, "vendor/basic/a.txt", "committed a\n") testutil.Git(t, repo, "add", "vendor/basic/a.txt") - testutil.WriteFile(t, repo, "vendor/basic/b.txt", "unstaged b\n") + testutil.Git(t, repo, "commit", "-m", "committed mirror change") + testutil.WriteFile(t, repo, "vendor/basic/b.txt", "staged b\n") + testutil.Git(t, repo, "add", "vendor/basic/b.txt") + testutil.WriteFile(t, repo, "vendor/basic/c.txt", "unstaged c\n") allDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic"}) assertContains(t, allDiff, "diff --git a/a.txt b/a.txt") assertContains(t, allDiff, "diff --git a/b.txt b/b.txt") + assertContains(t, allDiff, "diff --git a/c.txt b/c.txt") + + headDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--head"}) + assertContains(t, headDiff, "diff --git a/a.txt b/a.txt") + assertNotContains(t, headDiff, "diff --git a/b.txt b/b.txt") + assertNotContains(t, headDiff, "diff --git a/c.txt b/c.txt") + + rawHeadDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--", "HEAD"}) + if rawHeadDiff != headDiff { + t.Fatalf("raw HEAD diff differs from --head diff:\n--head:\n%s\nraw HEAD:\n%s", headDiff, rawHeadDiff) + } + + indexDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--index"}) + assertContains(t, indexDiff, "diff --git a/a.txt b/a.txt") + assertContains(t, indexDiff, "diff --git a/b.txt b/b.txt") + assertNotContains(t, indexDiff, "diff --git a/c.txt b/c.txt") cachedDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--", "--cached"}) - assertContains(t, cachedDiff, "diff --git a/a.txt b/a.txt") - assertNotContains(t, cachedDiff, "diff --git a/b.txt b/b.txt") + if cachedDiff != indexDiff { + t.Fatalf("raw --cached diff differs from --index diff:\n--index:\n%s\nraw --cached:\n%s", indexDiff, cachedDiff) + } reverseDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--", "-R", "--cached"}) assertContains(t, reverseDiff, "diff --git b/a.txt a/a.txt") - limitedDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--", "vendor/basic/b.txt"}) - assertContains(t, limitedDiff, "diff --git a/b.txt b/b.txt") + limitedDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--", "vendor/basic/c.txt"}) + assertContains(t, limitedDiff, "diff --git a/c.txt b/c.txt") assertNotContains(t, limitedDiff, "diff --git a/a.txt b/a.txt") + assertNotContains(t, limitedDiff, "diff --git a/b.txt b/b.txt") + + limitedHeadDiff := runCommandOK(t, repo, []string{"diff", "vendor/basic", "--head", "--", "--stat"}) + assertContains(t, limitedHeadDiff, "a.txt") + assertNotContains(t, limitedHeadDiff, "b.txt") + assertNotContains(t, limitedHeadDiff, "c.txt") +} + +func TestDiffCommandHeadRequiresDownstreamCommit(t *testing.T) { + repo := testutil.InitRepo(t) + testutil.WriteFile(t, repo, ".braids.json", "{\"config_version\":2,\"sources\":{}}\n") + + stderr := runCommandError(t, repo, []string{"diff", "--head"}) + assertContains(t, stderr, "diff --head requires a downstream HEAD commit") + + indexOut, indexErr := runCommandOKWithOutput(t, repo, []string{"diff", "--index"}) + if indexOut != "" || indexErr != "" { + t.Fatalf("unborn index diff output = (%q, %q), want empty", indexOut, indexErr) + } } func TestDiffCommandFromSubdirectoryPreservesRawPassthroughPathspecs(t *testing.T) { @@ -111,6 +151,12 @@ func TestDiffCommandSyncPushOnlyFiltersSources(t *testing.T) { sourceOut := runCommandOK(t, repo, []string{"diff", ":enabled", "--sync-push-only"}) assertContains(t, sourceOut, "Braid: Diffing mirror vendor/enabled-a") assertContains(t, sourceOut, "Braid: Diffing mirror vendor/enabled-b") + testutil.CommitAll(t, repo, "commit filtered changes") + headOut := runCommandOK(t, repo, []string{"diff", "--sync-push-only", "--head"}) + assertContains(t, headOut, "enabled a changed") + assertContains(t, headOut, "enabled b changed") + assertNotContains(t, headOut, "vendor/disabled") + assertNotContains(t, headOut, "disabled changed") disabledOut, disabledErr := runCommandOKWithOutput(t, repo, []string{"diff", "vendor/disabled", "--sync-push-only"}) if disabledOut != "" || disabledErr != "" { @@ -211,11 +257,19 @@ func TestDiffCommandSingleFilePrefixes(t *testing.T) { repo := initDownstream(t) runCommandOK(t, repo, []string{"add", upstream, "licenses/THIRD_PARTY.txt=LICENSE.txt"}) - testutil.WriteFile(t, repo, "licenses/THIRD_PARTY.txt", "changed license\n") - - out := runCommandOK(t, repo, []string{"diff", "licenses/THIRD_PARTY.txt"}) - assertContains(t, out, "diff --git a/LICENSE.txt b/THIRD_PARTY.txt") - assertContains(t, out, "changed license") + testutil.WriteFile(t, repo, "licenses/THIRD_PARTY.txt", "committed license\n") + testutil.Git(t, repo, "add", "licenses/THIRD_PARTY.txt") + testutil.Git(t, repo, "commit", "-m", "commit license change") + + headOut := runCommandOK(t, repo, []string{"diff", "licenses/THIRD_PARTY.txt", "--head"}) + assertContains(t, headOut, "diff --git a/LICENSE.txt b/THIRD_PARTY.txt") + assertContains(t, headOut, "committed license") + + testutil.WriteFile(t, repo, "licenses/THIRD_PARTY.txt", "staged license\n") + testutil.Git(t, repo, "add", "licenses/THIRD_PARTY.txt") + indexOut := runCommandOK(t, repo, []string{"diff", "licenses/THIRD_PARTY.txt", "--index"}) + assertContains(t, indexOut, "diff --git a/LICENSE.txt b/THIRD_PARTY.txt") + assertContains(t, indexOut, "staged license") } func TestDiffCommandSingleFileFromSubdirectoryUsesTopAnchoredLimiter(t *testing.T) { diff --git a/internal/command/status.go b/internal/command/status.go index 9c7163c..9b31fc0 100644 --- a/internal/command/status.go +++ b/internal/command/status.go @@ -178,7 +178,7 @@ func trackingLabel(m source.SourceMirror) string { } func locallyModified(ctx context.Context, git StatusGit, m source.SourceMirror) (bool, error) { - args, err := buildDiffArgs(ctx, git, m, nil) + args, err := buildDiffArgs(ctx, git, m, cli.DiffOptions{}) if err != nil { return false, err }