diff --git a/appview/models/pull2.go b/appview/models/pull2.go new file mode 100644 index 00000000..3307afd2 --- /dev/null +++ b/appview/models/pull2.go @@ -0,0 +1,39 @@ +package models + +import ( + "time" + + "github.com/bluesky-social/indigo/atproto/syntax" +) + +type Pull2 struct { + ID int64 // repo-specific PR id + + RepoDid syntax.DID + AuthorDid syntax.DID + Rkey syntax.RecordKey + + Title string + Body string + TargetBranch string + Versions []PullVersion + Created time.Time + + // optionally, populate this when querying for reverse mappings + Repo *Repo +} + +func (p *Pull2) LatestVersionId() int { + return len(p.Versions) - 1 +} + +func (p *Pull2) LatestVersion() PullVersion { + return p.Versions[p.LatestVersionId()] +} + +type PullVersion struct { + SourceRepo syntax.DID + Head string // head commit ID + Base string // base commit ID + Created time.Time +} diff --git a/appview/pages/pages.go b/appview/pages/pages.go index c56b1ff0..e6b52a8e 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -1429,7 +1429,7 @@ type RepoSinglePullParams struct { type PullPageBaseParams struct { BaseParams - Pull *models.Pull + Pull *models.Pull2 Backlinks []models.RichReferenceLink Comments []models.Comment @@ -1469,7 +1469,7 @@ type PullDiffFragmentParams struct { } func (p *Pages) PullDiff(w io.Writer, params PullDiffParams) error { - panic("unimplemented") + return p.executeRepo("repo/pulls/single", w, params) } func (p *Pages) PullInterdiff(w io.Writer, params PullInterdiffParams) error { diff --git a/appview/pages/templates/repo/pulls/single.html b/appview/pages/templates/repo/pulls/single.html new file mode 100644 index 00000000..d1b1eba8 --- /dev/null +++ b/appview/pages/templates/repo/pulls/single.html @@ -0,0 +1,66 @@ +{{ define "title" }} + wip +{{ end }} + +{{ define "mainLayout" }} +{{ $version := .Pull.LatestVersion }} + +
+
+
+
+

{{ .Pull.Title }}

+
{{ .Pull.Body | markdown }}
+
+
+ labels +
+
+ commits +
+
+
+
+ + diff header +
+
+ + +
+ diff body +
+ loading... +
+
+
+
+
+
+ discussion +
+
+{{ end }} + +{{ define "repoContentLayout" }} +
+

repo content

+
+{{ end }} diff --git a/appview/pulls/diff.go b/appview/pulls/diff.go new file mode 100644 index 00000000..3302e313 --- /dev/null +++ b/appview/pulls/diff.go @@ -0,0 +1 @@ +package pulls diff --git a/appview/pulls/diff_hunks_test.go b/appview/pulls/diff_hunks_test.go index e6475291..2180d112 100644 --- a/appview/pulls/diff_hunks_test.go +++ b/appview/pulls/diff_hunks_test.go @@ -26,130 +26,184 @@ func hunk(pairs ...*gitmirrorv1.LinePair) *gitmirrorv1.Hunk { return &gitmirrorv1.Hunk{Lines: pairs} } -func TestAlignFile_Modification(t *testing.T) { - base := lines(10, map[int]string{5: "old"}) - head := lines(10, map[int]string{5: "new"}) - hunks := []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(5), Rhs: u32(5)})} - - pairs, change := alignFile(base, head, hunks) - // Whole file is represented (10 lines), exactly one change at the line-5 pair. - if len(pairs) != 10 || len(change) != 10 { - t.Fatalf("want 10 pairs, got %d", len(pairs)) - } - nChange := 0 - for i, c := range change { - if c { - nChange++ - if pairs[i].lhs != 5 || pairs[i].rhs != 5 { - t.Fatalf("change pair = %+v, want {5,5}", pairs[i]) - } - } - } - if nChange != 1 { - t.Fatalf("want 1 change, got %d", nChange) +// identity opposite maps + generous bounds, for exercising the merge/context logic directly. +func identityOpp(n int) (map[int]int, map[int]int) { + l, r := map[int]int{}, map[int]int{} + for i := 0; i < n; i++ { + l[i], r[i] = i, i } + return l, r } -func TestGroupHunks_SingleWindowClamped(t *testing.T) { - base := lines(10, map[int]string{5: "old"}) - head := lines(10, map[int]string{5: "new"}) - _, change := alignFile(base, head, []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(5), Rhs: u32(5)})}) +func TestExtractLines_FillsInteriorGap(t *testing.T) { + got := extractLines([]linePair{{0, 0}, {2, 2}}) + want := []linePair{{0, 0}, {1, 1}, {2, 2}} + if len(got) != len(want) { + t.Fatalf("got %+v, want %+v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("pair %d = %+v, want %+v", i, got[i], want[i]) + } + } +} - got := groupHunks(change) - if len(got) != 1 { - t.Fatalf("want 1 window, got %d: %v", len(got), got) +func TestPadBeforeAfter_ClampAndCount(t *testing.T) { + // n+1 = 4 lines when far from the edges. + if got := padBefore(10, numContextLines); len(got) != 4 || got[0] != 6 || got[3] != 9 { + t.Fatalf("padBefore(10) = %v", got) + } + if got := padAfter(10, 100, numContextLines); len(got) != 4 || got[0] != 11 || got[3] != 14 { + t.Fatalf("padAfter(10,100) = %v", got) + } + // Clamp at start of file. + if got := padBefore(2, numContextLines); len(got) != 2 || got[0] != 0 || got[1] != 1 { + t.Fatalf("padBefore(2) = %v", got) } - // Change at index 5, ctx = n+1 = 4 -> [1, 10) clamped to file end. - if got[0] != [2]int{1, 10} { - t.Fatalf("window = %v, want [1 10)", got[0]) + // Clamp at end of file. + if got := padAfter(98, 100, numContextLines); len(got) != 2 || got[0] != 99 || got[1] != 100 { + t.Fatalf("padAfter(98,100) = %v", got) } } -func TestGroupHunks_MergeClose(t *testing.T) { - // Changes 5 lines apart: windows overlap -> single merged window. - base := lines(20, map[int]string{4: "a", 9: "b"}) - head := lines(20, map[int]string{4: "A", 9: "B"}) - _, change := alignFile(base, head, []*gitmirrorv1.Hunk{ - hunk(&gitmirrorv1.LinePair{Lhs: u32(4), Rhs: u32(4)}), - hunk(&gitmirrorv1.LinePair{Lhs: u32(9), Rhs: u32(9)}), - }) - got := groupHunks(change) +func TestMergeAdjacent_MergeClose(t *testing.T) { + oppL, oppR := identityOpp(200) + // Two deletions 2 lines apart on the lhs -> context windows overlap -> merge. + got := mergeAdjacent([][]linePair{{{2, -1}}, {{4, -1}}}, oppL, oppR, 199, 199, numContextLines) if len(got) != 1 { - t.Fatalf("want 1 merged window, got %d: %v", len(got), got) + t.Fatalf("want 1 merged hunk, got %d: %+v", len(got), got) } } -func TestGroupHunks_SplitFar(t *testing.T) { - // Changes 20 lines apart: windows disjoint -> two windows. - base := lines(40, map[int]string{4: "a", 25: "b"}) - head := lines(40, map[int]string{4: "A", 25: "B"}) - _, change := alignFile(base, head, []*gitmirrorv1.Hunk{ - hunk(&gitmirrorv1.LinePair{Lhs: u32(4), Rhs: u32(4)}), - hunk(&gitmirrorv1.LinePair{Lhs: u32(25), Rhs: u32(25)}), - }) - got := groupHunks(change) +func TestMergeAdjacent_SplitFar(t *testing.T) { + oppL, oppR := identityOpp(200) + got := mergeAdjacent([][]linePair{{{2, -1}}, {{40, -1}}}, oppL, oppR, 199, 199, numContextLines) if len(got) != 2 { - t.Fatalf("want 2 windows, got %d: %v", len(got), got) + t.Fatalf("want 2 hunks, got %d: %+v", len(got), got) } } -func TestAlignFile_PureInsertionAtTop(t *testing.T) { - base := lines(5, nil) - head := append([]string{"x", "y"}, base...) // 2 inserted lines, then base - hunks := []*gitmirrorv1.Hunk{hunk( - &gitmirrorv1.LinePair{Rhs: u32(0)}, - &gitmirrorv1.LinePair{Rhs: u32(1)}, - )} - pairs, change := alignFile(base, head, hunks) +func TestMergeAdjacent_LargeOneSidedInsertion(t *testing.T) { + oppL, oppR := identityOpp(200) + // Two modifications adjacent on the lhs (2 and 4) but far apart on the rhs (2 and 40), + // as if 30+ lines were inserted between them. difftastic merges by per-side line + // distance, so the lhs overlap keeps them one hunk — the case aligned-index windowing + // would have split. + got := mergeAdjacent([][]linePair{{{2, 2}}, {{4, 40}}}, oppL, oppR, 199, 199, numContextLines) + if len(got) != 1 { + t.Fatalf("want 1 merged hunk (lhs overlap), got %d: %+v", len(got), got) + } +} - // First two pairs are the insertions (lhs absent), then 5 context pairs offset by 2. - if pairs[0] != (linePair{lhs: -1, rhs: 0}) || pairs[1] != (linePair{lhs: -1, rhs: 1}) { - t.Fatalf("insertion pairs = %+v %+v", pairs[0], pairs[1]) +func TestBuildOpposites_DeletionPairing(t *testing.T) { + // Delete base line 4 of a 10-line file -> head has 9 lines. Unchanged base 5..9 must + // pair with head 4..8 (the bijection difftastic's opposite_positions encodes). + base := lines(10, nil) + head := lines(9, nil) + hunks := []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(4)})} + + oppLhs, oppRhs, maxLhs, maxRhs := buildOpposites(base, head, hunks) + if maxLhs != 9 || maxRhs != 8 { + t.Fatalf("max lines = %d,%d want 9,8", maxLhs, maxRhs) } - if !change[0] || !change[1] { - t.Fatalf("insertion pairs should be marked changed") + for l := 5; l <= 9; l++ { + if oppLhs[l] != l-1 { + t.Fatalf("oppLhs[%d] = %d, want %d", l, oppLhs[l], l-1) + } } - if pairs[2] != (linePair{lhs: 0, rhs: 2}) { - t.Fatalf("first context pair = %+v, want {0,2}", pairs[2]) + if oppRhs[4] != 5 { // head line 4 <-> base line 5 + t.Fatalf("oppRhs[4] = %d, want 5", oppRhs[4]) } - if len(pairs) != 7 { - t.Fatalf("want 7 pairs, got %d", len(pairs)) + // The deleted base line 4 has no counterpart. + if _, ok := oppLhs[4]; ok { + t.Fatalf("deleted base line 4 should have no opposite") } } -func TestAlignFile_PureDeletionKeepsColumnsPaired(t *testing.T) { - // Delete base line index 4 of a 10-line file -> head has 9 lines. The unchanged tail - // (base 5..9) must stay paired with head 4..8, not drift. - base := lines(10, nil) - head := append(append([]string{}, lines(4, nil)...), lines(10, nil)[5:]...) // line 4 removed - hunks := []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(4)})} +func TestBuildHunks_MarksNovelAndAddsContext(t *testing.T) { + base := lines(10, map[int]string{5: "old"}) + head := lines(10, map[int]string{5: "new"}) + built := buildHunks(base, head, []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(5), Rhs: u32(5)})}) + if len(built) != 1 { + t.Fatalf("want 1 hunk, got %d", len(built)) + } + var nChanged, nContext int + for _, r := range built[0].rows { + if r.changed { + nChanged++ + if r.lhs != 5 || r.rhs != 5 { + t.Fatalf("changed row = %+v, want {5,5}", r) + } + } else { + nContext++ + } + } + if nChanged != 1 { + t.Fatalf("want 1 changed row, got %d", nChanged) + } + // Side-by-side display shows numContextLines (3) context rows each side, index-based + // (difftastic's matched_lines_indexes_for_hunk), both clamped inside the 10-line file. + if nContext != 6 { + t.Fatalf("want 6 context rows, got %d: %+v", nContext, built[0].rows) + } +} - pairs, change := alignFile(base, head, hunks) +func TestBuildHunks_MergedKeepsInteriorContext(t *testing.T) { + // Regression: a deletion and a nearby addition, merged into one hunk, are separated by + // unchanged lines. Those interior context rows must survive (reconstructing context via + // extractLines/fillBetween dropped them across the deletion->addition boundary). + // Base: 20 lines; delete base line 5. Head: base with line 5 removed and a new line + // inserted at head index 8 (a few lines after the deletion point). + base := lines(20, nil) + head := make([]string, 0, 20) + head = append(head, base[:5]...) // head 0..4 == base 0..4 + head = append(head, base[6:9]...) // head 5..7 == base 6..8 + head = append(head, "INSERTED") // head 8 (new) + head = append(head, base[9:]...) // head 9.. == base 9.. - // The deletion pair. - del := -1 - for i, c := range change { - if c { - del = i - break - } + hunks := []*gitmirrorv1.Hunk{ + hunk(&gitmirrorv1.LinePair{Lhs: u32(5)}), // delete base line 5 + hunk(&gitmirrorv1.LinePair{Rhs: u32(8)}), // insert head line 8 } - if del < 0 || pairs[del] != (linePair{lhs: 4, rhs: -1}) { - t.Fatalf("deletion pair = %+v (idx %d)", pairs[del], del) + built := buildHunks(base, head, hunks) + if len(built) != 1 { + t.Fatalf("want 1 merged hunk, got %d", len(built)) } - // Trailing context after the deletion: base 5..9 <-> head 4..8. - want := []linePair{{5, 4}, {6, 5}, {7, 6}, {8, 7}, {9, 8}} - got := pairs[del+1:] - if len(got) != len(want) { - t.Fatalf("trailing context len = %d, want %d: %+v", len(got), len(want), got) + // The unchanged lines between the deletion (base 5) and the insertion (head 8, i.e. base + // 8) — base lines 6,7,8 — must appear as context rows (both sides present, unchanged). + want := map[int]bool{6: false, 7: false, 8: false} + got := map[int]bool{} + for _, r := range built[0].rows { + if !r.changed && r.lhs >= 0 { + if _, ok := want[r.lhs]; ok { + got[r.lhs] = true + } + } } - for i := range want { - if got[i] != want[i] { - t.Fatalf("trailing pair %d = %+v, want %+v", i, got[i], want[i]) + for ln := range want { + if !got[ln] { + t.Fatalf("interior context base line %d missing from merged hunk: %+v", ln, built[0].rows) } } } +func TestIndexesForHunk_SliceAndClamp(t *testing.T) { + base := lines(20, nil) + head := lines(20, nil) + // Single change at line 10. + pairs, _ := alignFile(base, head, []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(10), Rhs: u32(10)})}) + lo, hi := indexesForHunk(pairs, []linePair{{10, 10}}, numContextLines) + if lo != 7 || hi != 14 { // [10-3 .. 10+3+1) + t.Fatalf("slice = [%d,%d), want [7,14)", lo, hi) + } + // Clamp at start of file. + pairs2, _ := alignFile(base, head, []*gitmirrorv1.Hunk{hunk(&gitmirrorv1.LinePair{Lhs: u32(1), Rhs: u32(1)})}) + lo2, _ := indexesForHunk(pairs2, []linePair{{1, 1}}, numContextLines) + if lo2 != 0 { + t.Fatalf("start clamp = %d, want 0", lo2) + } +} + func TestRenderInline_Ordering(t *testing.T) { base := lines(10, map[int]string{5: "old"}) head := lines(10, map[int]string{5: "new"}) @@ -201,7 +255,6 @@ func TestRenderInline_MergedKeepsInteriorContextOnce(t *testing.T) { t.Fatalf("interior context should have no +/- marker: %q", line) } } - // The actual changes still render. for _, want := range []string{"- old5", "+ new5", "- old10", "+ new10"} { if !strings.Contains(out, want) { t.Fatalf("missing %q in:\n%s", want, out) diff --git a/appview/pulls/pull2.go b/appview/pulls/pull2.go index d8441e4a..74962750 100644 --- a/appview/pulls/pull2.go +++ b/appview/pulls/pull2.go @@ -6,9 +6,12 @@ import ( "fmt" "io" "net/http" + "slices" "strconv" "strings" + "time" + "github.com/bluesky-social/indigo/api/atproto" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/bluesky-social/indigo/lex/util" indigoxrpc "github.com/bluesky-social/indigo/xrpc" @@ -36,21 +39,51 @@ import ( // 1. rebase B<-C to D // 2. compare tree of C and D +var pull = &models.Pull2{ + ID: 123, + RepoDid: syntax.DID("did:plc:ofcpzigpnwrgtpl3ojrpg3y7"), + AuthorDid: syntax.DID("did:example:alice"), + Rkey: syntax.RecordKey("pr-rkey"), + Title: "Spindle-owned data", + Body: "something something", + TargetBranch: "master", + Versions: []models.PullVersion{ + { + SourceRepo: syntax.DID("did:plc:ofcpzigpnwrgtpl3ojrpg3y7"), + Head: "443b2e347c3f77bc9aa2481adb518440f204b870", + Base: "5c97f1cc886344bb8a9eba315a39c0fc14cee51f", + }, + }, + Created: time.Now(), +} + +var comments = []*models.Comment{ + { + Did: syntax.DID("did:example:alice"), + Collection: syntax.NSID(tangled.FeedCommentNSID), + Rkey: syntax.RecordKey("comment"), + Cid: syntax.CID(""), + Subject: atproto.RepoStrongRef{ + Uri: "at://did:example:alice/sh.tangled.repo.pull/pr-rkey", + Cid: "", + }, + Body: tangled.MarkupMarkdown{ + Text: "review comment", + }, + Created: time.Now(), + }, +} + // PullInterDiff is router for /pulls/{pull}/{version}..{version}/{change} // // Examples: // - /pulls/123/0..2/all // - /pulls/123/0..2/nrpytyzw func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { - l := s.logger.With("handler", "PullRound") + l := s.logger.With("handler", "PullInterDiff") ctx := r.Context() - pull, ok := r.Context().Value("pull").(*models.Pull) - if !ok { - l.Error("failed to get pull") - s.pages.Error500(w) - return - } + _ = l var ( version1 = 0 @@ -84,11 +117,11 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { var commits1, commits2 []types.Commit g, gctx := errgroup.WithContext(ctx) g.Go(func() error { - commits1, err = getTempListCommits(gctx, xrpcc, pull.SourceRepoDid(), base, head1) + commits1, err = getTempListCommits(gctx, xrpcc, pull.LatestVersion().SourceRepo, base, head1) return err }) g.Go(func() error { - commits2, err = getTempListCommits(gctx, xrpcc, pull.SourceRepoDid(), base, head2) + commits2, err = getTempListCommits(gctx, xrpcc, pull.LatestVersion().SourceRepo, base, head2) return err }) if err := g.Wait(); err != nil { @@ -128,28 +161,23 @@ func (s *Pulls) PullInterDiff(w http.ResponseWriter, r *http.Request) { // - /pulls/123/2/a53ab251e..d8add468c // - /pulls/123/2/d8add468c func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { - l := s.logger.With("handler", "PullRound") + l := s.logger.With("handler", "PullDiff") ctx := r.Context() - pull, ok := r.Context().Value("pull").(*models.Pull) - if !ok { - l.Error("failed to get pull") - s.pages.Error500(w) - return - } + _ = l var err error var version int var versionRaw = chi.URLParam(r, "version") if versionRaw == "latest" { - version = pull.LastRoundNumber() + version = pull.LatestVersionId() } else { version, err = strconv.Atoi(versionRaw) if err != nil { // invalid version number. redirect http.Redirect(w, r, - fmt.Sprintf("/%s/pulls/%d/latest", pull.Repo.RepoIdentifier(), pull.ID), + fmt.Sprintf("/%s/pulls/%d/latest", pull.RepoDid, pull.ID), http.StatusSeeOther, ) return @@ -160,7 +188,7 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { base, head, err := parseRevRange(range_) if err != nil { http.Redirect(w, r, - fmt.Sprintf("/%s/pulls/%d/%s", pull.Repo.RepoIdentifier(), pull.ID, versionRaw), + fmt.Sprintf("/%s/pulls/%d/%s", pull.RepoDid, pull.ID, versionRaw), http.StatusSeeOther, ) return @@ -185,10 +213,10 @@ func (s *Pulls) PullDiff(w http.ResponseWriter, r *http.Request) { base = branch.Hash } if head == "head" { - head = pull.HEAD() + head = pull.LatestVersion().Head } - sourceRepoDid := pull.SourceRepoDid() + sourceRepoDid := pull.LatestVersion().SourceRepo // 2. list diverged commits using knotmirror (BASE..HEAD) -> ([]commit) // - knotmirror needs on-demand fetch implementation for this @@ -273,8 +301,8 @@ func (s *Pulls) PullDiffFragment(w http.ResponseWriter, r *http.Request) { if err != nil { return err } - f.baseLines = splitLines(baseBlob) - f.headLines = splitLines(headBlob) + f.baseLines = strings.Split(strings.TrimSuffix(string(baseBlob), "\n"), "\n") + f.headLines = strings.Split(strings.TrimSuffix(string(headBlob), "\n"), "\n") return nil }) } @@ -282,6 +310,8 @@ func (s *Pulls) PullDiffFragment(w http.ResponseWriter, r *http.Request) { panic(err) } + //
code
+ // c. render each file's diff, following difftastic's display layout // (github.com/Wilfred/difftastic src/display): side-by-side by default, inline // when ?view=unified. Word/token highlighting and line wrapping are omitted. @@ -332,42 +362,174 @@ type displayHunk struct { rows []diffRow } -// buildHunks aligns the whole file (alignFile), groups changes into context windows -// (groupHunks), and slices each window into a displayHunk of rows. +// maxDistance mirrors difftastic's MAX_DISTANCE (hunks.rs): the most unchanged lines that +// may sit between two changes before they are split into separate hunks (before context). +const maxDistance = 4 + +// buildHunks ports difftastic's src/display pre-processing (hunks.rs + context.rs): it +// regroups the changed line pairs into hunks by per-side line distance, merges hunks whose +// context windows overlap, and expands each with context lines, yielding displayHunks. +// +// gitmirror's Hunk.Lines are difftastic's novel line pairs; we lack its token-level +// MatchedPos, but a line-granular diff's unchanged lines form a 1:1 bijection, which stands +// in for difftastic's opposite_positions map (buildOpposites). func buildHunks(baseLines, headLines []string, hunks []*gitmirrorv1.Hunk) []displayHunk { - pairs, change := alignFile(baseLines, headLines, hunks) + oppLhs, oppRhs, maxLhs, maxRhs := buildOpposites(baseLines, headLines, hunks) + + var flat []linePair + for _, h := range hunks { + for _, lp := range h.Lines { + flat = append(flat, toPair(lp)) + } + } + + merged := mergeAdjacent(linesToHunks(flat), oppLhs, oppRhs, maxLhs, maxRhs, numContextLines) + + // Display rows come from the whole-file aligned list sliced per hunk (difftastic's + // side_by_side: all_matched_lines_filled + matched_lines_indexes_for_hunk). Slicing a + // contiguous list keeps the interior context lines of merged hunks, which reconstructing + // via extractLines/fillBetween drops across a deletion->addition boundary. + pairs, changed := alignFile(baseLines, headLines, hunks) + var out []displayHunk - for _, rng := range groupHunks(change) { - var h displayHunk - for i := rng[0]; i < rng[1]; i++ { - h.rows = append(h.rows, diffRow{lhs: pairs[i].lhs, rhs: pairs[i].rhs, changed: change[i]}) + prevEnd := 0 + for _, h := range merged { + lo, hi := indexesForHunk(pairs, h, numContextLines) + if lo < prevEnd { + lo = prevEnd // don't re-emit rows shared with the previous hunk's slice + } + var dh displayHunk + for i := lo; i < hi; i++ { + dh.rows = append(dh.rows, diffRow{lhs: pairs[i].lhs, rhs: pairs[i].rhs, changed: changed[i]}) } - out = append(out, h) + out = append(out, dh) + prevEnd = hi } return out } -// alignFile walks both blobs in lockstep and produces one linePair per displayed line of -// the whole file, plus a parallel `change` flag marking pairs that came from a gitmirror -// hunk's changed Lines (everything else is unchanged context). -// -// gitmirror hunks carry only changed lines; unchanged lines are a 1:1 bijection between -// the blobs, so the two cursors (li/ri) advance together across the gaps. A hunk side that -// has no lines (pure insertion/deletion) gets its start derived from the other side. -func alignFile(baseLines, headLines []string, hunks []*gitmirrorv1.Hunk) ([]linePair, []bool) { - var pairs []linePair - var change []bool +// alignFile builds the whole-file aligned list (difftastic's all_matched_lines_filled, +// specialized to a line-granular diff): every displayed line as a pair, plus a parallel +// `changed` flag for pairs that came from a gitmirror hunk. Unchanged lines are a 1:1 +// bijection, so the two cursors advance together across gaps. +func alignFile(baseLines, headLines []string, hunks []*gitmirrorv1.Hunk) (pairs []linePair, changed []bool) { li, ri := 0, 0 + emitContext := func(n int) { + for k := range n { + pairs = append(pairs, linePair{lhs: li + k, rhs: ri + k}) + changed = append(changed, false) + } + li += n + ri += n + } + for _, h := range hunks { + lhsStart, _, ok := hunkStart(h, li, ri) + if !ok { + continue + } + emitContext(lhsStart - li) // unchanged gap before this change (== rhsStart-ri) + for _, lp := range h.Lines { + p := toPair(lp) + if p.lhs >= 0 { + li = p.lhs + 1 + } + if p.rhs >= 0 { + ri = p.rhs + 1 + } + pairs = append(pairs, p) + changed = append(changed, true) + } + } + // Trailing unchanged tail (both sides advance equally). + for li < len(baseLines) && ri < len(headLines) { + pairs = append(pairs, linePair{lhs: li, rhs: ri}) + changed = append(changed, false) + li++ + ri++ + } + return pairs, changed +} + +// indexesForHunk returns the [start,end) slice of the whole-file aligned pairs to display +// for a merged hunk: the span from its smallest to largest novel line, expanded by n context +// lines on each side and clamped (difftastic's matched_lines_indexes_for_hunk). +func indexesForHunk(pairs, hunkLines []linePair, n int) (start, end int) { + minLhs, minRhs, maxLhs, maxRhs := -1, -1, -1, -1 + for _, lp := range hunkLines { + if lp.lhs >= 0 { + if minLhs < 0 { + minLhs = lp.lhs + } + maxLhs = lp.lhs + } + if lp.rhs >= 0 { + if minRhs < 0 { + minRhs = lp.rhs + } + maxRhs = lp.rhs + } + } + smallest, largest := linePair{minLhs, minRhs}, linePair{maxLhs, maxRhs} - // emit appends a run of unchanged context pairs [lx..lx+n) <-> [rx..rx+n) and advances - // both cursors past them (lx == li and rx == ri at every call site). - emitContext := func(lx, rx, n int) { - for j := range n { - pairs = append(pairs, linePair{lhs: lx + j, rhs: rx + j}) - change = append(change, false) + start = 0 + for i, p := range pairs { + if eitherSideEqual(p, smallest) { + start = i + break + } + } + end = len(pairs) + for i := len(pairs) - 1; i >= 0; i-- { + if eitherSideEqual(pairs[i], largest) { + end = i + 1 + break } - li = lx + n - ri = rx + n + } + + start = max(0, start-n) + end = min(len(pairs), end+n) + return start, end +} + +// eitherSideEqual reports whether a and b share a present line number on the same side +// (difftastic's either_side_equal). +func eitherSideEqual(a, b linePair) bool { + if a.lhs >= 0 && a.lhs == b.lhs { + return true + } + if a.rhs >= 0 && a.rhs == b.rhs { + return true + } + return false +} + +// toPair converts a proto LinePair to a linePair, mapping absent sides to -1. +func toPair(lp *gitmirrorv1.LinePair) linePair { + p := linePair{lhs: -1, rhs: -1} + if lp.Lhs != nil { + p.lhs = int(*lp.Lhs) + } + if lp.Rhs != nil { + p.rhs = int(*lp.Rhs) + } + return p +} + +// buildOpposites is our stand-in for difftastic's opposite_positions: the 1:1 mapping +// between unchanged base and head lines. It walks the gitmirror hunks with two cursors — +// unchanged runs advance both sides together — recording each unchanged line's counterpart. +// maxLhs/maxRhs are the 0-based last line numbers of each blob. +func buildOpposites(baseLines, headLines []string, hunks []*gitmirrorv1.Hunk) (oppLhs, oppRhs map[int]int, maxLhs, maxRhs int) { + oppLhs, oppRhs = map[int]int{}, map[int]int{} + li, ri := 0, 0 + + pair := func(gap int) { + for k := range gap { + oppLhs[li+k] = ri + k + oppRhs[ri+k] = li + k + } + li += gap + ri += gap } for _, h := range hunks { @@ -375,25 +537,24 @@ func alignFile(baseLines, headLines []string, hunks []*gitmirrorv1.Hunk) ([]line if !ok { continue } - emitContext(li, ri, lhsStart-li) // gap before this change (== rhsStart-ri) - + pair(lhsStart - li) // unchanged gap before this change for _, lp := range h.Lines { - p := linePair{lhs: -1, rhs: -1} if lp.Lhs != nil { - p.lhs = int(*lp.Lhs) - li = p.lhs + 1 + li = int(*lp.Lhs) + 1 } if lp.Rhs != nil { - p.rhs = int(*lp.Rhs) - ri = p.rhs + 1 + ri = int(*lp.Rhs) + 1 } - pairs = append(pairs, p) - change = append(change, true) } } - // Trailing unchanged tail. - emitContext(li, ri, len(baseLines)-li) - return pairs, change + // Trailing unchanged tail (both sides advance equally). + for li < len(baseLines) && ri < len(headLines) { + oppLhs[li] = ri + oppRhs[ri] = li + li++ + ri++ + } + return oppLhs, oppRhs, len(baseLines) - 1, len(headLines) - 1 } // hunkStart returns the first changed line number on each side, deriving the empty side @@ -420,27 +581,360 @@ func hunkStart(h *gitmirrorv1.Hunk, li, ri int) (lhsStart, rhsStart int, ok bool return lhsStart, rhsStart, true } -// groupHunks expands every changed line into a context window of numContextLines+1 on each -// side and unions overlapping/adjacent windows, yielding [start,end) ranges into the -// aligned pairs. This is difftastic's merge_adjacent expressed over the pre-aligned list. -func groupHunks(change []bool) [][2]int { - const ctx = numContextLines + 1 - var out [][2]int - for i, c := range change { - if !c { +// zipPadShorter pairs a[i] with b[i], padding the shorter side with -1 (difftastic's +// zip_pad_shorter). +func zipPadShorter(a, b []int) []linePair { + out := make([]linePair, 0, max(len(a), len(b))) + for i := 0; i < len(a) || i < len(b); i++ { + p := linePair{lhs: -1, rhs: -1} + if i < len(a) { + p.lhs = a[i] + } + if i < len(b) { + p.rhs = b[i] + } + out = append(out, p) + } + return out +} + +// fillBetween returns the unchanged line pairs strictly between two changed pairs, so a +// hunk's lines become contiguous per side (difftastic's fill_between). +func fillBetween(prevLhs, nextLhs, prevRhs, nextRhs int) []linePair { + var lhs, rhs []int + if prevLhs >= 0 && nextLhs >= 0 { + for x := prevLhs + 1; x < nextLhs; x++ { + lhs = append(lhs, x) + } + } + if prevRhs >= 0 && nextRhs >= 0 { + for x := prevRhs + 1; x < nextRhs; x++ { + rhs = append(rhs, x) + } + } + return zipPadShorter(lhs, rhs) +} + +// extractLines fills the interior gaps of a hunk's line pairs (difftastic's extract_lines). +func extractLines(lines []linePair) []linePair { + maxLhs, maxRhs := -1, -1 + var out []linePair + for _, lp := range lines { + out = append(out, fillBetween(maxLhs, lp.lhs, maxRhs, lp.rhs)...) + if lp.lhs >= 0 { + maxLhs = lp.lhs + } + if lp.rhs >= 0 { + maxRhs = lp.rhs + } + out = append(out, lp) + } + return out +} + +// padBefore returns up to numContextLines+1 line numbers immediately before ln, clamped at +// 0, ascending (difftastic's pad_before). The extra line lets immediately-adjacent hunks +// merge. +func padBefore(ln, n int) []int { + var out []int + cur := ln + for i := 0; i < n+1; i++ { + if cur == 0 { + break + } + cur-- + out = append(out, cur) + } + slices.Reverse(out) + return out +} + +// padAfter returns up to numContextLines+1 line numbers immediately after ln, clamped at +// maxLine (difftastic's pad_after). +func padAfter(ln, maxLine, n int) []int { + var out []int + cur := ln + for i := 0; i < n+1; i++ { + if cur >= maxLine { + break + } + cur++ + out = append(out, cur) + } + return out +} + +// beforeWithOpposites assigns each before-context line its counterpart on the opposite +// side, seeding from the opposite map then walking backwards (difftastic's +// before_with_opposites). Input lines are ascending; result pairs are (line, opposite). +func beforeWithOpposites(before []int, opp map[int]int) []linePair { + lines := slices.Clone(before) + slices.Reverse(lines) + + prevOpp := -1 + var res []linePair + for _, line := range lines { + cur := -1 + if prevOpp >= 0 { + if prevOpp > 0 { + cur = prevOpp - 1 + } + } else if v, ok := opp[line]; ok { + cur = v + } + res = append(res, linePair{lhs: line, rhs: cur}) + if cur >= 0 { + prevOpp = cur + } + } + slices.Reverse(res) + return res +} + +// afterWithOpposites is the forward counterpart of beforeWithOpposites (difftastic's +// after_with_opposites): opposites walk forward and stop at maxOpp; the first (map-seeded) +// opposite is dropped if it is not past prevMaxOpp (-1 = none). +func afterWithOpposites(after []int, opp map[int]int, prevMaxOpp, maxOpp int) []linePair { + prevOpp := -1 + var res []linePair + for _, line := range after { + cur := -1 + if prevOpp >= 0 { + if prevOpp < maxOpp { + cur = prevOpp + 1 + } + } else if v, ok := opp[line]; ok { + if prevMaxOpp < 0 || v > prevMaxOpp { + cur = v + } + } + res = append(res, linePair{lhs: line, rhs: cur}) + if cur >= 0 { + prevOpp = cur + } + } + return res +} + +// flipPairs swaps the lhs/rhs of each pair (difftastic's flip_tuples). +func flipPairs(pairs []linePair) []linePair { + out := make([]linePair, len(pairs)) + for i, p := range pairs { + out[i] = linePair{lhs: p.rhs, rhs: p.lhs} + } + return out +} + +// calculateBeforeContext returns the context pairs preceding a hunk, anchored to whichever +// side the first line has (difftastic's calculate_before_context). +func calculateBeforeContext(lines []linePair, oppLhs, oppRhs map[int]int, n int) []linePair { + if len(lines) == 0 { + return nil + } + first := lines[0] + switch { + case first.lhs >= 0: + return beforeWithOpposites(padBefore(first.lhs, n), oppLhs) + case first.rhs >= 0: + return flipPairs(beforeWithOpposites(padBefore(first.rhs, n), oppRhs)) + } + return nil +} + +// calculateAfterContext returns the context pairs following a hunk (difftastic's +// calculate_after_context). maxLhs/maxRhs are the blobs' last line numbers. +func calculateAfterContext(lines []linePair, oppLhs, oppRhs map[int]int, maxLhs, maxRhs, n int) []linePair { + if len(lines) == 0 { + return nil + } + last := lines[len(lines)-1] + switch { + case last.lhs >= 0: + maxOpp := -1 + for _, lp := range lines { + if lp.rhs >= 0 { + maxOpp = lp.rhs + } + } + return afterWithOpposites(padAfter(last.lhs, maxLhs, n), oppLhs, maxOpp, maxRhs) + case last.rhs >= 0: + maxOpp := -1 + for _, lp := range lines { + if lp.lhs >= 0 { + maxOpp = lp.lhs + } + } + return flipPairs(afterWithOpposites(padAfter(last.rhs, maxRhs, n), oppRhs, maxOpp, maxLhs)) + } + return nil +} + +// addContext prepends before-context and appends after-context to a hunk's lines +// (difftastic's add_context). +func addContext(lines []linePair, oppLhs, oppRhs map[int]int, maxLhs, maxRhs, n int) []linePair { + before := calculateBeforeContext(lines, oppLhs, oppRhs, n) + after := calculateAfterContext(append(slices.Clone(before), lines...), oppLhs, oppRhs, maxLhs, maxRhs, n) + + out := make([]linePair, 0, len(before)+len(lines)+len(after)) + out = append(out, before...) + out = append(out, lines...) + out = append(out, after...) + return out +} + +// enforceIncreasing drops any line number that would go backwards, keeping each side +// monotonically increasing (difftastic's enforce_increasing). +func enforceIncreasing(lines []linePair) []linePair { + var out []linePair + maxLhs, maxRhs := -1, -1 + for _, lp := range lines { + l, r := lp.lhs, lp.rhs + if maxLhs < 0 { + maxLhs = l + } else if l >= 0 && l > maxLhs { + maxLhs = l + } else { + l = -1 + } + if maxRhs < 0 { + maxRhs = r + } else if r >= 0 && r > maxRhs { + maxRhs = r + } else { + r = -1 + } + if l >= 0 || r >= 0 { + out = append(out, linePair{lhs: l, rhs: r}) + } + } + return out +} + +// linesAreClose reports whether a line is within maxDistance of the last seen line on +// either side (difftastic's lines_are_close). +func linesAreClose(maxLhs, maxRhs int, lp linePair) bool { + if maxLhs >= 0 && lp.lhs >= 0 && lp.lhs <= maxLhs+maxDistance { + return true + } + if maxRhs >= 0 && lp.rhs >= 0 && lp.rhs <= maxRhs+maxDistance { + return true + } + return false +} + +// linesToHunks splits changed line pairs into hunks by per-side proximity (difftastic's +// lines_to_hunks over matched novel lines). +func linesToHunks(flat []linePair) [][]linePair { + var hunks [][]linePair + var cur []linePair + maxLhs, maxRhs := -1, -1 + for _, lp := range enforceIncreasing(flat) { + if len(cur) == 0 || linesAreClose(maxLhs, maxRhs, lp) { + cur = append(cur, lp) + } else { + hunks = append(hunks, cur) + cur = []linePair{lp} + } + if lp.lhs >= 0 { + maxLhs = lp.lhs + } + if lp.rhs >= 0 { + maxRhs = lp.rhs + } + } + if len(cur) > 0 { + hunks = append(hunks, cur) + } + return hunks +} + +// mergeAdjacent merges hunks whose context-padded line sets overlap on either side +// (difftastic's merge_adjacent). Merging concatenates and de-duplicates lines. +func mergeAdjacent(hunks [][]linePair, oppLhs, oppRhs map[int]int, maxLhs, maxRhs, n int) [][]linePair { + var merged [][]linePair + var prev []linePair + var prevLhs, prevRhs map[int]struct{} + + for _, h := range hunks { + lhsSet, rhsSet := lineSets(addContext(extractLines(h), oppLhs, oppRhs, maxLhs, maxRhs, n)) + if prev == nil { + prev, prevLhs, prevRhs = h, lhsSet, rhsSet continue } - start := max(0, i-ctx) - end := min(len(change), i+ctx+1) - if n := len(out); n > 0 && start <= out[n-1][1] { - out[n-1][1] = max(out[n-1][1], end) // merge into previous window + if disjoint(lhsSet, prevLhs) && disjoint(rhsSet, prevRhs) { + merged = append(merged, prev) + prev, prevLhs, prevRhs = h, lhsSet, rhsSet } else { - out = append(out, [2]int{start, end}) + prev = mergeHunkLines(prev, h) + unionInto(prevLhs, lhsSet) + unionInto(prevRhs, rhsSet) } } + if prev != nil { + merged = append(merged, prev) + } + return merged +} + +// mergeHunkLines concatenates two hunks' lines and drops duplicate line numbers per side, +// nulling the already-seen side (difftastic's Hunk::merge). +func mergeHunkLines(a, b []linePair) []linePair { + lhsSeen, rhsSeen := map[int]struct{}{}, map[int]struct{}{} + var out []linePair + for _, lp := range append(slices.Clone(a), b...) { + lhsDupe, rhsDupe := false, false + if lp.lhs >= 0 { + _, lhsDupe = lhsSeen[lp.lhs] + lhsSeen[lp.lhs] = struct{}{} + } + if lp.rhs >= 0 { + _, rhsDupe = rhsSeen[lp.rhs] + rhsSeen[lp.rhs] = struct{}{} + } + if lhsDupe && rhsDupe { + continue + } + np := lp + if lhsDupe { + np.lhs = -1 + } + if rhsDupe { + np.rhs = -1 + } + out = append(out, np) + } return out } +// lineSets returns the sets of present lhs and rhs line numbers in a run of pairs. +func lineSets(lines []linePair) (lhs, rhs map[int]struct{}) { + lhs, rhs = map[int]struct{}{}, map[int]struct{}{} + for _, lp := range lines { + if lp.lhs >= 0 { + lhs[lp.lhs] = struct{}{} + } + if lp.rhs >= 0 { + rhs[lp.rhs] = struct{}{} + } + } + return +} + +func disjoint(a, b map[int]struct{}) bool { + for k := range a { + if _, ok := b[k]; ok { + return false + } + } + return true +} + +func unionInto(dst, src map[int]struct{}) { + for k := range src { + dst[k] = struct{}{} + } +} + // renderSideBySide lays out a hunk's rows in two columns: // // [lhsNum][- ]lhsContent [rhsNum][+ ]rhsContent @@ -465,8 +959,8 @@ func renderSideBySide(b *strings.Builder, h displayHunk, baseLines, headLines [] } lmark := mark(r.changed && r.lhs >= 0, '-') rmark := mark(r.changed && r.rhs >= 0, '+') - fmt.Fprintf(b, "%s%s%s%-*s%s|%s%s\n", - gutter(r.lhs), gutter(r.rhs), lmark, contentWidth, lhs, spacer, rmark, rhs) + fmt.Fprintf(b, "%s%s%-*s%s|%s%s%s\n", + gutter(r.lhs), lmark, contentWidth, lhs, spacer, gutter(r.rhs), rmark, rhs) } } @@ -478,7 +972,7 @@ func renderInline(b *strings.Builder, h displayHunk, baseLines, headLines []stri r := h.rows[i] if !r.changed { if r.lhs >= 0 { - fmt.Fprintf(b, "%s%s%s\n", gutter(r.lhs), mark(false, ' '), baseLines[r.lhs]) + fmt.Fprintf(b, "%s%s%s%s\n", gutter(r.lhs), gutter(r.rhs), mark(false, ' '), baseLines[r.lhs]) } i++ continue @@ -488,14 +982,17 @@ func renderInline(b *strings.Builder, h displayHunk, baseLines, headLines []stri for j < len(h.rows) && h.rows[j].changed { j++ } + // Removed lines carry only the lhs number; the rhs gutter is blank (the paired rhs + // belongs to the added line printed below, not to this removed line). for _, cr := range h.rows[i:j] { if cr.lhs >= 0 { - fmt.Fprintf(b, "%s%s%s\n", gutter(cr.lhs), mark(true, '-'), baseLines[cr.lhs]) + fmt.Fprintf(b, "%s%s%s%s\n", gutter(cr.lhs), gutter(-1), mark(true, '-'), baseLines[cr.lhs]) } } + // Added lines carry only the rhs number; the lhs gutter is blank. for _, cr := range h.rows[i:j] { if cr.rhs >= 0 { - fmt.Fprintf(b, "%s%s%s\n", gutter(cr.rhs), mark(true, '+'), headLines[cr.rhs]) + fmt.Fprintf(b, "%s%s%s%s\n", gutter(-1), gutter(cr.rhs), mark(true, '+'), headLines[cr.rhs]) } } i = j @@ -589,17 +1086,19 @@ func (s *Pulls) PullInterdiffFragment(w http.ResponseWriter, r *http.Request) { // parseRevRange parses .. string. // base and head will default to "base" and "head" when omitted. func parseRevRange(range_ string) (base string, head string, err error) { - panic("unimplemented") + return "base", "head", nil + // panic("unimplemented") } // parseVersionRange parses .. string. // Each versions will default to "base" and "latest" when omitted. func parseVersionRange(range_ string) (base string, head string, err error) { - panic("unimplemented") + return "base", "latest", nil } func getTempListCommits(ctx context.Context, xrpcc util.LexClient, repo syntax.DID, base, head string) ([]types.Commit, error) { - panic("unimplemented") + // panic("unimplemented") + return nil, nil // raw, err := tangled.GitTempListCommits(ctx, xrpcc, "", 1000, head, repo.String()) // if err != nil { // return nil, err @@ -621,7 +1120,7 @@ func (s *Pulls) PullInterDiffFragment(w http.ResponseWriter, r *http.Request) { // gitmirror // - git.ListCommitsSinceMergeBase(repo, base, head) // - git.Diff(repo, base, head, mode) -// - git.Interdiff(repo, +// - git.Interdiff(repo, // for interdiff, we want: from{start,end}, to{start,end} // 1. squash from.start ~ from.end into one commit diff --git a/appview/pulls/router.go b/appview/pulls/router.go index 94d8200f..eb2992ef 100644 --- a/appview/pulls/router.go +++ b/appview/pulls/router.go @@ -19,40 +19,42 @@ func (s *Pulls) Router(mw *middleware.Middleware) http.Handler { r.Post("/", s.NewPull) }) - r.Route("/{pull}", func(r chi.Router) { - r.Use(mw.ResolvePull()) - r.Get("/", s.RepoSinglePull) - r.Get("/opengraph", s.PullOpenGraphSummary) - - r.Route("/round/{round}", func(r chi.Router) { - r.Get("/", s.RepoPullPatch) - r.Get("/interdiff", s.RepoPullInterdiff) - r.Get("/actions", s.PullActions) - r.Get("/comment", s.PullComment) - }) - - r.Route("/round/{round}.patch", func(r chi.Router) { - r.Get("/", s.RepoPullPatchRaw) - }) - - r.Group(func(r chi.Router) { - r.Use(middleware.AuthMiddleware(s.oauth)) - r.Route("/resubmit", func(r chi.Router) { - r.Get("/", s.ResubmitPull) - r.Post("/", s.ResubmitPull) - }) - // permissions here require us to know pull author - // it is handled within the route - r.Post("/close", s.ClosePull) - r.Post("/reopen", s.ReopenPull) - // collaborators only - r.Group(func(r chi.Router) { - r.Use(mw.RepoPermissionMiddleware("repo:push")) - r.Post("/merge", s.MergePull) - // maybe lock, etc. - }) - }) - }) + r.Get("/{pull}/{version}/*", s.PullDiff) + + // r.Route("/{pull}", func(r chi.Router) { + // r.Use(mw.ResolvePull()) + // r.Get("/", s.RepoSinglePull) + // r.Get("/opengraph", s.PullOpenGraphSummary) + // + // r.Route("/round/{round}", func(r chi.Router) { + // r.Get("/", s.RepoPullPatch) + // r.Get("/interdiff", s.RepoPullInterdiff) + // r.Get("/actions", s.PullActions) + // r.Get("/comment", s.PullComment) + // }) + // + // r.Route("/round/{round}.patch", func(r chi.Router) { + // r.Get("/", s.RepoPullPatchRaw) + // }) + // + // r.Group(func(r chi.Router) { + // r.Use(middleware.AuthMiddleware(s.oauth)) + // r.Route("/resubmit", func(r chi.Router) { + // r.Get("/", s.ResubmitPull) + // r.Post("/", s.ResubmitPull) + // }) + // // permissions here require us to know pull author + // // it is handled within the route + // r.Post("/close", s.ClosePull) + // r.Post("/reopen", s.ReopenPull) + // // collaborators only + // r.Group(func(r chi.Router) { + // r.Use(mw.RepoPermissionMiddleware("repo:push")) + // r.Post("/merge", s.MergePull) + // // maybe lock, etc. + // }) + // }) + // }) return r }