Bugfix for image compare and minor improvements to image compare

modules/git/commit.go

@@ -355,8 +355,11 @@ func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, erro
 // HasFile returns true if the file given exists on this commit
 // This does only mean it's there - it does not mean the file was changed during the commit.
 func (c *Commit) HasFile(filename string) (bool, error) {
-	result, err := c.repo.LsFiles(filename)
-	return result[0] == filename, err
+	_, err := c.GetBlobByPath(filename)
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
 // GetSubModules get all the sub modules of current revision git tree

routers/repo/commit.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/services/compare"
 	"code.gitea.io/gitea/services/gitdiff"
 )
 
@@ -239,33 +240,25 @@ func Diff(ctx *context.Context) {
 	ctx.Data["CommitID"] = commitID
 	ctx.Data["Username"] = userName
 	ctx.Data["Reponame"] = repoName
-	ctx.Data["IsImageFile"] = commit.IsImageFile
-	ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
-		result, err := commit.ImageInfo(name)
-		if err != nil {
-			log.Error("ImageInfo failed: %v", err)
-			return nil
-		}
-		return result
-	}
-	ctx.Data["ImageInfoBase"] = ctx.Data["ImageInfo"]
+
+	var parentCommit *git.Commit
 	if commit.ParentCount() > 0 {
-		parentCommit, err := ctx.Repo.GitRepo.GetCommit(parents[0])
+		parentCommit, err = ctx.Repo.GitRepo.GetCommit(parents[0])
 		if err != nil {
 			ctx.NotFound("GetParentCommit", err)
 			return
 		}
-		ctx.Data["ImageInfo"] = parentCommit.ImageInfo
 	}
+	compare.SetImageCompareContext(ctx, parentCommit, commit)
+	headTarget := path.Join(userName, repoName)
+	compare.SetPathsCompareContext(ctx, parentCommit, commit, headTarget)
 	ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
 	ctx.Data["Commit"] = commit
 	ctx.Data["Verification"] = models.ParseCommitWithSignature(commit)
 	ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
 	ctx.Data["Diff"] = diff
 	ctx.Data["Parents"] = parents
 	ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
-	ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "src", "commit", commitID)
-	ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", commitID)
 
 	note := &git.Note{}
 	err = git.GetNote(ctx.Repo.GitRepo, commitID, note)
@@ -275,10 +268,6 @@ func Diff(ctx *context.Context) {
 		ctx.Data["NoteAuthor"] = models.ValidateCommitWithEmail(note.Commit)
 	}
 
-	if commit.ParentCount() > 0 {
-		ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "src", "commit", parents[0])
-		ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", parents[0])
-	}
 	ctx.Data["BranchName"], err = commit.GetBranchName()
 	if err != nil {
 		ctx.ServerError("commit.GetBranchName", err)

routers/repo/compare.go

@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/services/compare"
 	"code.gitea.io/gitea/services/gitdiff"
 )
 
@@ -291,43 +292,10 @@ func PrepareCompareDiff(
 	ctx.Data["title"] = title
 	ctx.Data["Username"] = headUser.Name
 	ctx.Data["Reponame"] = headRepo.Name
-	ctx.Data["IsImageFile"] = headCommit.IsImageFile
-	ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
-		result, err := headCommit.ImageInfo(name)
-		if err != nil {
-			log.Error("ImageInfo failed: %v", err)
-			return nil
-		}
-		return result
-	}
-	ctx.Data["FileExistsInBaseCommit"] = func(filename string) bool {
-		result, err := baseCommit.HasFile(filename)
-		if err != nil {
-			log.Error(
-				"Error while checking if file \"%s\" exists in base commit \"%s\" (repo: %s): %v",
-				filename,
-				baseCommit,
-				baseGitRepo.Path,
-				err)
-			return false
-		}
-		return result
-	}
-	ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
-		result, err := baseCommit.ImageInfo(name)
-		if err != nil {
-			log.Error("ImageInfo failed: %v", err)
-			return nil
-		}
-		return result
-	}
 
+	compare.SetImageCompareContext(ctx, baseCommit, headCommit)
 	headTarget := path.Join(headUser.Name, repo.Name)
-	baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-	ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", headCommitID)
-	ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(headTarget, "raw", "commit", headCommitID)
-	ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "src", "commit", baseCommitID)
-	ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "raw", "commit", baseCommitID)
+	compare.SetPathsCompareContext(ctx, baseCommit, headCommit, headTarget)
 
 	return false
 }

routers/repo/pull.go

@@ -25,6 +25,7 @@ import (
 	pull_service "code.gitea.io/gitea/modules/pull"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/services/compare"
 	"code.gitea.io/gitea/services/gitdiff"
 
 	"github.com/unknwon/com"
@@ -565,29 +566,8 @@ func ViewPullFiles(ctx *context.Context) {
 		return
 	}
 
-	ctx.Data["IsImageFile"] = commit.IsImageFile
-	ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
-		result, err := baseCommit.ImageInfo(name)
-		if err != nil {
-			log.Error("ImageInfo failed: %v", err)
-			return nil
-		}
-		return result
-	}
-	ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
-		result, err := commit.ImageInfo(name)
-		if err != nil {
-			log.Error("ImageInfo failed: %v", err)
-			return nil
-		}
-		return result
-	}
-
-	baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-	ctx.Data["SourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", endCommitID)
-	ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(headTarget, "raw", "commit", endCommitID)
-	ctx.Data["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "src", "commit", startCommitID)
-	ctx.Data["BeforeRawPath"] = setting.AppSubURL + "/" + path.Join(baseTarget, "raw", "commit", startCommitID)
+	compare.SetImageCompareContext(ctx, baseCommit, commit)
+	compare.SetPathsCompareContext(ctx, baseCommit, commit, headTarget)
 
 	ctx.Data["RequireHighlightJS"] = true
 	ctx.Data["RequireTribute"] = true

services/compare/compare.go

@@ -0,0 +1,69 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package compare
+
+import (
+	"fmt"
+	"path"
+
+	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// SetPathsCompareContext sets context data for source and raw paths
+func SetPathsCompareContext(ctx *context.Context, base *git.Commit, head *git.Commit, headTarget string) {
+	sourcePath := setting.AppSubURL + "/%s/src/commit/%s"
+	rawPath := setting.AppSubURL + "/%s/raw/commit/%s"
+
+	ctx.Data["SourcePath"] = fmt.Sprintf(sourcePath, headTarget, head.ID)
+	ctx.Data["RawPath"] = fmt.Sprintf(rawPath, headTarget, head.ID)
+	if base != nil {
+		baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
+		ctx.Data["BeforeSourcePath"] = fmt.Sprintf(sourcePath, baseTarget, base.ID)
+		ctx.Data["BeforeRawPath"] = fmt.Sprintf(rawPath, baseTarget, base.ID)
+	}
+}
+
+// SetImageCompareContext sets context data that is required by image compare template
+func SetImageCompareContext(ctx *context.Context, base *git.Commit, head *git.Commit) {
+	ctx.Data["IsImageFile"] = head.IsImageFile
+	ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData {
+		if base == nil {
+			return nil
+		}
+		result, err := base.ImageInfo(name)
+		if err != nil {
+			log.Error("ImageInfo failed: %v", err)
+			return nil
+		}
+		return result
+	}
+	ctx.Data["FileExistsInBaseCommit"] = func(filename string) bool {
+		if base == nil {
+			return false
+		}
+		result, err := base.HasFile(filename)
+		if err != nil {
+			log.Error(
+				"Error while checking if file \"%s\" exists in base commit \"%s\" (repo: %s): %v",
+				filename,
+				base,
+				ctx.Repo.GitRepo.Path,
+				err)
+			return false
+		}
+		return result
+	}
+	ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData {
+		result, err := head.ImageInfo(name)
+		if err != nil {
+			log.Error("ImageInfo failed: %v", err)
+			return nil
+		}
+		return result
+	}
+}

templates/repo/diff/image_diff.tmpl

@@ -26,21 +26,53 @@
 </tr>
 {{ $imageInfoBase := (call .root.ImageInfoBase .file.OldName) }}
 {{ $imageInfoHead := (call .root.ImageInfo .file.Name) }}
-{{if and $imageInfoBase $imageInfoHead }}
+{{if or $imageInfoBase $imageInfoHead }}
 <tr>
  	<td class="halfwidth center">
- 		{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}red{{end}}">{{$imageInfoBase.Width}}</span>
+ 	{{if $imageInfoBase }}
+ 		{{ $classWidth := "" }}
+ 		{{ $classHeight := "" }}
+ 		{{ $classByteSize := "" }}
+ 		{{if $imageInfoHead}}
+			{{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}
+				{{ $classWidth = "red" }}
+			{{end}}
+			{{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}
+				{{ $classHeight = "red" }}
+			{{end}}
+			{{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}
+				{{ $classByteSize = "red" }}
+			{{end}}
+ 		{{end}}
+ 		{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{$classWidth}}">{{$imageInfoBase.Width}}</span>
  		&nbsp;|&nbsp;
- 	    {{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}red{{end}}">{{$imageInfoBase.Height}}</span>
+ 	    {{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{$classHeight}}">{{$imageInfoBase.Height}}</span>
  		&nbsp;|&nbsp;
- 	    {{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}red{{end}}">{{FileSize $imageInfoBase.ByteSize}}</span>
+ 	    {{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{$classByteSize}}">{{FileSize $imageInfoBase.ByteSize}}</span>
+ 	{{end}}
  	</td>
  	<td class="halfwidth center">
- 		{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}green{{end}}">{{$imageInfoHead.Width}}</span>
+ 	{{if $imageInfoHead }}
+ 		{{ $classWidth := "" }}
+ 		{{ $classHeight := "" }}
+ 		{{ $classByteSize := "" }}
+ 		{{if $imageInfoBase}}
+			{{if not (eq $imageInfoBase.Width $imageInfoHead.Width)}}
+				{{ $classWidth = "green" }}
+			{{end}}
+			{{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}
+				{{ $classHeight = "green" }}
+			{{end}}
+			{{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}
+				{{ $classByteSize = "green" }}
+			{{end}}
+ 		{{end}}
+ 		{{.root.i18n.Tr "repo.diff.file_image_width"}}: <span class="text {{$classWidth}}">{{$imageInfoHead.Width}}</span>
  		&nbsp;|&nbsp;
- 	    {{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{if not (eq $imageInfoBase.Height $imageInfoHead.Height)}}green{{end}}">{{$imageInfoHead.Height}}</span>
+ 	    {{.root.i18n.Tr "repo.diff.file_image_height"}}: <span class="text {{$classHeight}}">{{$imageInfoHead.Height}}</span>
  		&nbsp;|&nbsp;
- 	    {{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{if not (eq $imageInfoBase.ByteSize $imageInfoHead.ByteSize)}}green{{end}}">{{FileSize $imageInfoHead.ByteSize}}</span>
+ 	    {{.root.i18n.Tr "repo.diff.file_byte_size"}}: <span class="text {{$classByteSize}}">{{FileSize $imageInfoHead.ByteSize}}</span>
+ 	{{end}}
  	</td>
  </tr>
 {{end}}