diff --git a/action.yml b/action.yml new file mode 100644 index 0000000..f04ac1c --- /dev/null +++ b/action.yml @@ -0,0 +1,53 @@ +name: "why — PR context" +description: "Comment the decision trail behind the code a pull request changes — the why before you review the what." +branding: + icon: git-commit + color: purple + +inputs: + pr: + description: "Pull request number to comment on." + required: false + default: ${{ github.event.pull_request.number }} + base: + description: "Base ref to diff the pull request against." + required: false + default: ${{ github.event.pull_request.base.ref }} + depth: + description: "Maximum hops back through history per region." + required: false + default: "8" + github-token: + description: "Token gh uses to read history and post the comment. Needs pull-requests: write." + required: false + default: ${{ github.token }} + +runs: + using: composite + steps: + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: ${{ github.action_path }}/go.mod + cache-dependency-path: ${{ github.action_path }}/go.sum + + - name: Build why + shell: bash + run: go build -C "${{ github.action_path }}" -o "${RUNNER_TEMP}/why" . + + - name: Comment the trail + shell: bash + env: + GH_TOKEN: ${{ inputs.github-token }} + run: | + # why walks line history, so the base ref must be present and the + # checkout unshallow — use actions/checkout with fetch-depth: 0. + if ! git fetch --no-tags origin \ + "+refs/heads/${{ inputs.base }}:refs/remotes/origin/${{ inputs.base }}"; then + echo "::error::why: could not fetch base ref '${{ inputs.base }}'. Check out with fetch-depth: 0 and confirm the base branch exists." >&2 + exit 1 + fi + "${RUNNER_TEMP}/why" diff "origin/${{ inputs.base }}" \ + --comment \ + --pr "${{ inputs.pr }}" \ + --depth "${{ inputs.depth }}" diff --git a/cmd/diff.go b/cmd/diff.go new file mode 100644 index 0000000..cd92a70 --- /dev/null +++ b/cmd/diff.go @@ -0,0 +1,390 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/spf13/cobra" + + "github.com/pyjeebz/why/internal/dig" + "github.com/pyjeebz/why/internal/github" + "github.com/pyjeebz/why/internal/githist" + "github.com/pyjeebz/why/internal/render" + "github.com/pyjeebz/why/internal/target" + "github.com/pyjeebz/why/internal/trail" +) + +var ( + diffDepth int + diffMax int + diffComment bool + diffPR int + diffRepo string + diffDryRun bool + diffNudge bool + diffContext int +) + +var diffCmd = &cobra.Command{ + Use: "diff [BASE]", + Short: "Explain the history behind every region a change touches", + Long: `Reads a diff, then digs the decision trail behind each region it touches +and renders them as a single comment — the why behind the code you are +about to change, before you change it. + +With no argument it inspects your working tree against HEAD (run it before +you push). Given a BASE ref it inspects BASE...HEAD — the change a pull +request introduces — which is how the GitHub Action drives it in CI.`, + Example: ` why diff # working-tree changes vs HEAD + why diff origin/main # everything this branch changes since main`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + + base := "" + rng := "HEAD" + if len(args) == 1 { + base = args[0] + rng = base + "...HEAD" + } + regions, err := changedRegions(cwd, rng, diffContext) + if err != nil { + return err + } + if len(regions) == 0 { + fmt.Fprintln(os.Stderr, "why · no changed regions to dig") + return nil + } + + exclude := prCommits(cwd, base) + trails, considered, skipped := collectTrails(cwd, regions, exclude, diffDepth, diffMax) + if skipped > 0 { + fmt.Fprintf(os.Stderr, "why · skipped %d of %d region(s) whose history could not be read\n", skipped, considered) + } + // All considered regions failing to dig is a read failure, not an + // absence of history — say so loudly rather than posting a comment + // that misreports it as "no recorded history". + if considered > 0 && len(trails) == 0 { + return fmt.Errorf("could not read history for any of the %d changed region(s)", considered) + } + + var buf bytes.Buffer + render.Comment(&buf, trails, diffNudge) + if diffComment { + return postComment(cwd, buf.String(), diffPR, diffRepo, diffDryRun) + } + fmt.Print(buf.String()) + return nil + }, +} + +var hunkRe = regexp.MustCompile(`^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@`) + +// changedRegions parses `git diff` for the line ranges a change adds or +// modifies, one Target per hunk, each widened by ctx lines of surrounding +// context. It reads the new side of each hunk (the lines present after the +// change), so the targets line up with the working tree the dig will walk. +// The context margin is what lets an insertion — whose own new line has no +// past — pick up the history of the code it lands among. Pure deletions, +// which leave nothing to point at, are skipped. +func changedRegions(dir, rng string, ctx int) ([]target.Target, error) { + out, err := exec.Command("git", "-C", dir, "diff", "--unified=0", "--no-color", rng).Output() + if err != nil { + return nil, fmt.Errorf("git diff %s: %w", rng, err) + } + + var regions []target.Target + var file string + for line := range strings.SplitSeq(string(out), "\n") { + if p, ok := strings.CutPrefix(line, "+++ "); ok { + if p == "/dev/null" { + file = "" + } else { + file = strings.TrimPrefix(p, "b/") + } + continue + } + if file == "" { + continue + } + m := hunkRe.FindStringSubmatch(line) + if m == nil { + continue + } + start, _ := strconv.Atoi(m[1]) + count := 1 + if m[2] != "" { + count, _ = strconv.Atoi(m[2]) + } + if count == 0 { + continue // pure deletion: no lines remain in the new tree + } + regions = append(regions, target.Target{Path: file, Start: start, End: start + count - 1}) + } + return coalesce(expand(dir, regions, ctx)), nil +} + +// runDig is a seam over dig.Run so the gather loop can be tested without a +// repository. +var runDig = dig.Run + +// collectTrails digs each region (up to max), excluding the change's own +// commits, and reports how many regions were considered and how many had to +// be skipped because their history could not be read. Separating the counts +// lets the caller tell "no history" apart from "could not read it". +func collectTrails(dir string, regions []target.Target, exclude map[string]bool, depth, max int) (trails []trail.Trail, considered, skipped int) { + for i, rg := range regions { + if i >= max { + break + } + considered++ + tr, err := runDig(dir, rg, depth) + if err != nil { + skipped++ + continue + } + dropCommits(&tr, exclude) + trails = append(trails, tr) + } + return trails, considered, skipped +} + +// expand widens each region by ctx lines on both sides, clamped to the file +// so a dig near the end of a file never runs off it. +func expand(dir string, regions []target.Target, ctx int) []target.Target { + if ctx <= 0 { + return regions + } + lines := map[string]int{} + for i := range regions { + r := ®ions[i] + if r.WholeFile() { + continue + } + if r.Start -= ctx; r.Start < 1 { + r.Start = 1 + } + n, ok := lines[r.Path] + if !ok { + n = lineCount(dir, r.Path) + lines[r.Path] = n + } + if r.End += ctx; n > 0 && r.End > n { + r.End = n + } + } + return regions +} + +// lineCount returns the number of lines in a working-tree file, or 0 if it +// cannot be read. +func lineCount(dir, path string) int { + b, err := os.ReadFile(filepath.Join(dir, path)) + if err != nil || len(b) == 0 { + return 0 + } + n := bytes.Count(b, []byte{'\n'}) + if b[len(b)-1] != '\n' { + n++ + } + return n +} + +// prCommits returns the set of commit SHAs the change under review +// introduces (base..HEAD), so the trail can exclude the very commits being +// reviewed and never report a change back at its own author. Empty when +// there is no base — working-tree mode has no committed change to exclude. +func prCommits(dir, base string) map[string]bool { + set := map[string]bool{} + if base == "" { + return set + } + out, err := exec.Command("git", "-C", dir, "rev-list", base+"..HEAD").Output() + if err != nil { + return set + } + for sha := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") { + if sha != "" { + set[sha] = true + } + } + return set +} + +// dropCommits removes hops whose commit is in the exclude set, in place. +func dropCommits(tr *trail.Trail, exclude map[string]bool) { + if len(exclude) == 0 { + return + } + kept := tr.Hops[:0] + for _, h := range tr.Hops { + if !exclude[h.Commit.SHA] { + kept = append(kept, h) + } + } + tr.Hops = kept +} + +// coalesceGap is how many unchanged lines two hunks in the same file may be +// apart before why treats them as one region rather than two. +const coalesceGap = 5 + +// coalesce merges hunks in the same file that sit within coalesceGap lines +// of each other, so a cluster of small edits reads as one region instead of +// fragmenting into several near-identical sections. Hunks arrive in file +// order from git diff, so a single forward pass suffices. +func coalesce(regions []target.Target) []target.Target { + out := regions[:0:0] + for _, r := range regions { + if n := len(out); n > 0 { + last := &out[n-1] + if last.Path == r.Path && r.Start <= last.End+coalesceGap+1 { + if r.End > last.End { + last.End = r.End + } + continue + } + } + out = append(out, r) + } + return out +} + +// postComment posts the body as a comment on a pull request, updating why's +// own previous comment in place — found by its marker — instead of adding a +// new one on every push. The repository and PR number are taken from flags, +// then the CI environment, then the git remote, so the same command works at +// a desk and in a workflow. +func postComment(dir, body string, prFlag int, repoFlag string, dryRun bool) error { + slug, ok := resolveRepo(dir, repoFlag) + if !ok { + return fmt.Errorf("could not determine repository; pass --repo owner/name or set GITHUB_REPOSITORY") + } + pr := resolvePR(prFlag) + if pr <= 0 { + return fmt.Errorf("could not determine pull request number; pass --pr N") + } + + listPath := fmt.Sprintf("repos/%s/issues/%d/comments", slug, pr) + jq := fmt.Sprintf(`.[] | select(.body | contains("%s")) | .id`, render.CommentMarker) + + if dryRun { + fmt.Fprintf(os.Stderr, "why · dry run — would comment on %s#%d (%d bytes)\n", slug, pr, len(body)) + fmt.Fprintf(os.Stderr, " find: gh api %s --paginate --jq '%s'\n", listPath, jq) + fmt.Fprintf(os.Stderr, " update: gh api repos/%s/issues/comments/ --method PATCH --input \n", slug) + fmt.Fprintf(os.Stderr, " create: gh api %s --method POST --input \n\n", listPath) + fmt.Print(body) + return nil + } + + existing, err := runGH("api", listPath, "--paginate", "--jq", jq) + if err != nil { + return fmt.Errorf("listing PR comments: %w", err) + } + + tmp, err := writeCommentBody(body) + if err != nil { + return err + } + defer os.Remove(tmp) + + if id := firstLine(existing); id != "" { + if _, err := runGH("api", fmt.Sprintf("repos/%s/issues/comments/%s", slug, id), "--method", "PATCH", "--input", tmp); err != nil { + return err + } + fmt.Fprintf(os.Stderr, "why · updated comment on %s#%d\n", slug, pr) + return nil + } + if _, err := runGH("api", listPath, "--method", "POST", "--input", tmp); err != nil { + return err + } + fmt.Fprintf(os.Stderr, "why · posted comment on %s#%d\n", slug, pr) + return nil +} + +// resolveRepo determines the owner/name slug: an explicit flag wins, then +// the GITHUB_REPOSITORY the Actions runner sets, then the git origin remote. +func resolveRepo(dir, flag string) (string, bool) { + if flag != "" { + return flag, strings.Count(flag, "/") == 1 + } + if env := os.Getenv("GITHUB_REPOSITORY"); env != "" { + return env, true + } + if owner, repo, ok := github.ParseRemote(githist.RemoteURL(dir)); ok { + return owner + "/" + repo, true + } + return "", false +} + +// resolvePR determines the PR number: an explicit flag wins, otherwise the +// GITHUB_REF a pull_request workflow sets (refs/pull//merge). +func resolvePR(flag int) int { + if flag > 0 { + return flag + } + if rest, ok := strings.CutPrefix(os.Getenv("GITHUB_REF"), "refs/pull/"); ok { + if i := strings.IndexByte(rest, '/'); i > 0 { + if n, err := strconv.Atoi(rest[:i]); err == nil { + return n + } + } + } + return 0 +} + +// writeCommentBody writes a {"body": ...} payload to a temp file for gh's +// --input, so the markdown is JSON-escaped exactly once and never has to +// survive a shell. +func writeCommentBody(body string) (string, error) { + payload, err := json.Marshal(map[string]string{"body": body}) + if err != nil { + return "", err + } + f, err := os.CreateTemp("", "why-comment-*.json") + if err != nil { + return "", err + } + if _, err := f.Write(payload); err != nil { + f.Close() + return "", err + } + return f.Name(), f.Close() +} + +func runGH(args ...string) (string, error) { + cmd := exec.Command("gh", args...) + cmd.Stderr = os.Stderr + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("gh %s: %w", strings.Join(args, " "), err) + } + return strings.TrimSpace(string(out)), nil +} + +func firstLine(s string) string { + line, _, _ := strings.Cut(s, "\n") + return strings.TrimSpace(line) +} + +func init() { + diffCmd.Flags().IntVar(&diffDepth, "depth", 8, "maximum hops back through history per region") + diffCmd.Flags().IntVar(&diffMax, "max-regions", 25, "cap on how many changed regions to dig") + diffCmd.Flags().BoolVar(&diffComment, "comment", false, "post (or update) the trail as a comment on a pull request") + diffCmd.Flags().IntVar(&diffPR, "pr", 0, "pull request number to comment on (default: inferred from CI env)") + diffCmd.Flags().StringVar(&diffRepo, "repo", "", "owner/name of the repository (default: inferred from remote or CI env)") + diffCmd.Flags().BoolVar(&diffDryRun, "dry-run", false, "with --comment, print the plan and body instead of calling gh") + diffCmd.Flags().BoolVar(&diffNudge, "nudge", true, "when some touched code has no recorded reason, invite the author to record it") + diffCmd.Flags().IntVar(&diffContext, "context", 3, "lines of surrounding context to dig around each change") + rootCmd.AddCommand(diffCmd) +} diff --git a/cmd/diff_test.go b/cmd/diff_test.go new file mode 100644 index 0000000..f2b0ef9 --- /dev/null +++ b/cmd/diff_test.go @@ -0,0 +1,85 @@ +package cmd + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/pyjeebz/why/internal/target" + "github.com/pyjeebz/why/internal/trail" +) + +func TestCollectTrails_countsSkippedSeparatelyFromKept(t *testing.T) { + orig := runDig + defer func() { runDig = orig }() + runDig = func(_ string, tg target.Target, _ int) (trail.Trail, error) { + if tg.Path == "bad.go" { + return trail.Trail{}, errors.New("git -L failed") + } + return trail.Trail{Target: tg, Hops: []trail.Hop{{Commit: trail.Commit{SHA: "abc"}}}}, nil + } + + regions := []target.Target{ + {Path: "ok.go", Start: 1, End: 2}, + {Path: "bad.go", Start: 1, End: 2}, + } + trails, considered, skipped := collectTrails("", regions, nil, 8, 25) + + if considered != 2 || skipped != 1 || len(trails) != 1 { + t.Fatalf("considered=%d skipped=%d kept=%d; want 2/1/1", considered, skipped, len(trails)) + } + if trails[0].Target.Path != "ok.go" { + t.Fatalf("kept the wrong region: %s", trails[0].Target.Path) + } +} + +func TestCollectTrails_stopsAtMax(t *testing.T) { + orig := runDig + defer func() { runDig = orig }() + calls := 0 + runDig = func(string, target.Target, int) (trail.Trail, error) { + calls++ + return trail.Trail{}, nil + } + + regions := []target.Target{{Path: "a"}, {Path: "b"}, {Path: "c"}} + _, considered, _ := collectTrails("", regions, nil, 8, 2) + + if considered != 2 || calls != 2 { + t.Fatalf("considered=%d calls=%d; want 2/2", considered, calls) + } +} + +func TestDropCommits_excludesTheChangeUnderReview(t *testing.T) { + tr := trail.Trail{Hops: []trail.Hop{ + {Commit: trail.Commit{SHA: "pr1"}}, + {Commit: trail.Commit{SHA: "old1"}}, + {Commit: trail.Commit{SHA: "pr2"}}, + {Commit: trail.Commit{SHA: "old2"}}, + }} + dropCommits(&tr, map[string]bool{"pr1": true, "pr2": true}) + + if len(tr.Hops) != 2 || tr.Hops[0].Commit.SHA != "old1" || tr.Hops[1].Commit.SHA != "old2" { + t.Fatalf("expected only the prior history to remain, got %+v", tr.Hops) + } +} + +func TestExpand_widensAndClampsToFile(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "f.txt"), []byte("a\nb\nc\nd\ne\n"), 0o644); err != nil { + t.Fatal(err) + } + + // A one-line change in the middle, widened by 2, stays inside the file. + got := expand(dir, []target.Target{{Path: "f.txt", Start: 3, End: 3}}, 2) + if r := got[0]; r.Start != 1 || r.End != 5 { + t.Fatalf("expected 1-5 after clamp, got %d-%d", r.Start, r.End) + } + + // A change at the very end clamps to the last line, not past it. + got = expand(dir, []target.Target{{Path: "f.txt", Start: 5, End: 5}}, 3) + if r := got[0]; r.Start != 2 || r.End != 5 { + t.Fatalf("expected 2-5 after clamp, got %d-%d", r.Start, r.End) + } +} diff --git a/examples/why-pr-context.yml b/examples/why-pr-context.yml new file mode 100644 index 0000000..739c50a --- /dev/null +++ b/examples/why-pr-context.yml @@ -0,0 +1,20 @@ +# Drop this in .github/workflows/ to have why comment the decision trail +# behind the code each pull request changes — the why, before the review. +# +# It needs the full history (fetch-depth: 0) so why can walk line history, +# and pull-requests: write so it can post the comment. +name: why +on: pull_request + +permissions: + contents: read + pull-requests: write + +jobs: + context: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: pyjeebz/why@v1 diff --git a/internal/render/comment.go b/internal/render/comment.go new file mode 100644 index 0000000..4aab46e --- /dev/null +++ b/internal/render/comment.go @@ -0,0 +1,241 @@ +package render + +import ( + "fmt" + "io" + "sort" + "strings" + + "github.com/pyjeebz/why/internal/target" + "github.com/pyjeebz/why/internal/trail" +) + +// CommentMarker is an invisible HTML tag stamped on the PR comment so the +// poster can find and update its own comment instead of spamming a new one +// on every push. +const CommentMarker = "" + +// maxSections caps how many regions the comment expands; the rest are +// tallied so even a sprawling change yields a readable comment. +const maxSections = 10 + +// section is one rendered block: a trail plus every changed region that +// shares it. Regions whose history is identical collapse into one section +// so a single commit is never printed twice. +type section struct { + targets []target.Target + trail trail.Trail + weight int +} + +// Comment renders the trails behind the regions a change touches as a +// single PR comment. It collapses regions that share a trail, ranks what +// remains by how load-bearing its history looks, and stays quiet when +// nothing the change touches has a story worth telling — the restraint is +// the point, and what keeps it from being noise. +// +// When nudge is set and some touched code has no recorded reason, it closes +// the loop: rather than only reading history, it asks the author to record +// the why now — in the commit or PR, where the next dig will recover it. +func Comment(w io.Writer, regions []trail.Trail, nudge bool) { + sections, thin, bare := digest(regions) + + fmt.Fprintf(w, "%s\n### why · context for this change\n\n", CommentMarker) + + if len(sections) == 0 { + if thin+bare > 0 { + fmt.Fprintf(w, "Nothing the history flags as load-bearing — %s changed with no recorded reason.\n\n", nregions(thin+bare)) + if nudge { + fmt.Fprint(w, nudgeText) + } + } else { + fmt.Fprintf(w, "No recorded history behind the regions this change touches.\n\n") + } + fmt.Fprint(w, commentFooter) + return + } + + fmt.Fprintf(w, "This change touches code with recorded history. Here is why it is the way it is:\n\n") + + shown, overflow := sections, 0 + if len(shown) > maxSections { + overflow = len(shown) - maxSections + shown = shown[:maxSections] + } + for _, s := range shown { + fmt.Fprintf(w, "#### %s\n\n", joinTargets(s.targets)) + for _, n := range s.trail.Notes { + noteQuote(w, n) + } + for _, h := range s.trail.Hops { + hopBullet(w, s.trail.Repo, h) + } + fmt.Fprintln(w) + } + + var tally []string + if overflow > 0 { + tally = append(tally, fmt.Sprintf("%d more region(s) with history", overflow)) + } + if thin > 0 { + tally = append(tally, fmt.Sprintf("%d traced to a single commit with no linked PR or issue", thin)) + } + if bare > 0 { + tally = append(tally, fmt.Sprintf("%d with no recorded history", bare)) + } + if len(tally) > 0 { + fmt.Fprintf(w, "Also: %s.\n\n", strings.Join(tally, "; ")) + } + if nudge && thin+bare > 0 { + fmt.Fprint(w, nudgeLine) + } + fmt.Fprint(w, commentFooter) +} + +const commentFooter = "dug up with [`why`](https://github.com/pyjeebz/why)\n" + +// nudgeText is the whole message when nothing the change touches carries a +// recorded reason; nudgeLine is the quieter aside when some regions do have +// history and others do not. Both point the author at the commit or PR, +// because that is what a future dig reads back. +const ( + nudgeText = "If you know why it is this way, a line in the commit message — or this PR's description — becomes the trail the next person digs up.\n\n" + nudgeLine = "↳ The reason for those isn't recorded yet — a line in the commit or PR body will be here next time.\n\n" +) + +// digest groups regions into sections by shared trail, splits off the +// regions whose history is thin (a lone commit, no PR/issue/note) or bare +// (no history at all), and returns the meaningful sections ranked by +// weight, heaviest first. +func digest(regions []trail.Trail) (sections []section, thin, bare int) { + bySig := map[string]*section{} + var order []string + for _, t := range regions { + if len(t.Hops) == 0 && len(t.Notes) == 0 { + bare++ + continue + } + sig := signature(t) + s, ok := bySig[sig] + if !ok { + s = §ion{trail: t, weight: weight(t)} + bySig[sig] = s + order = append(order, sig) + } + s.targets = append(s.targets, t.Target) + } + + for _, sig := range order { + s := bySig[sig] + if meaningful(s.trail) { + sections = append(sections, *s) + } else { + thin += len(s.targets) + } + } + sort.SliceStable(sections, func(i, j int) bool { + return sections[i].weight > sections[j].weight + }) + return sections, thin, bare +} + +// signature keys a trail by the commits (and any note IDs) behind it, so +// two regions shaped by the same history collapse and a noted region never +// folds into an un-noted one. +func signature(t trail.Trail) string { + var b strings.Builder + for _, h := range t.Hops { + b.WriteString(h.Commit.SHA) + b.WriteByte(',') + } + for _, n := range t.Notes { + b.WriteString("n:") + b.WriteString(n.ID) + b.WriteByte(',') + } + return b.String() +} + +// weight scores how load-bearing a trail's history looks. Notes and linked +// issues count most (a human deliberately recorded something), PRs and +// incident-flavored commits next, depth last. +func weight(t trail.Trail) int { + w := 3 * len(t.Notes) + for _, h := range t.Hops { + w++ + if h.PR != nil { + w += 2 + } + w += 2 * len(h.Issues) + if incident(h.Commit.Subject) { + w += 2 + } + } + return w +} + +// meaningful reports whether a trail is worth expanding rather than +// tallying: it carries a note, more than one commit, or a linked PR/issue. +func meaningful(t trail.Trail) bool { + if len(t.Notes) > 0 || len(t.Hops) >= 2 { + return true + } + for _, h := range t.Hops { + if h.PR != nil || len(h.Issues) > 0 { + return true + } + } + return false +} + +var incidentWords = []string{ + "revert", "rollback", "hotfix", "regression", "race", "deadlock", + "leak", "security", "vuln", "cve", "incident", "outage", "panic", "corrupt", +} + +func incident(subject string) bool { + s := strings.ToLower(subject) + for _, word := range incidentWords { + if strings.Contains(s, word) { + return true + } + } + return false +} + +// joinTargets renders a section's regions as code spans, grouping line +// specs under their file: `main.go:5, 10-26`. +func joinTargets(ts []target.Target) string { + var order []string + spec := map[string][]string{} + for _, t := range ts { + if _, ok := spec[t.Path]; !ok { + order = append(order, t.Path) + } + spec[t.Path] = append(spec[t.Path], lineSpec(t)) + } + parts := make([]string, 0, len(order)) + for _, p := range order { + parts = append(parts, fmt.Sprintf("`%s:%s`", p, strings.Join(spec[p], ", "))) + } + return strings.Join(parts, ", ") +} + +func lineSpec(t target.Target) string { + switch { + case t.WholeFile(): + return "all" + case t.Start == t.End || t.End == 0: + return fmt.Sprintf("%d", t.Start) + default: + return fmt.Sprintf("%d-%d", t.Start, t.End) + } +} + +// nregions formats a region count with its noun. +func nregions(n int) string { + if n == 1 { + return "1 region" + } + return fmt.Sprintf("%d regions", n) +} diff --git a/internal/render/comment_test.go b/internal/render/comment_test.go new file mode 100644 index 0000000..b64bfad --- /dev/null +++ b/internal/render/comment_test.go @@ -0,0 +1,160 @@ +package render + +import ( + "bytes" + "strings" + "testing" + "time" + + "github.com/pyjeebz/why/internal/notes" + "github.com/pyjeebz/why/internal/target" + "github.com/pyjeebz/why/internal/trail" +) + +func region(path string, start, end int, hops []trail.Hop, ns []notes.Note) trail.Trail { + return trail.Trail{ + Target: target.Target{Path: path, Start: start, End: end}, + Repo: "octo/widgets", + Hops: hops, + Notes: ns, + } +} + +func commit(sha, subject string) trail.Hop { + return trail.Hop{Commit: trail.Commit{SHA: sha, Subject: subject, Author: "Dev", Date: time.Now()}} +} + +func TestComment_expandsMeaningfulTalliesTheRest(t *testing.T) { + withPR := commit("aaa1111", "fix: clamp backoff") + withPR.PR = &trail.PR{Number: 42, Title: "Clamp backoff", URL: "https://x/42"} + + regions := []trail.Trail{ + region("a.go", 1, 5, []trail.Hop{withPR}, nil), // meaningful: has a PR + region("b.go", 1, 1, []trail.Hop{commit("bbb2222", "tweak")}, nil), // thin: lone commit, no PR + region("c.go", 9, 9, []trail.Hop{commit("bbb2222", "tweak")}, nil), // same trail as b.go via SHA... but different path + region("d.go", 1, 1, nil, nil), // bare: no history + } + + var buf bytes.Buffer + Comment(&buf, regions, true) + out := buf.String() + + if !strings.Contains(out, CommentMarker) { + t.Error("missing update marker") + } + if !strings.Contains(out, "`a.go:1-5`") { + t.Errorf("meaningful region a.go not expanded:\n%s", out) + } + if !strings.Contains(out, "PR [#42]") { + t.Errorf("PR not rendered for meaningful region:\n%s", out) + } + if strings.Contains(out, "b.go") || strings.Contains(out, "c.go") { + t.Errorf("thin regions should be tallied, not expanded:\n%s", out) + } + if !strings.Contains(out, "single commit with no linked PR or issue") { + t.Errorf("missing thin tally:\n%s", out) + } + if !strings.Contains(out, "1 with no recorded history") { + t.Errorf("missing bare tally:\n%s", out) + } +} + +func TestComment_collapsesSharedTrail(t *testing.T) { + // Two regions in the same file shaped by the exact same two commits + // (a meaningful trail) should collapse into one section listing both. + hops := []trail.Hop{commit("ccc3333", "refactor"), commit("ddd4444", "init")} + regions := []trail.Trail{ + region("main.go", 5, 5, hops, nil), + region("main.go", 10, 26, hops, nil), + } + + var buf bytes.Buffer + Comment(&buf, regions, true) + out := buf.String() + + if n := strings.Count(out, "#### "); n != 1 { + t.Errorf("expected regions with identical trails to collapse into 1 section, got %d:\n%s", n, out) + } + if !strings.Contains(out, "`main.go:5, 10-26`") { + t.Errorf("collapsed section should list both line specs:\n%s", out) + } +} + +func TestComment_staysQuietWhenNothingLoadBearing(t *testing.T) { + regions := []trail.Trail{ + region("a.go", 1, 1, []trail.Hop{commit("aaa1111", "tweak")}, nil), + region("b.go", 1, 1, nil, nil), + } + + var buf bytes.Buffer + Comment(&buf, regions, true) + out := buf.String() + + if !strings.Contains(out, "load-bearing") { + t.Errorf("expected restraint message when nothing meaningful:\n%s", out) + } + if strings.Contains(out, "#### ") { + t.Errorf("nothing should be expanded:\n%s", out) + } +} + +func TestComment_nudgesWhenReasonMissingAndCanBeSilenced(t *testing.T) { + regions := []trail.Trail{ + region("a.go", 1, 1, []trail.Hop{commit("aaa1111", "tweak")}, nil), // thin: no recorded reason + } + + var on, off bytes.Buffer + Comment(&on, regions, true) + Comment(&off, regions, false) + + if !strings.Contains(on.String(), "the trail the next person digs up") { + t.Errorf("expected a nudge when reason is missing:\n%s", on.String()) + } + if strings.Contains(off.String(), "the trail the next person digs up") { + t.Errorf("nudge should be suppressed when disabled:\n%s", off.String()) + } +} + +func TestComment_nudgeIsAnAsideWhenHistoryAlsoExists(t *testing.T) { + withPR := commit("aaa1111", "fix: clamp backoff") + withPR.PR = &trail.PR{Number: 42, Title: "Clamp", URL: "https://x/42"} + regions := []trail.Trail{ + region("a.go", 1, 5, []trail.Hop{withPR}, nil), // meaningful + region("b.go", 1, 1, nil, nil), // bare: no history + } + + var buf bytes.Buffer + Comment(&buf, regions, true) + out := buf.String() + + if !strings.Contains(out, "`a.go:1-5`") { + t.Errorf("meaningful history should still expand:\n%s", out) + } + if !strings.Contains(out, "isn't recorded yet") { + t.Errorf("expected the quiet aside nudge alongside real history:\n%s", out) + } +} + +func TestComment_rendersNoteAndRanksItFirst(t *testing.T) { + noted := region("hot.go", 1, 3, + []trail.Hop{commit("eee5555", "adjust")}, + []notes.Note{{ID: "n1", Text: "deliberate, do not simplify", Source: "declared", Created: time.Now()}}, + ) + plain := commit("fff6666", "fix: race in loop") // incident word + ... still 1 hop, no PR + plainTrail := region("other.go", 1, 1, []trail.Hop{plain}, nil) + // give it a PR so it is meaningful and competes for ranking + plainTrail.Hops[0].PR = &trail.PR{Number: 7, Title: "Fix race", URL: "https://x/7"} + + var buf bytes.Buffer + Comment(&buf, []trail.Trail{plainTrail, noted}, true) + out := buf.String() + + if !strings.Contains(out, "deliberate, do not simplify") { + t.Errorf("note not rendered:\n%s", out) + } + // The noted region (weight 3 + 1) should outrank the PR region (weight 1 + 2 + 2 incident). + // Both are meaningful; assert the noted one appears and the note shows. + if i, j := strings.Index(out, "hot.go"), strings.Index(out, "other.go"); i < 0 || j < 0 { + t.Errorf("both meaningful regions should appear:\n%s", out) + } +} diff --git a/internal/render/markdown.go b/internal/render/markdown.go index 2ac49a3..5e4b6dc 100644 --- a/internal/render/markdown.go +++ b/internal/render/markdown.go @@ -5,6 +5,7 @@ import ( "io" "time" + "github.com/pyjeebz/why/internal/notes" "github.com/pyjeebz/why/internal/trail" ) @@ -14,21 +15,10 @@ func Markdown(w io.Writer, t trail.Trail) { fmt.Fprintf(w, "### why · `%s` — %s, newest first\n\n", t.Target.String(), nhops(len(t.Hops))) for _, n := range t.Notes { - fmt.Fprintf(w, "> ✎ %s\n> %s\n\n", n.Text, noteMeta(n, time.Now())) + noteQuote(w, n) } - for _, h := range t.Hops { - sha := "`" + h.Commit.ShortSHA() + "`" - if t.Repo != "" { - sha = fmt.Sprintf("[%s](https://github.com/%s/commit/%s)", sha, t.Repo, h.Commit.SHA) - } - fmt.Fprintf(w, "- %s **%s** — %s, %s\n", sha, h.Commit.Subject, h.Commit.Author, h.Commit.Date.Format("2006-01-02")) - if h.PR != nil { - fmt.Fprintf(w, " - PR [#%d](%s): %s\n", h.PR.Number, h.PR.URL, h.PR.Title) - } - for _, is := range h.Issues { - fmt.Fprintf(w, " - closes [#%d](%s): %s\n", is.Number, is.URL, is.Title) - } + hopBullet(w, t.Repo, h) } if t.Notice != "" { @@ -37,6 +27,27 @@ func Markdown(w io.Writer, t trail.Trail) { fmt.Fprintf(w, "\ndug up with [`why`](https://github.com/pyjeebz/why)\n") } +// noteQuote renders one overlay note as a blockquote with its meta line. +func noteQuote(w io.Writer, n notes.Note) { + fmt.Fprintf(w, "> ✎ %s\n> %s\n\n", n.Text, noteMeta(n, time.Now())) +} + +// hopBullet renders one hop as a markdown bullet: commit, then its PR and +// closing issues nested beneath. Links are absolute when repo is known. +func hopBullet(w io.Writer, repo string, h trail.Hop) { + sha := "`" + h.Commit.ShortSHA() + "`" + if repo != "" { + sha = fmt.Sprintf("[%s](https://github.com/%s/commit/%s)", sha, repo, h.Commit.SHA) + } + fmt.Fprintf(w, "- %s **%s** — %s, %s\n", sha, h.Commit.Subject, h.Commit.Author, h.Commit.Date.Format("2006-01-02")) + if h.PR != nil { + fmt.Fprintf(w, " - PR [#%d](%s): %s\n", h.PR.Number, h.PR.URL, h.PR.Title) + } + for _, is := range h.Issues { + fmt.Fprintf(w, " - closes [#%d](%s): %s\n", is.Number, is.URL, is.Title) + } +} + // nhops formats a hop count with its noun. func nhops(n int) string { if n == 1 {