Skip to content

Commit f92a0b6

Browse files
saithotechknowlogick
saitho
authored and
techknowlogick
committed
Bugfix for image compare and minor improvements to image compare (#8289)
* Resolve error when comparing images Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Check blob existence instead of git-ls when checking if file exists Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Show file metadata also when a file was newly added Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Fixes error in commit view Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Excludes assigning path and image infos for compare routers to service package Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Removes nil default and fixes import order Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Adds missing comments Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Moves methods for assigning compare data to context into repo router package Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com> * Show image compare for deleted images as well. Simplify check if image should be displayed Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
1 parent de8a0a3 commit f92a0b6

File tree

6 files changed

+106
-91
lines changed

6 files changed

+106
-91
lines changed

‎modules/git/commit.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,11 @@ func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, erro
355355
// HasFile returns true if the file given exists on this commit
356356
// This does only mean it's there - it does not mean the file was changed during the commit.
357357
func (c *Commit) HasFile(filename string) (bool, error) {
358-
result, err := c.repo.LsFiles(filename)
359-
return result[0] == filename, err
358+
_, err := c.GetBlobByPath(filename)
359+
if err != nil {
360+
return false, err
361+
}
362+
return true, nil
360363
}
361364

362365
// GetSubModules get all the sub modules of current revision git tree

‎routers/repo/commit.go

+6-18
Original file line numberDiff line numberDiff line change
@@ -239,33 +239,25 @@ func Diff(ctx *context.Context) {
239239
ctx.Data["CommitID"] = commitID
240240
ctx.Data["Username"] = userName
241241
ctx.Data["Reponame"] = repoName
242-
ctx.Data["IsImageFile"] = commit.IsImageFile
243-
ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
244-
result, err := commit.ImageInfo(name)
245-
if err != nil {
246-
log.Error("ImageInfo failed: %v", err)
247-
return nil
248-
}
249-
return result
250-
}
251-
ctx.Data["ImageInfoBase"] = ctx.Data["ImageInfo"]
242+
243+
var parentCommit *git.Commit
252244
if commit.ParentCount() > 0 {
253-
parentCommit, err := ctx.Repo.GitRepo.GetCommit(parents[0])
245+
parentCommit, err = ctx.Repo.GitRepo.GetCommit(parents[0])
254246
if err != nil {
255247
ctx.NotFound("GetParentCommit", err)
256248
return
257249
}
258-
ctx.Data["ImageInfo"] = parentCommit.ImageInfo
259250
}
251+
setImageCompareContext(ctx, parentCommit, commit)
252+
headTarget := path.Join(userName, repoName)
253+
setPathsCompareContext(ctx, parentCommit, commit, headTarget)
260254
ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
261255
ctx.Data["Commit"] = commit
262256
ctx.Data["Verification"] = models.ParseCommitWithSignature(commit)
263257
ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
264258
ctx.Data["Diff"] = diff
265259
ctx.Data["Parents"] = parents
266260
ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
267-
ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "src", "commit", commitID)
268-
ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", commitID)
269261

270262
note := &git.Note{}
271263
err = git.GetNote(ctx.Repo.GitRepo, commitID, note)
@@ -275,10 +267,6 @@ func Diff(ctx *context.Context) {
275267
ctx.Data["NoteAuthor"] = models.ValidateCommitWithEmail(note.Commit)
276268
}
277269

278-
if commit.ParentCount() > 0 {
279-
ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "src", "commit", parents[0])
280-
ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", parents[0])
281-
}
282270
ctx.Data["BranchName"], err = commit.GetBranchName()
283271
if err != nil {
284272
ctx.ServerError("commit.GetBranchName", err)

‎routers/repo/compare.go

+42-35
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package repo
66

77
import (
8+
"fmt"
89
"path"
910
"strings"
1011

@@ -21,6 +22,45 @@ const (
2122
tplCompare base.TplName = "repo/diff/compare"
2223
)
2324

25+
// setPathsCompareContext sets context data for source and raw paths
26+
func setPathsCompareContext(ctx *context.Context, base *git.Commit, head *git.Commit, headTarget string) {
27+
sourcePath := setting.AppSubURL + "/%s/src/commit/%s"
28+
rawPath := setting.AppSubURL + "/%s/raw/commit/%s"
29+
30+
ctx.Data["SourcePath"] = fmt.Sprintf(sourcePath, headTarget, head.ID)
31+
ctx.Data["RawPath"] = fmt.Sprintf(rawPath, headTarget, head.ID)
32+
if base != nil {
33+
baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
34+
ctx.Data["BeforeSourcePath"] = fmt.Sprintf(sourcePath, baseTarget, base.ID)
35+
ctx.Data["BeforeRawPath"] = fmt.Sprintf(rawPath, baseTarget, base.ID)
36+
}
37+
}
38+
39+
// setImageCompareContext sets context data that is required by image compare template
40+
func setImageCompareContext(ctx *context.Context, base *git.Commit, head *git.Commit) {
41+
ctx.Data["IsImageFileInHead"] = head.IsImageFile
42+
ctx.Data["IsImageFileInBase"] = base.IsImageFile
43+
ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
44+
if base == nil {
45+
return nil
46+
}
47+
result, err := base.ImageInfo(name)
48+
if err != nil {
49+
log.Error("ImageInfo failed: %v", err)
50+
return nil
51+
}
52+
return result
53+
}
54+
ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
55+
result, err := head.ImageInfo(name)
56+
if err != nil {
57+
log.Error("ImageInfo failed: %v", err)
58+
return nil
59+
}
60+
return result
61+
}
62+
}
63+
2464
// ParseCompareInfo parse compare info between two commit for preparing comparing references
2565
func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *git.Repository, *git.CompareInfo, string, string) {
2666
baseRepo := ctx.Repo.Repository
@@ -291,43 +331,10 @@ func PrepareCompareDiff(
291331
ctx.Data["title"] = title
292332
ctx.Data["Username"] = headUser.Name
293333
ctx.Data["Reponame"] = headRepo.Name
294-
ctx.Data["IsImageFile"] = headCommit.IsImageFile
295-
ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
296-
result, err := headCommit.ImageInfo(name)
297-
if err != nil {
298-
log.Error("ImageInfo failed: %v", err)
299-
return nil
300-
}
301-
return result
302-
}
303-
ctx.Data["FileExistsInBaseCommit"] = func(filename string) bool {
304-
result, err := baseCommit.HasFile(filename)
305-
if err != nil {
306-
log.Error(
307-
"Error while checking if file \"%s\" exists in base commit \"%s\" (repo: %s): %v",
308-
filename,
309-
baseCommit,
310-
baseGitRepo.Path,
311-
err)
312-
return false
313-
}
314-
return result
315-
}
316-
ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
317-
result, err := baseCommit.ImageInfo(name)
318-
if err != nil {
319-
log.Error("ImageInfo failed: %v", err)
320-
return nil
321-
}
322-
return result
323-
}
324334

335+
setImageCompareContext(ctx, baseCommit, headCommit)
325336
headTarget := path.Join(headUser.Name, repo.Name)
326-
baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
327-
ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", headCommitID)
328-
ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(headTarget, "raw", "commit", headCommitID)
329-
ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "src", "commit", baseCommitID)
330-
ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "raw", "commit", baseCommitID)
337+
setPathsCompareContext(ctx, baseCommit, headCommit, headTarget)
331338

332339
return false
333340
}

‎routers/repo/pull.go

+2-23
Original file line numberDiff line numberDiff line change
@@ -564,29 +564,8 @@ func ViewPullFiles(ctx *context.Context) {
564564
return
565565
}
566566

567-
ctx.Data["IsImageFile"] = commit.IsImageFile
568-
ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
569-
result, err := baseCommit.ImageInfo(name)
570-
if err != nil {
571-
log.Error("ImageInfo failed: %v", err)
572-
return nil
573-
}
574-
return result
575-
}
576-
ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
577-
result, err := commit.ImageInfo(name)
578-
if err != nil {
579-
log.Error("ImageInfo failed: %v", err)
580-
return nil
581-
}
582-
return result
583-
}
584-
585-
baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
586-
ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", endCommitID)
587-
ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(headTarget, "raw", "commit", endCommitID)
588-
ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "src", "commit", startCommitID)
589-
ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "raw", "commit", startCommitID)
567+
setImageCompareContext(ctx, baseCommit, commit)
568+
setPathsCompareContext(ctx, baseCommit, commit, headTarget)
590569

591570
ctx.Data["RequireHighlightJS"] = true
592571
ctx.Data["RequireTribute"] = true

‎templates/repo/diff/box.tmpl

+6-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,12 @@
106106
</h4>
107107
<div class="ui attached unstackable table segment">
108108
{{if ne $file.Type 4}}
109-
{{$isImage := (call $.IsImageFile $file.Name)}}
109+
{{$isImage := false}}
110+
{{if $file.IsDeleted}}
111+
{{$isImage = (call $.IsImageFileInBase $file.Name)}}
112+
{{else}}
113+
{{$isImage = (call $.IsImageFileInHead $file.Name)}}
114+
{{end}}
110115
<div class="file-body file-code code-view code-diff {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}}">
111116
<table>
112117
<tbody>

‎templates/repo/diff/image_diff.tmpl

+45-12
Original file line numberDiff line numberDiff line change
@@ -11,36 +11,69 @@
1111
</tr>
1212
<tr>
1313
<td class="halfwidth center">
14-
{{ $oldImageExists := (call .root.FileExistsInBaseCommit .file.OldName) }}
15-
{{if $oldImageExists}}
14+
{{if or .file.IsDeleted (not .file.IsCreated)}}
1615
<a href="{{$imagePathOld}}" target="_blank">
1716
<img src="{{$imagePathOld}}" class="border red" />
1817
</a>
1918
{{end}}
2019
</td>
2120
<td class="halfwidth center">
22-
<a href="{{$imagePathNew}}" target="_blank">
23-
<img src="{{$imagePathNew}}" class="border green" />
24-
</a>
21+
{{if or .file.IsCreated (not .file.IsDeleted)}}
22+
<a href="{{$imagePathNew}}" target="_blank">
23+
<img src="{{$imagePathNew}}" class="border green" />
24+
</a>
25+
{{end}}
2526
</td>
2627
</tr>
2728
{{ $imageInfoBase := (call .root.ImageInfoBase .file.OldName) }}
2829
{{ $imageInfoHead := (call .root.ImageInfo .file.Name) }}
29-
{{if and $imageInfoBase $imageInfoHead }}
30+
{{if or $imageInfoBase $imageInfoHead }}
3031
<tr>
3132
<td class="halfwidth center">
32-
{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}red{{end}}">{{$imageInfoBase.Width}}</span>
33+
{{if $imageInfoBase }}
34+
{{ $classWidth := "" }}
35+
{{ $classHeight := "" }}
36+
{{ $classByteSize := "" }}
37+
{{if $imageInfoHead}}
38+
{{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}
39+
{{ $classWidth = "red" }}
40+
{{end}}
41+
{{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}
42+
{{ $classHeight = "red" }}
43+
{{end}}
44+
{{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}
45+
{{ $classByteSize = "red" }}
46+
{{end}}
47+
{{end}}
48+
{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{$classWidth}}">{{$imageInfoBase.Width}}</span>
3349
&nbsp;|&nbsp;
34-
{{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}red{{end}}">{{$imageInfoBase.Height}}</span>
50+
{{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{$classHeight}}">{{$imageInfoBase.Height}}</span>
3551
&nbsp;|&nbsp;
36-
{{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}red{{end}}">{{FileSize $imageInfoBase.ByteSize}}</span>
52+
{{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{$classByteSize}}">{{FileSize $imageInfoBase.ByteSize}}</span>
53+
{{end}}
3754
</td>
3855
<td class="halfwidth center">
39-
{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}green{{end}}">{{$imageInfoHead.Width}}</span>
56+
{{if $imageInfoHead }}
57+
{{ $classWidth := "" }}
58+
{{ $classHeight := "" }}
59+
{{ $classByteSize := "" }}
60+
{{if $imageInfoBase}}
61+
{{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}
62+
{{ $classWidth = "green" }}
63+
{{end}}
64+
{{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}
65+
{{ $classHeight = "green" }}
66+
{{end}}
67+
{{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}
68+
{{ $classByteSize = "green" }}
69+
{{end}}
70+
{{end}}
71+
{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{$classWidth}}">{{$imageInfoHead.Width}}</span>
4072
&nbsp;|&nbsp;
41-
{{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}green{{end}}">{{$imageInfoHead.Height}}</span>
73+
{{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{$classHeight}}">{{$imageInfoHead.Height}}</span>
4274
&nbsp;|&nbsp;
43-
{{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}green{{end}}">{{FileSize $imageInfoHead.ByteSize}}</span>
75+
{{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{$classByteSize}}">{{FileSize $imageInfoHead.ByteSize}}</span>
76+
{{end}}
4477
</td>
4578
</tr>
4679
{{end}}

0 commit comments

Comments
 (0)