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>
This commit is contained in:
Mario Lubenka 2019-10-04 21:58:54 +02:00 committed by techknowlogick
parent de8a0a3938
commit f92a0b68fe
6 changed files with 106 additions and 91 deletions

View file

@ -355,8 +355,11 @@ func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, erro
// HasFile returns true if the file given exists on this commit // 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. // 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) { func (c *Commit) HasFile(filename string) (bool, error) {
result, err := c.repo.LsFiles(filename) _, err := c.GetBlobByPath(filename)
return result[0] == filename, err if err != nil {
return false, err
}
return true, nil
} }
// GetSubModules get all the sub modules of current revision git tree // GetSubModules get all the sub modules of current revision git tree

View file

@ -239,24 +239,18 @@ func Diff(ctx *context.Context) {
ctx.Data["CommitID"] = commitID ctx.Data["CommitID"] = commitID
ctx.Data["Username"] = userName ctx.Data["Username"] = userName
ctx.Data["Reponame"] = repoName ctx.Data["Reponame"] = repoName
ctx.Data["IsImageFile"] = commit.IsImageFile
ctx.Data["ImageInfo"] = func(name string) *git.ImageMetaData { var parentCommit *git.Commit
result, err := commit.ImageInfo(name)
if err != nil {
log.Error("ImageInfo failed: %v", err)
return nil
}
return result
}
ctx.Data["ImageInfoBase"] = ctx.Data["ImageInfo"]
if commit.ParentCount() > 0 { if commit.ParentCount() > 0 {
parentCommit, err := ctx.Repo.GitRepo.GetCommit(parents[0]) parentCommit, err = ctx.Repo.GitRepo.GetCommit(parents[0])
if err != nil { if err != nil {
ctx.NotFound("GetParentCommit", err) ctx.NotFound("GetParentCommit", err)
return return
} }
ctx.Data["ImageInfo"] = parentCommit.ImageInfo
} }
setImageCompareContext(ctx, parentCommit, commit)
headTarget := path.Join(userName, repoName)
setPathsCompareContext(ctx, parentCommit, commit, headTarget)
ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID) ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
ctx.Data["Commit"] = commit ctx.Data["Commit"] = commit
ctx.Data["Verification"] = models.ParseCommitWithSignature(commit) ctx.Data["Verification"] = models.ParseCommitWithSignature(commit)
@ -264,8 +258,6 @@ func Diff(ctx *context.Context) {
ctx.Data["Diff"] = diff ctx.Data["Diff"] = diff
ctx.Data["Parents"] = parents ctx.Data["Parents"] = parents
ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0 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{} note := &git.Note{}
err = git.GetNote(ctx.Repo.GitRepo, commitID, note) err = git.GetNote(ctx.Repo.GitRepo, commitID, note)
@ -275,10 +267,6 @@ func Diff(ctx *context.Context) {
ctx.Data["NoteAuthor"] = models.ValidateCommitWithEmail(note.Commit) 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() ctx.Data["BranchName"], err = commit.GetBranchName()
if err != nil { if err != nil {
ctx.ServerError("commit.GetBranchName", err) ctx.ServerError("commit.GetBranchName", err)

View file

@ -5,6 +5,7 @@
package repo package repo
import ( import (
"fmt"
"path" "path"
"strings" "strings"
@ -21,6 +22,45 @@ const (
tplCompare base.TplName = "repo/diff/compare" tplCompare base.TplName = "repo/diff/compare"
) )
// 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["IsImageFileInHead"] = head.IsImageFile
ctx.Data["IsImageFileInBase"] = base.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["ImageInfo"] = func(name string) *git.ImageMetaData {
result, err := head.ImageInfo(name)
if err != nil {
log.Error("ImageInfo failed: %v", err)
return nil
}
return result
}
}
// ParseCompareInfo parse compare info between two commit for preparing comparing references // ParseCompareInfo parse compare info between two commit for preparing comparing references
func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *git.Repository, *git.CompareInfo, string, string) { func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *git.Repository, *git.CompareInfo, string, string) {
baseRepo := ctx.Repo.Repository baseRepo := ctx.Repo.Repository
@ -291,43 +331,10 @@ func PrepareCompareDiff(
ctx.Data["title"] = title ctx.Data["title"] = title
ctx.Data["Username"] = headUser.Name ctx.Data["Username"] = headUser.Name
ctx.Data["Reponame"] = headRepo.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
}
setImageCompareContext(ctx, baseCommit, headCommit)
headTarget := path.Join(headUser.Name, repo.Name) headTarget := path.Join(headUser.Name, repo.Name)
baseTarget := path.Join(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) setPathsCompareContext(ctx, baseCommit, headCommit, headTarget)
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)
return false return false
} }

View file

@ -564,29 +564,8 @@ func ViewPullFiles(ctx *context.Context) {
return return
} }
ctx.Data["IsImageFile"] = commit.IsImageFile setImageCompareContext(ctx, baseCommit, commit)
ctx.Data["ImageInfoBase"] = func(name string) *git.ImageMetaData { setPathsCompareContext(ctx, baseCommit, commit, headTarget)
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)
ctx.Data["RequireHighlightJS"] = true ctx.Data["RequireHighlightJS"] = true
ctx.Data["RequireTribute"] = true ctx.Data["RequireTribute"] = true

View file

@ -106,7 +106,12 @@
</h4> </h4>
<div class="ui attached unstackable table segment"> <div class="ui attached unstackable table segment">
{{if ne $file.Type 4}} {{if ne $file.Type 4}}
{{$isImage := (call $.IsImageFile $file.Name)}} {{$isImage := false}}
{{if $file.IsDeleted}}
{{$isImage = (call $.IsImageFileInBase $file.Name)}}
{{else}}
{{$isImage = (call $.IsImageFileInHead $file.Name)}}
{{end}}
<div class="file-body file-code code-view code-diff {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}}"> <div class="file-body file-code code-view code-diff {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}}">
<table> <table>
<tbody> <tbody>

View file

@ -11,36 +11,69 @@
</tr> </tr>
<tr> <tr>
<td class="halfwidth center"> <td class="halfwidth center">
{{ $oldImageExists := (call .root.FileExistsInBaseCommit .file.OldName) }} {{if or .file.IsDeleted (not .file.IsCreated)}}
{{if $oldImageExists}}
<a href="{{$imagePathOld}}" target="_blank"> <a href="{{$imagePathOld}}" target="_blank">
<img src="{{$imagePathOld}}" class="border red" /> <img src="{{$imagePathOld}}" class="border red" />
</a> </a>
{{end}} {{end}}
</td> </td>
<td class="halfwidth center"> <td class="halfwidth center">
<a href="{{$imagePathNew}}" target="_blank"> {{if or .file.IsCreated (not .file.IsDeleted)}}
<img src="{{$imagePathNew}}" class="border green" /> <a href="{{$imagePathNew}}" target="_blank">
</a> <img src="{{$imagePathNew}}" class="border green" />
</a>
{{end}}
</td> </td>
</tr> </tr>
{{ $imageInfoBase := (call .root.ImageInfoBase .file.OldName) }} {{ $imageInfoBase := (call .root.ImageInfoBase .file.OldName) }}
{{ $imageInfoHead := (call .root.ImageInfo .file.Name) }} {{ $imageInfoHead := (call .root.ImageInfo .file.Name) }}
{{if and $imageInfoBase $imageInfoHead }} {{if or $imageInfoBase $imageInfoHead }}
<tr> <tr>
<td class="halfwidth center"> <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; &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; &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>
<td class="halfwidth center"> <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; &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; &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> </td>
</tr> </tr>
{{end}} {{end}}