From a37236314c88ea7990d4d7b1beeef74dd0aa112b Mon Sep 17 00:00:00 2001 From: Mario Lubenka Date: Mon, 16 Sep 2019 11:03:22 +0200 Subject: [PATCH] Adds side-by-side diff for images (#6784) * Adds side-by-side diff for images Signed-off-by: Mario Lubenka * Explain blank imports Signed-off-by: Mario Lubenka * Use complete word for width and height labels on image compare Signed-off-by: Mario Lubenka * Update index.css from master Signed-off-by: Mario Lubenka * Moves ImageInfo to git commit file Signed-off-by: Mario Lubenka * Assign ImageInfo function for template and sets correct target for BeforeSourcePath Signed-off-by: Mario Lubenka * Adds missing comment Signed-off-by: Mario Lubenka * Return error if ImageInfo failed Signed-off-by: Mario Lubenka * Avoid template panic when ImageInfo failed for some reason Signed-off-by: Mario Lubenka * Show file size on image diff Signed-off-by: Mario Lubenka * Removes unused helper function Signed-off-by: Mario Lubenka * Reverts copyright year change Signed-off-by: Mario Lubenka * Close file reader Signed-off-by: Mario Lubenka * Update commit.go Sets correct data key * Moves reader.Close() up a few lines * Updates index.css * Updates CSS file Signed-off-by: Mario Lubenka * Transfers adjustments for image compare to compare.go file Signed-off-by: Mario Lubenka * Adjusts variable name Signed-off-by: Mario Lubenka * Apply lesshint recommendations Signed-off-by: Mario Lubenka * Do not show old image on image compare if it is not in index of base commit Signed-off-by: Mario Lubenka * Change file size text Signed-off-by: Mario Lubenka --- modules/git/commit.go | 49 ++++++++++++++++++++++++++ options/locale/locale_en-US.ini | 5 +++ public/css/index.css | 11 ++++++ public/less/_base.less | 39 +++++++++++++++++++++ public/less/_repository.less | 4 +++ routers/repo/commit.go | 20 ++++++++++- routers/repo/compare.go | 54 ++++++++++++++++++++++++++++- routers/repo/pull.go | 27 ++++++++++++++- templates/repo/diff/box.tmpl | 22 ++++++------ templates/repo/diff/image_diff.tmpl | 46 ++++++++++++++++++++++++ 10 files changed, 262 insertions(+), 15 deletions(-) create mode 100644 templates/repo/diff/image_diff.tmpl diff --git a/modules/git/commit.go b/modules/git/commit.go index 2f7f53e7ce..83e03c2795 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -10,6 +10,11 @@ import ( "bytes" "container/list" "fmt" + "image" + "image/color" + _ "image/gif" // for processing gif images + _ "image/jpeg" // for processing jpeg images + _ "image/png" // for processing png images "io" "net/http" "strconv" @@ -158,6 +163,43 @@ func (c *Commit) IsImageFile(name string) bool { return isImage } +// ImageMetaData represents metadata of an image file +type ImageMetaData struct { + ColorModel color.Model + Width int + Height int + ByteSize int64 +} + +// ImageInfo returns information about the dimensions of an image +func (c *Commit) ImageInfo(name string) (*ImageMetaData, error) { + if !c.IsImageFile(name) { + return nil, nil + } + + blob, err := c.GetBlobByPath(name) + if err != nil { + return nil, err + } + reader, err := blob.DataAsync() + if err != nil { + return nil, err + } + defer reader.Close() + config, _, err := image.DecodeConfig(reader) + if err != nil { + return nil, err + } + + metadata := ImageMetaData{ + ColorModel: config.ColorModel, + Width: config.Width, + Height: config.Height, + ByteSize: blob.Size(), + } + return &metadata, nil +} + // GetCommitByPath return the commit of relative path object. func (c *Commit) GetCommitByPath(relpath string) (*Commit, error) { return c.repo.getCommitByPathWithID(c.ID, relpath) @@ -310,6 +352,13 @@ func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, erro return c.repo.FileChangedBetweenCommits(filename, pastCommit, c.ID.String()) } +// 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 +} + // GetSubModules get all the sub modules of current revision git tree func (c *Commit) GetSubModules() (*ObjectCache, error) { if c.submoduleCache != nil { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 45e71e0595..8a1dc95b9f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1358,6 +1358,11 @@ diff.whitespace_ignore_at_eol = Ignore changes in whitespace at EOL diff.stats_desc = %d changed files with %d additions and %d deletions diff.bin = BIN diff.view_file = View File +diff.file_before = Before +diff.file_after = After +diff.file_image_width = Width +diff.file_image_height = Height +diff.file_byte_size = Size diff.file_suppressed = File diff suppressed because it is too large diff.too_many_files = Some files were not shown because too many files changed in this diff diff.comment.placeholder = Leave a comment diff --git a/public/css/index.css b/public/css/index.css index f58e56bf0a..113fdbf631 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -140,6 +140,16 @@ a{cursor:pointer} .ui .migrate{color:#888!important;opacity:.5} .ui .migrate a{color:#444!important} .ui .migrate a:hover{color:#000!important} +.ui .border{border:1px solid} +.ui .border.red{border-color:#d95c5c!important} +.ui .border.blue{border-color:#428bca!important} +.ui .border.black{border-color:#444} +.ui .border.grey{border-color:#767676!important} +.ui .border.light.grey{border-color:#888!important} +.ui .border.green{border-color:#6cc644!important} +.ui .border.purple{border-color:#6e5494!important} +.ui .border.yellow{border-color:#fbbd08!important} +.ui .border.gold{border-color:#a1882b!important} .ui .branch-tag-choice{line-height:20px} @media only screen and (max-width:767px){.ui.pagination.menu .item.navigation span.navigation_label,.ui.pagination.menu .item:not(.active):not(.navigation){display:none} } @@ -670,6 +680,7 @@ footer .ui.left,footer .ui.right{line-height:40px} .repository .diff-file-box .code-diff td{padding:0 0 0 10px!important;border-top:0} .repository .diff-file-box .code-diff .lines-num{border-color:#d4d4d5;border-right-width:1px;border-right-style:solid;padding:0 5px!important} .repository .diff-file-box .code-diff tbody tr td.halfwidth{width:49%} +.repository .diff-file-box .code-diff tbody tr td.center{text-align:center} .repository .diff-file-box .code-diff tbody tr .removed-code{background-color:#f99} .repository .diff-file-box .code-diff tbody tr .added-code{background-color:#9f9} .repository .diff-file-box .code-diff tbody tr [data-line-num]::before{content:attr(data-line-num);text-align:right} diff --git a/public/less/_base.less b/public/less/_base.less index 487779c137..e295be368d 100644 --- a/public/less/_base.less +++ b/public/less/_base.less @@ -600,6 +600,45 @@ code, } } + .border { + border: 1px solid; + &.red { + border-color: #d95c5c !important; + } + + &.blue { + border-color: #428bca !important; + } + + &.black { + border-color: #444444; + } + + &.grey { + border-color: #767676 !important; + } + + &.light.grey { + border-color: #888888 !important; + } + + &.green { + border-color: #6cc644 !important; + } + + &.purple { + border-color: #6e5494 !important; + } + + &.yellow { + border-color: #fbbd08 !important; + } + + &.gold { + border-color: #a1882b !important; + } + } + .branch-tag-choice { line-height: 20px; } diff --git a/public/less/_repository.less b/public/less/_repository.less index 8eb4a45bd3..74ca683c4e 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -1364,6 +1364,10 @@ width: 49%; } + td.center { + text-align: center; + } + .removed-code { background-color: #ff9999; } diff --git a/routers/repo/commit.go b/routers/repo/commit.go index c3181cbe46..919ebabf42 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -240,6 +240,23 @@ func Diff(ctx *context.Context) { 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"] + if commit.ParentCount() > 0 { + parentCommit, err := ctx.Repo.GitRepo.GetCommit(parents[0]) + if err != nil { + ctx.NotFound("GetParentCommit", err) + return + } + ctx.Data["ImageInfo"] = parentCommit.ImageInfo + } ctx.Data["Title"] = commit.Summary() + " ยท " + base.ShortSha(commitID) ctx.Data["Commit"] = commit ctx.Data["Verification"] = models.ParseCommitWithSignature(commit) @@ -248,6 +265,7 @@ func Diff(ctx *context.Context) { 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) @@ -259,8 +277,8 @@ func Diff(ctx *context.Context) { 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["RawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", commitID) ctx.Data["BranchName"], err = commit.GetBranchName() if err != nil { ctx.ServerError("commit.GetBranchName", err) diff --git a/routers/repo/compare.go b/routers/repo/compare.go index 4f9a918a7c..7c55dfca30 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -247,6 +247,26 @@ func PrepareCompareDiff( return false } + baseGitRepo := ctx.Repo.GitRepo + baseCommitID := baseBranch + if ctx.Data["BaseIsCommit"] == false { + if ctx.Data["BaseIsTag"] == true { + baseCommitID, err = baseGitRepo.GetTagCommitID(baseBranch) + } else { + baseCommitID, err = baseGitRepo.GetBranchCommitID(baseBranch) + } + if err != nil { + ctx.ServerError("GetRefCommitID", err) + return false + } + } + + baseCommit, err := baseGitRepo.GetCommit(baseCommitID) + if err != nil { + ctx.ServerError("GetCommit", err) + return false + } + compareInfo.Commits = models.ValidateCommitsWithEmails(compareInfo.Commits) compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits) compareInfo.Commits = models.ParseCommitsWithStatus(compareInfo.Commits, headRepo) @@ -272,11 +292,43 @@ func PrepareCompareDiff( 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 + } 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["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", compareInfo.MergeBase) 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 } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 14d2f50821..14b8670a20 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -535,6 +535,11 @@ func ViewPullFiles(ctx *context.Context) { ctx.Data["Diff"] = diff ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0 + baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID) + if err != nil { + ctx.ServerError("GetCommit", err) + return + } commit, err := gitRepo.GetCommit(endCommitID) if err != nil { ctx.ServerError("GetCommit", err) @@ -542,9 +547,29 @@ func ViewPullFiles(ctx *context.Context) { } 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["BeforeSourcePath"] = setting.AppSubURL + "/" + path.Join(headTarget, "src", "commit", startCommitID) 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["RequireTribute"] = true if ctx.Data["Assignees"], err = ctx.Repo.Repository.GetAssignees(); err != nil { diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 6c1ae2e447..edc04f9068 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -107,14 +107,12 @@
{{if ne $file.Type 4}} {{$isImage := (call $.IsImageFile $file.Name)}} - {{if and $isImage}} -
- -
- {{else}} -
- - +
+
+ + {{if $isImage}} + {{template "repo/diff/image_diff" dict "file" . "root" $}} + {{else}} {{if $.IsSplitStyle}} {{$highlightClass := $file.GetHighlightClass}} {{range $j, $section := $file.Sections}} @@ -164,10 +162,10 @@ {{else}} {{template "repo/diff/section_unified" dict "file" . "root" $}} {{end}} - -
-
- {{end}} + {{end}} + + +
{{end}} diff --git a/templates/repo/diff/image_diff.tmpl b/templates/repo/diff/image_diff.tmpl new file mode 100644 index 0000000000..8fa7f6b872 --- /dev/null +++ b/templates/repo/diff/image_diff.tmpl @@ -0,0 +1,46 @@ +{{ $imagePathOld := printf "%s/%s" .root.BeforeRawPath (EscapePound .file.OldName) }} +{{ $imagePathNew := printf "%s/%s" .root.RawPath (EscapePound .file.Name) }} + + + + {{.root.i18n.Tr "repo.diff.file_before"}} + + + {{.root.i18n.Tr "repo.diff.file_after"}} + + + + + {{ $oldImageExists := (call .root.FileExistsInBaseCommit .file.OldName) }} + {{if $oldImageExists}} + + + + {{end}} + + + + + + + +{{ $imageInfoBase := (call .root.ImageInfoBase .file.OldName) }} +{{ $imageInfoHead := (call .root.ImageInfo .file.Name) }} +{{if and $imageInfoBase $imageInfoHead }} + + + {{.root.i18n.Tr "repo.diff.file_image_width"}}: {{$imageInfoBase.Width}} +  |  + {{.root.i18n.Tr "repo.diff.file_image_height"}}: {{$imageInfoBase.Height}} +  |  + {{.root.i18n.Tr "repo.diff.file_byte_size"}}: {{FileSize $imageInfoBase.ByteSize}} + + + {{.root.i18n.Tr "repo.diff.file_image_width"}}: {{$imageInfoHead.Width}} +  |  + {{.root.i18n.Tr "repo.diff.file_image_height"}}: {{$imageInfoHead.Height}} +  |  + {{.root.i18n.Tr "repo.diff.file_byte_size"}}: {{FileSize $imageInfoHead.ByteSize}} + + +{{end}}