diff --git a/appview/pulls/opengraph.go b/appview/pulls/opengraph.go --- a/appview/pulls/opengraph.go +++ b/appview/pulls/opengraph.go @@ -18,7 +18,7 @@ "tangled.org/core/patchutil" "tangled.org/core/types" ) -func (s *Pulls) drawPullSummaryCard(pull *models.Pull, repo *models.Repo, commentCount int, diffStats types.DiffStat, filesChanged int) (*ogcard.Card, error) { +func (s *Pulls) drawPullSummaryCard(pull *models.Pull, repo *models.Repo, commentCount int, diffStats types.DiffFileStat, filesChanged int) (*ogcard.Card, error) { width, height := ogcard.DefaultSize() mainCard, err := ogcard.NewCard(width, height) if err != nil { @@ -284,7 +284,7 @@ } commentCount := len(comments) // Calculate diff stats from latest submission using patchutil - var diffStats types.DiffStat + var diffStats types.DiffFileStat filesChanged := 0 if len(pull.Submissions) > 0 { latestSubmission := pull.Submissions[len(pull.Submissions)-1] diff --git a/knotserver/git/diff.go b/knotserver/git/diff.go --- a/knotserver/git/diff.go +++ b/knotserver/git/diff.go @@ -64,19 +64,14 @@ ndiff.IsRename = d.IsRename for _, tf := range d.TextFragments { ndiff.TextFragments = append(ndiff.TextFragments, *tf) - for _, l := range tf.Lines { - switch l.Op { - case gitdiff.OpAdd: - nd.Stat.Insertions += 1 - case gitdiff.OpDelete: - nd.Stat.Deletions += 1 - } - } + nd.Stat.Insertions += tf.LinesAdded + nd.Stat.Deletions += tf.LinesDeleted } nd.Diff = append(nd.Diff, ndiff) } + nd.Stat.FilesChanged += len(diffs) nd.Commit.FromGoGitCommit(c) return &nd, nil diff --git a/patchutil/interdiff.go b/patchutil/interdiff.go --- a/patchutil/interdiff.go +++ b/patchutil/interdiff.go @@ -5,6 +5,7 @@ "fmt" "strings" "github.com/bluekeyes/go-gitdiff/gitdiff" + "tangled.org/core/appview/filetree" "tangled.org/core/types" ) @@ -12,12 +13,34 @@ type InterdiffResult struct { Files []*InterdiffFile } -func (i *InterdiffResult) AffectedFiles() []string { - files := make([]string, len(i.Files)) - for _, f := range i.Files { - files = append(files, f.Name) +func (i *InterdiffResult) Stats() types.DiffStat { + var ins, del int64 + for _, s := range i.ChangedFiles() { + stat := s.Stats() + ins += stat.Insertions + del += stat.Deletions + } + return types.DiffStat{ + Insertions: ins, + Deletions: del, + FilesChanged: len(i.Files), } - return files +} + +func (i *InterdiffResult) ChangedFiles() []types.DiffFileRenderer { + drs := make([]types.DiffFileRenderer, len(i.Files)) + for i, s := range i.Files { + drs[i] = s + } + return drs +} + +func (i *InterdiffResult) FileTree() *filetree.FileTreeNode { + fs := make([]string, len(i.Files)) + for i, s := range i.Files { + fs[i] = s.Name + } + return filetree.FileTree(fs) } func (i *InterdiffResult) String() string { @@ -36,7 +59,11 @@ Name string Status InterdiffFileStatus } -func (s *InterdiffFile) Split() *types.SplitDiff { +func (s *InterdiffFile) Id() string { + return s.Name +} + +func (s *InterdiffFile) Split() types.SplitDiff { fragments := make([]types.SplitFragment, len(s.TextFragments)) for i, fragment := range s.TextFragments { @@ -49,15 +76,44 @@ RightLines: rightLines, } } - return &types.SplitDiff{ + return types.SplitDiff{ Name: s.Id(), TextFragments: fragments, } } -// used by html elements as a unique ID for hrefs -func (s *InterdiffFile) Id() string { - return s.Name +func (s *InterdiffFile) CanRender() string { + if s.Status.IsUnchanged() { + return "This file has not been changed." + } else if s.Status.IsRebased() { + return "This patch was likely rebased, as context lines do not match." + } else if s.Status.IsError() { + return "Failed to calculate interdiff for this file." + } else { + return "" + } +} + +func (s *InterdiffFile) Names() types.DiffFileName { + var n types.DiffFileName + n.New = s.Name + return n +} + +func (s *InterdiffFile) Stats() types.DiffFileStat { + var ins, del int64 + + if s.File != nil { + for _, f := range s.TextFragments { + ins += f.LinesAdded + del += f.LinesDeleted + } + } + + return types.DiffFileStat{ + Insertions: ins, + Deletions: del, + } } func (s *InterdiffFile) String() string { diff --git a/patchutil/patchutil_test.go b/patchutil/patchutil_test.go --- a/patchutil/patchutil_test.go +++ b/patchutil/patchutil_test.go @@ -4,6 +4,8 @@ import ( "errors" "reflect" "testing" + + "tangled.org/core/types" ) func TestIsPatchValid(t *testing.T) { @@ -323,3 +325,10 @@ } }) } } + +func TestImplsInterfaces(t *testing.T) { + id := &InterdiffResult{} + _ = isDiffsRenderer(id) +} + +func isDiffsRenderer[S types.DiffRenderer](S) bool { return true } diff --git a/types/diff.go b/types/diff.go --- a/types/diff.go +++ b/types/diff.go @@ -1,16 +1,31 @@ package types import ( + "net/url" + "github.com/bluekeyes/go-gitdiff/gitdiff" + "tangled.org/core/appview/filetree" ) type DiffOpts struct { Split bool `json:"split"` } -type TextFragment struct { - Header string `json:"comment"` - Lines []gitdiff.Line `json:"lines"` +func (d DiffOpts) Encode() string { + values := make(url.Values) + if d.Split { + values.Set("diff", "split") + } else { + values.Set("diff", "unified") + } + return values.Encode() +} + +// A nicer git diff representation. +type NiceDiff struct { + Commit Commit `json:"commit"` + Stat DiffStat `json:"stat"` + Diff []Diff `json:"diff"` } type Diff struct { @@ -26,13 +41,8 @@ IsCopy bool `json:"is_copy"` IsRename bool `json:"is_rename"` } -type DiffStat struct { - Insertions int64 - Deletions int64 -} - -func (d *Diff) Stats() DiffStat { - var stats DiffStat +func (d Diff) Stats() DiffFileStat { + var stats DiffFileStat for _, f := range d.TextFragments { stats.Insertions += f.LinesAdded stats.Deletions += f.LinesDeleted @@ -40,15 +50,15 @@ } return stats } -// A nicer git diff representation. -type NiceDiff struct { - Commit Commit `json:"commit"` - Stat struct { - FilesChanged int `json:"files_changed"` - Insertions int `json:"insertions"` - Deletions int `json:"deletions"` - } `json:"stat"` - Diff []Diff `json:"diff"` +type DiffStat struct { + Insertions int64 `json:"insertions"` + Deletions int64 `json:"deletions"` + FilesChanged int `json:"files_changed"` +} + +type DiffFileStat struct { + Insertions int64 + Deletions int64 } type DiffTree struct { @@ -58,29 +68,67 @@ Patch string `json:"patch"` Diff []*gitdiff.File `json:"diff"` } -func (d *NiceDiff) ChangedFiles() []string { - files := make([]string, len(d.Diff)) +type DiffFileName struct { + Old string + New string +} - for i, f := range d.Diff { - if f.IsDelete { - files[i] = f.Name.Old +func (d NiceDiff) ChangedFiles() []DiffFileRenderer { + drs := make([]DiffFileRenderer, len(d.Diff)) + for i, s := range d.Diff { + drs[i] = s + } + return drs +} + +func (d NiceDiff) FileTree() *filetree.FileTreeNode { + fs := make([]string, len(d.Diff)) + for i, s := range d.Diff { + n := s.Names() + if n.New == "" { + fs[i] = n.Old } else { - files[i] = f.Name.New + fs[i] = n.New } } + return filetree.FileTree(fs) +} - return files +func (d NiceDiff) Stats() DiffStat { + return d.Stat } -// used by html elements as a unique ID for hrefs -func (d *Diff) Id() string { +func (d Diff) Id() string { if d.IsDelete { return d.Name.Old } return d.Name.New } -func (d *Diff) Split() *SplitDiff { +func (d Diff) Names() DiffFileName { + var n DiffFileName + if d.IsDelete { + n.Old = d.Name.Old + return n + } else if d.IsCopy || d.IsRename { + n.Old = d.Name.Old + n.New = d.Name.New + return n + } else { + n.New = d.Name.New + return n + } +} + +func (d Diff) CanRender() string { + if d.IsBinary { + return "This is a binary file and will not be displayed." + } + + return "" +} + +func (d Diff) Split() SplitDiff { fragments := make([]SplitFragment, len(d.TextFragments)) for i, fragment := range d.TextFragments { leftLines, rightLines := SeparateLines(&fragment) @@ -91,7 +139,7 @@ RightLines: rightLines, } } - return &SplitDiff{ + return SplitDiff{ Name: d.Id(), TextFragments: fragments, } diff --git a/types/diff_test.go b/types/diff_test.go --- a/types/diff_test.go +++ b/types/diff_test.go @@ -1,6 +1,8 @@ package types -import "testing" +import ( + "testing" +) func TestDiffId(t *testing.T) { tests := []struct { @@ -105,8 +107,15 @@ t.Fatalf("ChangedFiles() returned %d items, want %d", len(changedFiles), len(nd.Diff)) } for i, diff := range nd.Diff { - if changedFiles[i] != diff.Id() { + if changedFiles[i].Id() != diff.Id() { t.Errorf("ChangedFiles()[%d] = %q, but Diff.Id() = %q", i, changedFiles[i], diff.Id()) } } } + +func TestImplsInterfaces(t *testing.T) { + nd := NiceDiff{} + _ = isDiffsRenderer(nd) +} + +func isDiffsRenderer[S DiffRenderer](S) bool { return true } diff --git a/types/split.go b/types/split.go --- a/types/split.go +++ b/types/split.go @@ -22,8 +22,7 @@ Name string `json:"name"` TextFragments []SplitFragment `json:"fragments"` } -// used by html elements as a unique ID for hrefs -func (d *SplitDiff) Id() string { +func (d SplitDiff) Id() string { return d.Name }