Skip to content

Commit f4f1a87

Browse files
pjbgfpjbgf
authored
Merge pull request #971 from nodivbyzero/fix-177-diff-print-file-stats
plumbing: no panic in printStats function. Fixes #177
2 parents ca05e2c + d058d58 commit f4f1a87

3 files changed

Lines changed: 152 additions & 55 deletions

File tree

‎plumbing/object/commit_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ func (s *SuiteCommit) TestStat(c *C) {
455455
c.Assert(fileStats[1].Name, Equals, "php/crappy.php")
456456
c.Assert(fileStats[1].Addition, Equals, 259)
457457
c.Assert(fileStats[1].Deletion, Equals, 0)
458-
c.Assert(fileStats[1].String(), Equals, " php/crappy.php | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n")
458+
c.Assert(fileStats[1].String(), Equals, " php/crappy.php | 259 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n")
459459
}
460460

461461
func (s *SuiteCommit) TestVerify(c *C) {

‎plumbing/object/patch.go‎

Lines changed: 41 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"io"
9-
"math"
9+
"strconv"
1010
"strings"
1111

1212
"github.com/go-git/go-git/v5/plumbing"
@@ -234,69 +234,56 @@ func (fileStats FileStats) String() string {
234234
return printStat(fileStats)
235235
}
236236

237+
// printStat prints the stats of changes in content of files.
238+
// Original implementation: https://github.com/git/git/blob/1a87c842ece327d03d08096395969aca5e0a6996/diff.c#L2615
239+
// Parts of the output:
240+
// <pad><filename><pad>|<pad><changeNumber><pad><+++/---><newline>
241+
// example: " main.go | 10 +++++++--- "
237242
func printStat(fileStats []FileStat) string {
238-
padLength := float64(len(" "))
239-
newlineLength := float64(len("\n"))
240-
separatorLength := float64(len("|"))
241-
// Soft line length limit. The text length calculation below excludes
242-
// length of the change number. Adding that would take it closer to 80,
243-
// but probably not more than 80, until it's a huge number.
244-
lineLength := 72.0
245-
246-
// Get the longest filename and longest total change.
247-
var longestLength float64
248-
var longestTotalChange float64
249-
for _, fs := range fileStats {
250-
if int(longestLength) < len(fs.Name) {
251-
longestLength = float64(len(fs.Name))
252-
}
253-
totalChange := fs.Addition + fs.Deletion
254-
if int(longestTotalChange) < totalChange {
255-
longestTotalChange = float64(totalChange)
256-
}
257-
}
258-
259-
// Parts of the output:
260-
// <pad><filename><pad>|<pad><changeNumber><pad><+++/---><newline>
261-
// example: " main.go | 10 +++++++--- "
262-
263-
// <pad><filename><pad>
264-
leftTextLength := padLength + longestLength + padLength
265-
266-
// <pad><number><pad><+++++/-----><newline>
267-
// Excluding number length here.
268-
rightTextLength := padLength + padLength + newlineLength
243+
maxGraphWidth := uint(53)
244+
maxNameLen := 0
245+
maxChangeLen := 0
269246

270-
totalTextArea := leftTextLength + separatorLength + rightTextLength
271-
heightOfHistogram := lineLength - totalTextArea
247+
scaleLinear := func(it, width, max uint) uint {
248+
if it == 0 || max == 0 {
249+
return 0
250+
}
272251

273-
// Scale the histogram.
274-
var scaleFactor float64
275-
if longestTotalChange > heightOfHistogram {
276-
// Scale down to heightOfHistogram.
277-
scaleFactor = longestTotalChange / heightOfHistogram
278-
} else {
279-
scaleFactor = 1.0
252+
return 1 + (it * (width - 1) / max)
280253
}
281254

282-
finalOutput := ""
283255
for _, fs := range fileStats {
284-
addn := float64(fs.Addition)
285-
deln := float64(fs.Deletion)
286-
addc := int(math.Floor(addn/scaleFactor))
287-
delc := int(math.Floor(deln/scaleFactor))
288-
if addc < 0 {
289-
addc = 0
256+
if len(fs.Name) > maxNameLen {
257+
maxNameLen = len(fs.Name)
290258
}
291-
if delc < 0 {
292-
delc = 0
259+
260+
changes := strconv.Itoa(fs.Addition + fs.Deletion)
261+
if len(changes) > maxChangeLen {
262+
maxChangeLen = len(changes)
293263
}
294-
adds := strings.Repeat("+", addc)
295-
dels := strings.Repeat("-", delc)
296-
finalOutput += fmt.Sprintf(" %s | %d %s%s\n", fs.Name, (fs.Addition + fs.Deletion), adds, dels)
297264
}
298265

299-
return finalOutput
266+
result := ""
267+
for _, fs := range fileStats {
268+
add := uint(fs.Addition)
269+
del := uint(fs.Deletion)
270+
np := maxNameLen - len(fs.Name)
271+
cp := maxChangeLen - len(strconv.Itoa(fs.Addition+fs.Deletion))
272+
273+
total := add + del
274+
if total > maxGraphWidth {
275+
add = scaleLinear(add, maxGraphWidth, total)
276+
del = scaleLinear(del, maxGraphWidth, total)
277+
}
278+
279+
adds := strings.Repeat("+", int(add))
280+
dels := strings.Repeat("-", int(del))
281+
namePad := strings.Repeat(" ", np)
282+
changePad := strings.Repeat(" ", cp)
283+
284+
result += fmt.Sprintf(" %s%s | %s%d %s%s\n", fs.Name, namePad, changePad, total, adds, dels)
285+
}
286+
return result
300287
}
301288

302289
func getFileStatsFromFilePatches(filePatches []fdiff.FilePatch) FileStats {

‎plumbing/object/patch_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,113 @@ func (s *PatchSuite) TestStatsWithSubmodules(c *C) {
4545
c.Assert(err, IsNil)
4646
c.Assert(p, NotNil)
4747
}
48+
49+
func (s *PatchSuite) TestFileStatsString(c *C) {
50+
testCases := []struct {
51+
description string
52+
input FileStats
53+
expected string
54+
}{
55+
56+
{
57+
description: "no files changed",
58+
input: []FileStat{},
59+
expected: "",
60+
},
61+
{
62+
description: "one file touched - no changes",
63+
input: []FileStat{
64+
{
65+
Name: "file1",
66+
},
67+
},
68+
expected: " file1 | 0 \n",
69+
},
70+
{
71+
description: "one file changed",
72+
input: []FileStat{
73+
{
74+
Name: "file1",
75+
Addition: 1,
76+
},
77+
},
78+
expected: " file1 | 1 +\n",
79+
},
80+
{
81+
description: "one file changed with one addition and one deletion",
82+
input: []FileStat{
83+
{
84+
Name: ".github/workflows/git.yml",
85+
Addition: 1,
86+
Deletion: 1,
87+
},
88+
},
89+
expected: " .github/workflows/git.yml | 2 +-\n",
90+
},
91+
{
92+
description: "two files changed",
93+
input: []FileStat{
94+
{
95+
Name: ".github/workflows/git.yml",
96+
Addition: 1,
97+
Deletion: 1,
98+
},
99+
{
100+
Name: "cli/go-git/go.mod",
101+
Addition: 4,
102+
Deletion: 4,
103+
},
104+
},
105+
expected: " .github/workflows/git.yml | 2 +-\n cli/go-git/go.mod | 8 ++++----\n",
106+
},
107+
{
108+
description: "three files changed",
109+
input: []FileStat{
110+
{
111+
Name: ".github/workflows/git.yml",
112+
Addition: 3,
113+
Deletion: 3,
114+
},
115+
{
116+
Name: "worktree.go",
117+
Addition: 107,
118+
},
119+
{
120+
Name: "worktree_test.go",
121+
Addition: 75,
122+
},
123+
},
124+
expected: " .github/workflows/git.yml | 6 +++---\n" +
125+
" worktree.go | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n" +
126+
" worktree_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n",
127+
},
128+
{
129+
description: "three files changed with deletions and additions",
130+
input: []FileStat{
131+
{
132+
Name: ".github/workflows/git.yml",
133+
Addition: 3,
134+
Deletion: 3,
135+
},
136+
{
137+
Name: "worktree.go",
138+
Addition: 107,
139+
Deletion: 217,
140+
},
141+
{
142+
Name: "worktree_test.go",
143+
Addition: 75,
144+
Deletion: 275,
145+
},
146+
},
147+
expected: " .github/workflows/git.yml | 6 +++---\n" +
148+
" worktree.go | 324 ++++++++++++++++++-----------------------------------\n" +
149+
" worktree_test.go | 350 ++++++++++++-----------------------------------------\n",
150+
},
151+
}
152+
153+
for _, tc := range testCases {
154+
c.Log("Executing test cases:", tc.description)
155+
c.Assert(printStat(tc.input), Equals, tc.expected)
156+
}
157+
}

0 commit comments

Comments
 (0)