diff --git a/cmd/diff.go b/cmd/diff.go index 97e8b56..7f927ed 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -168,8 +168,9 @@ func runDiffFull(ctx context.Context, source string, paramMap map[string]string, // Returns the number of repos that had changes. func printTargetDiffs(root string, w io.Writer) (int, error) { repos := gitRepoDirs(root) + color := isTerminalWriter(w) colorFlag := "--color=never" - if isTerminalWriter(w) { + if color { colorFlag = "--color=always" } @@ -187,7 +188,7 @@ func printTargetDiffs(root string, w io.Writer) (int, error) { continue } changed++ - fmt.Fprintf(w, "\n# %s\n", filepath.Base(dir)) + fmt.Fprint(w, targetDiffHeader(dir, base, color)) fmt.Fprint(w, out) if !strings.HasSuffix(out, "\n") { fmt.Fprintln(w) @@ -196,6 +197,70 @@ func printTargetDiffs(root string, w io.Writer) (int, error) { return changed, nil } +// targetDiffHeader renders the "which module / which repo" banner above a +// target's git diff: the module name (from the numbered clone dir), the origin +// remote URL, and the base branch — all read from the clone itself. +func targetDiffHeader(dir, base string, color bool) string { + module := moduleFromDir(filepath.Base(dir)) + branch := strings.TrimPrefix(base, "refs/remotes/origin/") + repo := "" + if url, err := runGit(dir, "remote", "get-url", "origin"); err == nil { + repo = strings.TrimSpace(url) + } + if branch != "" && branch != "HEAD" { + if repo != "" { + repo += " (" + branch + ")" + } else { + repo = branch + } + } + return diffHeaderLine(module, repo, color) +} + +// moduleFromDir strips the "NN-" execution-order prefix local mode adds to a +// clone directory, leaving the module name. +func moduleFromDir(name string) string { + i := 0 + for i < len(name) && name[i] >= '0' && name[i] <= '9' { + i++ + } + if i > 0 && i < len(name) && name[i] == '-' { + return name[i+1:] + } + return name +} + +// diffHeaderLine renders a module/target banner matching the quick-mode diff +// header: an inverted module chip followed by a muted target label. +func diffHeaderLine(module, target string, color bool) string { + var b strings.Builder + b.WriteByte('\n') + switch { + case !color: + if module != "" { + b.WriteString("[" + module + "]") + } + if target != "" { + if module != "" { + b.WriteByte(' ') + } + b.WriteString(target) + } + default: + if module != "" { + b.WriteString("\033[7m " + module + " \033[0m") + } + if target != "" { + if module != "" { + b.WriteByte(' ') + } + b.WriteString("\033[38;5;244m" + target + "\033[0m") + } + } + b.WriteByte('\n') + return b.String() +} + // gitRepoDirs returns root (if a git repo) followed by its immediate git-repo // subdirectories, in directory-name order — matching the numbered subdirs that // local mode clones into. diff --git a/cmd/diff_test.go b/cmd/diff_test.go index 4a8eb73..9cee0ca 100644 --- a/cmd/diff_test.go +++ b/cmd/diff_test.go @@ -119,6 +119,11 @@ spec: if !strings.Contains(out, "-hello") || !strings.Contains(out, "+HELLO") { t.Errorf("expected shell-op change (notes.txt) in diff, got:\n%s", out) } + // DF4: the diff must carry a header identifying the module and target repo, + // so it stays legible away from the surrounding operation logs. + if !strings.Contains(out, "demo") || !strings.Contains(out, upstream) { + t.Errorf("expected module/repo header in diff, got:\n%s", out) + } } // DF2: --quick simulates the run (dry-run) — it prints newFiles/patch unified @@ -156,6 +161,10 @@ spec: if !strings.Contains(out, "new content") { t.Errorf("expected rendered content in quick diff, got:\n%s", out) } + // Quick diffs carry a module header for context, just like full mode. + if !strings.Contains(out, "quick-demo") { + t.Errorf("expected module header in quick diff, got:\n%s", out) + } if _, err := os.Stat(filepath.Join(targetDir, "new.txt")); !os.IsNotExist(err) { t.Error("--quick must not write files to the target") } diff --git a/pkg/action/action.go b/pkg/action/action.go index 8f8d753..271b7e8 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log/slog" + "strings" ) // PRResult records one pull/merge request created during a run. @@ -44,34 +45,85 @@ func (s *RunSummary) Print(w io.Writer) { } } +// diffEntry is one captured file diff plus the context needed to read it once +// diffs are printed together at the end: which module produced it and which +// target (repo) it applies to. +type diffEntry struct { + module string + target string + text string // uncolored unified diff +} + // DiffCollector accumulates file diffs across a whole run, shared by parent // and child module executions. Execution is sequential, so no locking is // needed. Diffs are held rather than written inline as each file is merged, // so the whole set can be printed once at the very end of the run — after the -// per-operation logs — where they are easy to read. +// per-operation logs. Each diff carries a module/target header so it stays +// legible out of the surrounding log context. type DiffCollector struct { - // Diffs holds each captured file diff as uncolored unified-diff text. - Diffs []string + entries []diffEntry } -// Add records one file diff. Safe to call on a nil collector. -func (c *DiffCollector) Add(diff string) { +// Add records one file diff along with the module and target it belongs to. +// Safe to call on a nil collector. +func (c *DiffCollector) Add(module, target, diff string) { if c == nil { return } - c.Diffs = append(c.Diffs, diff) + c.entries = append(c.entries, diffEntry{module: module, target: target, text: diff}) } // Print writes all collected diffs to w, colorized when w is a terminal. -// Nothing is written when the collector is nil or empty. +// A module/target header is written before each diff (deduplicated across a +// run of diffs from the same module and target). Nothing is written when the +// collector is nil or empty. func (c *DiffCollector) Print(w io.Writer) { - if c == nil || len(c.Diffs) == 0 { + if c == nil || len(c.entries) == 0 { return } color := isTerminalWriter(w) - for _, diff := range c.Diffs { - fmt.Fprint(w, colorizeDiff(diff, color)) + var lastModule, lastTarget string + for i, e := range c.entries { + if i == 0 || e.module != lastModule || e.target != lastTarget { + fmt.Fprint(w, diffHeader(e.module, e.target, color)) + lastModule, lastTarget = e.module, e.target + } + fmt.Fprint(w, colorizeDiff(e.text, color)) + } +} + +// diffHeader renders the "which module / which repo" banner shown above a diff. +// Returns just a leading blank line when there is no context to show. +func diffHeader(module, target string, color bool) string { + if module == "" && target == "" { + return "\n" + } + var b strings.Builder + b.WriteByte('\n') + switch { + case !color: + if module != "" { + b.WriteString("[" + module + "]") + } + if target != "" { + if module != "" { + b.WriteByte(' ') + } + b.WriteString(target) + } + default: + if module != "" { + b.WriteString(diffColorInvert + " " + module + " " + diffColorReset) + } + if target != "" { + if module != "" { + b.WriteByte(' ') + } + b.WriteString(diffColorMuted + target + diffColorReset) + } } + b.WriteByte('\n') + return b.String() } // ExecutionContext holds runtime state shared across actions. @@ -82,6 +134,10 @@ type ExecutionContext struct { ModuleDir string // TargetDir is the path to the target repository working directory. TargetDir string + // TargetLabel is a human-readable identity for the target (repo URL and + // branch, or the target dir), shown as a header above collected diffs so + // they stay legible out of the surrounding log context. May be empty. + TargetLabel string // Params are the resolved template parameters. Params map[string]string // Excludes are glob patterns for files/dirs to exclude from template walking. diff --git a/pkg/action/diff.go b/pkg/action/diff.go index 113aac5..7794c0e 100644 --- a/pkg/action/diff.go +++ b/pkg/action/diff.go @@ -10,10 +10,12 @@ import ( // ANSI color codes for diff output. const ( - diffColorReset = "\033[0m" - diffColorRed = "\033[31m" - diffColorGreen = "\033[32m" - diffColorCyan = "\033[36m" + diffColorReset = "\033[0m" + diffColorRed = "\033[31m" + diffColorGreen = "\033[32m" + diffColorCyan = "\033[36m" + diffColorInvert = "\033[7m" + diffColorMuted = "\033[38;5;244m" // mid gray — matches the log handler's muted tone ) // printDiff computes a unified diff between old and new content and hands it to @@ -54,7 +56,7 @@ func printDiff(execCtx *ExecutionContext, path, oldContent, newContent string) { return } - execCtx.Diffs.Add(text) + execCtx.Diffs.Add(execCtx.ModuleName, execCtx.TargetLabel, text) } // colorizeDiff applies ANSI colors to unified diff lines. diff --git a/pkg/module/module.go b/pkg/module/module.go index 6cdaede..1def6da 100644 --- a/pkg/module/module.go +++ b/pkg/module/module.go @@ -162,19 +162,38 @@ func evalParamCommand(name, command, moduleDir string, logger *slog.Logger) (str // NewExecutionContext creates an ExecutionContext for this module. func (m *Module) NewExecutionContext(targetDir string, opts RunOptions) *action.ExecutionContext { return &action.ExecutionContext{ - ModuleName: m.Config.Metadata.Name, - ModuleDir: m.Dir, - TargetDir: targetDir, - Params: m.Params, - Excludes: m.Config.Spec.Excludes, - Includes: m.Config.Spec.Includes, - DryRun: opts.DryRun, - LocalRun: opts.LocalRun, - ShowDiff: opts.ShowDiff, - Diffs: opts.Diffs, - GitAuthor: opts.GitAuthor, - GitEmail: opts.GitEmail, - Summary: opts.Summary, - Logger: m.Logger, + ModuleName: m.Config.Metadata.Name, + ModuleDir: m.Dir, + TargetDir: targetDir, + TargetLabel: m.targetLabel(targetDir), + Params: m.Params, + Excludes: m.Config.Spec.Excludes, + Includes: m.Config.Spec.Includes, + DryRun: opts.DryRun, + LocalRun: opts.LocalRun, + ShowDiff: opts.ShowDiff, + Diffs: opts.Diffs, + GitAuthor: opts.GitAuthor, + GitEmail: opts.GitEmail, + Summary: opts.Summary, + Logger: m.Logger, } } + +// targetLabel is a human-readable identity for the module's target, used as a +// header above collected diffs: the rendered repo URL and branch when the +// module has a target spec, otherwise the target directory it runs against. +func (m *Module) targetLabel(targetDir string) string { + t := m.Config.Spec.Target + if t == nil { + return targetDir + } + url, err := tmpl.RenderString(t.URL, m.Params) + if err != nil || url == "" { + return targetDir + } + if branch, err := tmpl.RenderString(t.Branch, m.Params); err == nil && branch != "" { + return url + " (" + branch + ")" + } + return url +} diff --git a/specs/module.md b/specs/module.md index e2f30ae..9b9e8c2 100644 --- a/specs/module.md +++ b/specs/module.md @@ -922,6 +922,13 @@ Full mode clones into a workspace: a temporary directory that is removed once th diff is printed, unless `--target-path` is supplied, in which case the clones are kept there for inspection. +#### DF4: Diffs are headed with module and target + +Because diffs print together at the end — away from the per-operation logs — +each diff is preceded by a header naming the module that produced it and the +target it applies to (repo URL and branch, or the target directory). Consecutive +diffs sharing the same module and target reuse a single header. + --- ## Validation Rules