From 73474c043bfdeeb33cd58bdfe42592fea3083422 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sun, 3 Jan 2016 19:26:46 -0200 Subject: [PATCH 1/4] Highlighting differences of lines in the diff view. --- models/git_diff.go | 85 ++++++++++++++++++++++++++++++++++++ models/git_diff_test.go | 29 ++++++++++++ public/css/gogs.css | 6 +++ public/less/_repository.less | 8 ++++ routers/repo/commit.go | 6 +++ templates/repo/diff_box.tmpl | 6 +-- 6 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 models/git_diff_test.go diff --git a/models/git_diff.go b/models/git_diff.go index 22075ef76..9656813bf 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -13,6 +13,8 @@ import ( "os" "os/exec" "strings" + "html/template" + "html" "github.com/Unknwon/com" "golang.org/x/net/html/charset" @@ -23,6 +25,7 @@ import ( "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/process" + "github.com/sergi/go-diff/diffmatchpatch" ) // Diff line types. @@ -45,6 +48,7 @@ type DiffLine struct { RightIdx int Type int Content string + ParsedContent template.HTML } func (d DiffLine) GetType() int { @@ -56,6 +60,87 @@ type DiffSection struct { Lines []*DiffLine } +func diffToHtml(diffRecord []diffmatchpatch.Diff, lineType int) template.HTML { + result := "" + for _, s := range diffRecord { + if s.Type == diffmatchpatch.DiffInsert && lineType == DIFF_LINE_ADD { + result = result + ""+html.EscapeString(s.Text)+"" + } else if s.Type == diffmatchpatch.DiffDelete && lineType == DIFF_LINE_DEL { + result = result + ""+html.EscapeString(s.Text)+"" + } else if s.Type == diffmatchpatch.DiffEqual { + result = result + html.EscapeString(s.Text) + } + } + return template.HTML(result) +} + +func (diffSection *DiffSection) GetLeftLine(idx int, sliceIdx int) *DiffLine { + for i, diffLine := range diffSection.Lines { + if diffLine.LeftIdx == idx && diffLine.RightIdx == 0 { + // ignore the the lines are too far from each other + if i > sliceIdx-5 && i < sliceIdx+5 { + return diffLine + } else { + return nil + } + } + } + return nil +} + +func (diffSection *DiffSection) GetRightLine(idx int, sliceIdx int) *DiffLine { + for i, diffLine := range diffSection.Lines { + if diffLine.RightIdx == idx && diffLine.LeftIdx == 0 { + // ignore the the lines are too far from each other + if i > sliceIdx-5 && i < sliceIdx+5 { + return diffLine + } else { + return nil + } + } + } + return nil +} + +// computes diff of each diff line and set the HTML on diffLine.ParsedContent +func (diffSection *DiffSection) ComputeLinesDiff() { + for i, diffLine := range diffSection.Lines { + var compareDiffLine *DiffLine + var diff1, diff2 string + + // default content: as is + diffLine.ParsedContent = template.HTML(html.EscapeString(diffLine.Content[1:])) + + // just compute diff for adds and removes + if diffLine.Type != DIFF_LINE_ADD && diffLine.Type != DIFF_LINE_DEL { + continue + } + + // try to find equivalent diff line. ignore, otherwise + if diffLine.Type == DIFF_LINE_ADD { + compareDiffLine = diffSection.GetLeftLine(diffLine.RightIdx, i) + if compareDiffLine == nil { + continue + } + diff1 = compareDiffLine.Content + diff2 = diffLine.Content + } else { + compareDiffLine = diffSection.GetRightLine(diffLine.LeftIdx, i) + if compareDiffLine == nil { + continue + } + diff1 = diffLine.Content + diff2 = compareDiffLine.Content + } + + dmp := diffmatchpatch.New() + diffRecord := dmp.DiffMain(diff1[1:], diff2[1:], true) + diffRecord = dmp.DiffCleanupSemantic(diffRecord) + + diffLine.ParsedContent = diffToHtml(diffRecord, diffLine.Type) + } +} + type DiffFile struct { Name string OldName string diff --git a/models/git_diff_test.go b/models/git_diff_test.go new file mode 100644 index 000000000..4f4d47bf9 --- /dev/null +++ b/models/git_diff_test.go @@ -0,0 +1,29 @@ +package models + +import ( + dmp "github.com/sergi/go-diff/diffmatchpatch" + "html/template" + "testing" +) + +func assertEqual(t *testing.T, s1 string, s2 template.HTML) { + if s1 != string(s2) { + t.Errorf("%s should be equal %s", s2, s1) + } +} + +func TestDiffToHtml(t *testing.T) { + assertEqual(t, "foo bar biz", diffToHtml([]dmp.Diff{ + dmp.Diff{dmp.DiffEqual, "foo "}, + dmp.Diff{dmp.DiffInsert, "bar"}, + dmp.Diff{dmp.DiffDelete, " baz"}, + dmp.Diff{dmp.DiffEqual, " biz"}, + }, DIFF_LINE_ADD)) + + assertEqual(t, "foo bar biz", diffToHtml([]dmp.Diff{ + dmp.Diff{dmp.DiffEqual, "foo "}, + dmp.Diff{dmp.DiffDelete, "bar"}, + dmp.Diff{dmp.DiffInsert, " baz"}, + dmp.Diff{dmp.DiffEqual, " biz"}, + }, DIFF_LINE_DEL)) +} diff --git a/public/css/gogs.css b/public/css/gogs.css index 0160c5d91..fd9359af1 100755 --- a/public/css/gogs.css +++ b/public/css/gogs.css @@ -2764,6 +2764,12 @@ footer .container .links > *:first-child { #delete-repo-modal .ui.message { width: 100%!important; } +.removed-code { + background-color: #ff9999; +} +.added-code { + background-color: #99ff99; +} .organization { padding-top: 15px; padding-bottom: 80px; diff --git a/public/less/_repository.less b/public/less/_repository.less index 441832daf..7bb9c8a94 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -1217,3 +1217,11 @@ width: 100%!important; } } + +.removed-code { + background-color: #ff9999; +} + +.added-code { + background-color: #99ff99; +} diff --git a/routers/repo/commit.go b/routers/repo/commit.go index c3fc4d177..9e46c2e84 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -168,6 +168,12 @@ func Diff(ctx *middleware.Context) { } } + for _, diffFile := range diff.Files { + for _, diffSection := range diffFile.Sections { + diffSection.ComputeLinesDiff() + } + } + ctx.Data["IsSplitStyle"] = ctx.Query("style") == "split" ctx.Data["Username"] = userName ctx.Data["Reponame"] = repoName diff --git a/templates/repo/diff_box.tmpl b/templates/repo/diff_box.tmpl index a12f4de4e..f7851822f 100644 --- a/templates/repo/diff_box.tmpl +++ b/templates/repo/diff_box.tmpl @@ -76,13 +76,13 @@ {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} - 
{{if $line.LeftIdx}}{{$line.Content}}{{end}}
+ 
{{if $line.LeftIdx}}{{$line.ParsedContent}}{{end}}
{{if $line.RightIdx}}{{$line.RightIdx}}{{end}} - 
{{if $line.RightIdx}}{{$line.Content}}{{end}}
+ 
{{if $line.RightIdx}}{{$line.ParsedContent}}{{end}}
{{end}} @@ -104,7 +104,7 @@ {{end}} - 
{{$line.Content}}
+ 
{{$line.ParsedContent}}
{{end}} From 81ed5c4bee112beacd8e0f34b336b4f3b6c6a6c1 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Wed, 6 Jan 2016 18:00:40 -0200 Subject: [PATCH 2/4] Declaring specific types for enums constants. This makes the code more strict since you can't assign or compare values of different types without proper cast. --- models/git_diff.go | 39 +++++++++++++++++++++--------------- templates/repo/diff_box.tmpl | 8 ++++---- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/models/git_diff.go b/models/git_diff.go index 9656813bf..2b9bda253 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -28,31 +28,34 @@ import ( "github.com/sergi/go-diff/diffmatchpatch" ) -// Diff line types. -const ( - DIFF_LINE_PLAIN = iota + 1 - DIFF_LINE_ADD - DIFF_LINE_DEL - DIFF_LINE_SECTION -) +type DiffLineType uint8 const ( - DIFF_FILE_ADD = iota + 1 - DIFF_FILE_CHANGE - DIFF_FILE_DEL - DIFF_FILE_RENAME + DIFF_LINE_PLAIN DiffLineType = iota + 1 + DIFF_LINE_ADD DiffLineType = iota + 1 + DIFF_LINE_DEL DiffLineType = iota + 1 + DIFF_LINE_SECTION DiffLineType = iota + 1 +) + +type DiffFileType uint8 + +const ( + DIFF_FILE_ADD DiffFileType = iota + 1 + DIFF_FILE_CHANGE DiffFileType = iota + 1 + DIFF_FILE_DEL DiffFileType = iota + 1 + DIFF_FILE_RENAME DiffFileType = iota + 1 ) type DiffLine struct { LeftIdx int RightIdx int - Type int + Type DiffLineType Content string ParsedContent template.HTML } -func (d DiffLine) GetType() int { - return d.Type +func (d *DiffLine) GetType() int { + return int(d.Type) } type DiffSection struct { @@ -60,7 +63,7 @@ type DiffSection struct { Lines []*DiffLine } -func diffToHtml(diffRecord []diffmatchpatch.Diff, lineType int) template.HTML { +func diffToHtml(diffRecord []diffmatchpatch.Diff, lineType DiffLineType) template.HTML { result := "" for _, s := range diffRecord { if s.Type == diffmatchpatch.DiffInsert && lineType == DIFF_LINE_ADD { @@ -146,7 +149,7 @@ type DiffFile struct { OldName string Index int Addition, Deletion int - Type int + Type DiffFileType IsCreated bool IsDeleted bool IsBin bool @@ -154,6 +157,10 @@ type DiffFile struct { Sections []*DiffSection } +func (diffFile *DiffFile) GetType() int { + return int(diffFile.Type) +} + type Diff struct { TotalAddition, TotalDeletion int Files []*DiffFile diff --git a/templates/repo/diff_box.tmpl b/templates/repo/diff_box.tmpl index f7851822f..a3245d1c2 100644 --- a/templates/repo/diff_box.tmpl +++ b/templates/repo/diff_box.tmpl @@ -26,7 +26,7 @@ {{end}} -   +   {{.Name}} {{end}} @@ -71,7 +71,7 @@ {{if $.IsSplitStyle}} {{range $j, $section := .Sections}} {{range $k, $line := .Lines}} - + {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} @@ -90,8 +90,8 @@ {{else}} {{range $j, $section := .Sections}} {{range $k, $line := .Lines}} - - {{if eq .Type 4}} + + {{if eq .GetType 4}} {{if gt $j 0}}{{end}} From bf11ad19eab0de5aef297a21ed58c6cd811a137c Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Thu, 7 Jan 2016 11:27:35 -0200 Subject: [PATCH 3/4] Semantic fixes. --- models/git_diff.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/models/git_diff.go b/models/git_diff.go index 2b9bda253..ec1fc1890 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -19,31 +19,31 @@ import ( "github.com/Unknwon/com" "golang.org/x/net/html/charset" "golang.org/x/text/transform" + "github.com/sergi/go-diff/diffmatchpatch" "github.com/gogits/git-module" "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/process" - "github.com/sergi/go-diff/diffmatchpatch" ) type DiffLineType uint8 const ( - DIFF_LINE_PLAIN DiffLineType = iota + 1 - DIFF_LINE_ADD DiffLineType = iota + 1 - DIFF_LINE_DEL DiffLineType = iota + 1 - DIFF_LINE_SECTION DiffLineType = iota + 1 + DIFF_LINE_PLAIN DiffLineType = iota + 1 + DIFF_LINE_ADD + DIFF_LINE_DEL + DIFF_LINE_SECTION ) type DiffFileType uint8 const ( - DIFF_FILE_ADD DiffFileType = iota + 1 - DIFF_FILE_CHANGE DiffFileType = iota + 1 - DIFF_FILE_DEL DiffFileType = iota + 1 - DIFF_FILE_RENAME DiffFileType = iota + 1 + DIFF_FILE_ADD DiffFileType = iota + 1 + DIFF_FILE_CHANGE + DIFF_FILE_DEL + DIFF_FILE_RENAME ) type DiffLine struct { @@ -80,7 +80,7 @@ func diffToHtml(diffRecord []diffmatchpatch.Diff, lineType DiffLineType) templat func (diffSection *DiffSection) GetLeftLine(idx int, sliceIdx int) *DiffLine { for i, diffLine := range diffSection.Lines { if diffLine.LeftIdx == idx && diffLine.RightIdx == 0 { - // ignore the the lines are too far from each other + // ignore if the lines are too far from each other if i > sliceIdx-5 && i < sliceIdx+5 { return diffLine } else { @@ -94,7 +94,7 @@ func (diffSection *DiffSection) GetLeftLine(idx int, sliceIdx int) *DiffLine { func (diffSection *DiffSection) GetRightLine(idx int, sliceIdx int) *DiffLine { for i, diffLine := range diffSection.Lines { if diffLine.RightIdx == idx && diffLine.LeftIdx == 0 { - // ignore the the lines are too far from each other + // ignore if the lines are too far from each other if i > sliceIdx-5 && i < sliceIdx+5 { return diffLine } else { From 697b0e2aba454325d5b70736823440e1db2c40ba Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Fri, 8 Jan 2016 10:50:25 -0200 Subject: [PATCH 4/4] Fix: now highlights in diff view are getting the correct lines. --- models/git_diff.go | 42 +++++++++++++++++++---------------------- models/git_diff_test.go | 41 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 23 deletions(-) diff --git a/models/git_diff.go b/models/git_diff.go index ec1fc1890..bfbc9c73a 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -77,28 +77,24 @@ func diffToHtml(diffRecord []diffmatchpatch.Diff, lineType DiffLineType) templat return template.HTML(result) } -func (diffSection *DiffSection) GetLeftLine(idx int, sliceIdx int) *DiffLine { - for i, diffLine := range diffSection.Lines { - if diffLine.LeftIdx == idx && diffLine.RightIdx == 0 { - // ignore if the lines are too far from each other - if i > sliceIdx-5 && i < sliceIdx+5 { - return diffLine - } else { - return nil - } - } - } - return nil -} +// get an specific line by type (add or del) and file line number +func (diffSection *DiffSection) GetLine(lineType DiffLineType, idx int) *DiffLine { + difference := 0 -func (diffSection *DiffSection) GetRightLine(idx int, sliceIdx int) *DiffLine { - for i, diffLine := range diffSection.Lines { - if diffLine.RightIdx == idx && diffLine.LeftIdx == 0 { - // ignore if the lines are too far from each other - if i > sliceIdx-5 && i < sliceIdx+5 { + for _, diffLine := range diffSection.Lines { + if diffLine.Type == DIFF_LINE_PLAIN { + // get the difference of line numbers between ADD and DEL versions + difference = diffLine.RightIdx - diffLine.LeftIdx + continue + } + + if lineType == DIFF_LINE_DEL { + if diffLine.RightIdx == 0 && diffLine.LeftIdx == idx - difference { + return diffLine + } + } else if lineType == DIFF_LINE_ADD { + if diffLine.LeftIdx == 0 && diffLine.RightIdx == idx + difference { return diffLine - } else { - return nil } } } @@ -107,7 +103,7 @@ func (diffSection *DiffSection) GetRightLine(idx int, sliceIdx int) *DiffLine { // computes diff of each diff line and set the HTML on diffLine.ParsedContent func (diffSection *DiffSection) ComputeLinesDiff() { - for i, diffLine := range diffSection.Lines { + for _, diffLine := range diffSection.Lines { var compareDiffLine *DiffLine var diff1, diff2 string @@ -121,14 +117,14 @@ func (diffSection *DiffSection) ComputeLinesDiff() { // try to find equivalent diff line. ignore, otherwise if diffLine.Type == DIFF_LINE_ADD { - compareDiffLine = diffSection.GetLeftLine(diffLine.RightIdx, i) + compareDiffLine = diffSection.GetLine(DIFF_LINE_DEL, diffLine.RightIdx) if compareDiffLine == nil { continue } diff1 = compareDiffLine.Content diff2 = diffLine.Content } else { - compareDiffLine = diffSection.GetRightLine(diffLine.LeftIdx, i) + compareDiffLine = diffSection.GetLine(DIFF_LINE_ADD, diffLine.LeftIdx) if compareDiffLine == nil { continue } diff --git a/models/git_diff_test.go b/models/git_diff_test.go index 4f4d47bf9..a7b9bdd62 100644 --- a/models/git_diff_test.go +++ b/models/git_diff_test.go @@ -12,6 +12,12 @@ func assertEqual(t *testing.T, s1 string, s2 template.HTML) { } } +func assertLineEqual(t *testing.T, d1 *DiffLine, d2 *DiffLine) { + if d1 != d2 { + t.Errorf("%v should be equal %v", d1, d2) + } +} + func TestDiffToHtml(t *testing.T) { assertEqual(t, "foo bar biz", diffToHtml([]dmp.Diff{ dmp.Diff{dmp.DiffEqual, "foo "}, @@ -27,3 +33,38 @@ func TestDiffToHtml(t *testing.T) { dmp.Diff{dmp.DiffEqual, " biz"}, }, DIFF_LINE_DEL)) } + +// test if GetLine is return the correct lines +func TestGetLine(t *testing.T) { + ds := DiffSection{Lines: []*DiffLine{ + &DiffLine{LeftIdx: 28, RightIdx: 28, Type: DIFF_LINE_PLAIN}, + &DiffLine{LeftIdx: 29, RightIdx: 29, Type: DIFF_LINE_PLAIN}, + &DiffLine{LeftIdx: 30, RightIdx: 30, Type: DIFF_LINE_PLAIN}, + &DiffLine{LeftIdx: 31, RightIdx: 0, Type: DIFF_LINE_DEL}, + &DiffLine{LeftIdx: 0, RightIdx: 31, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 0, RightIdx: 32, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 32, RightIdx: 33, Type: DIFF_LINE_PLAIN}, + &DiffLine{LeftIdx: 33, RightIdx: 0, Type: DIFF_LINE_DEL}, + &DiffLine{LeftIdx: 34, RightIdx: 0, Type: DIFF_LINE_DEL}, + &DiffLine{LeftIdx: 35, RightIdx: 0, Type: DIFF_LINE_DEL}, + &DiffLine{LeftIdx: 36, RightIdx: 0, Type: DIFF_LINE_DEL}, + &DiffLine{LeftIdx: 0, RightIdx: 34, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 0, RightIdx: 35, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 0, RightIdx: 36, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 0, RightIdx: 37, Type: DIFF_LINE_ADD}, + &DiffLine{LeftIdx: 37, RightIdx: 38, Type: DIFF_LINE_PLAIN}, + &DiffLine{LeftIdx: 38, RightIdx: 39, Type: DIFF_LINE_PLAIN}, + }} + + assertLineEqual(t, ds.GetLine(DIFF_LINE_ADD, 31), ds.Lines[4]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_DEL, 31), ds.Lines[3]) + + assertLineEqual(t, ds.GetLine(DIFF_LINE_ADD, 33), ds.Lines[11]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_ADD, 34), ds.Lines[12]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_ADD, 35), ds.Lines[13]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_ADD, 36), ds.Lines[14]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_DEL, 34), ds.Lines[7]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_DEL, 35), ds.Lines[8]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_DEL, 36), ds.Lines[9]) + assertLineEqual(t, ds.GetLine(DIFF_LINE_DEL, 37), ds.Lines[10]) +}