From 9c6aeb47f7979a434bf30408992c06118c142771 Mon Sep 17 00:00:00 2001 From: Norwin Date: Mon, 28 Jun 2021 01:13:20 +0200 Subject: [PATCH] Link to previous blames in file blame page (#16259) Adds a link to each blame hunk, to view the blame of an earlier version of the file, similar to GitHub. Also refactors the blame render from fmtstring based to template based. * Fix blame bottom line and add blame prior button * Jump to previous parent commit from the commit. * Fix previous commit link * Fix previous blame link * Fix the given file not exist in the previous commit. * Fix blameRow struct not export * fix theming issues, rename template var * remove unused LastCommit fetch * fix location of blame-hunk divider * rewrite previous commit checks * remove duplicate commit lookup its already resolved and stored in ctx.Repo.Commit! * split out blamePart processing into function Co-authored-by: rogerluo410 --- options/locale/locale_en-US.ini | 1 + routers/web/repo/blame.go | 179 ++++++++++++----------- templates/repo/blame.tmpl | 39 ++++- web_src/js/index.js | 25 +++- web_src/less/_base.less | 17 ++- web_src/less/themes/theme-arc-green.less | 6 +- 6 files changed, 166 insertions(+), 101 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index e0ece8f9f0..dcdfa611ec 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -809,6 +809,7 @@ delete_preexisting_label = Delete delete_preexisting = Delete pre-existing files delete_preexisting_content = Delete files in %s delete_preexisting_success = Deleted unadopted files in %s +blame_prior = View blame prior to this change transfer.accept = Accept Transfer transfer.accept_desc = Transfer to "%s" diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 1a3e1dcb9c..4ade9e9a93 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -5,7 +5,6 @@ package repo import ( - "bytes" "container/list" "fmt" "html" @@ -18,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/highlight" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/timeutil" ) @@ -27,6 +25,20 @@ const ( tplBlame base.TplName = "repo/home" ) +type blameRow struct { + RowNumber int + Avatar gotemplate.HTML + RepoLink string + PartSha string + PreviousSha string + PreviousShaURL string + IsFirstCommit bool + CommitURL string + CommitMessage string + CommitSince gotemplate.HTML + Code gotemplate.HTML +} + // RefBlame render blame page func RefBlame(ctx *context.Context) { fileName := ctx.Repo.TreePath @@ -39,19 +51,6 @@ func RefBlame(ctx *context.Context) { repoName := ctx.Repo.Repository.Name commitID := ctx.Repo.CommitID - commit, err := ctx.Repo.GitRepo.GetCommit(commitID) - if err != nil { - if git.IsErrNotExist(err) { - ctx.NotFound("Repo.GitRepo.GetCommit", err) - } else { - ctx.ServerError("Repo.GitRepo.GetCommit", err) - } - return - } - if len(commitID) != 40 { - commitID = commit.ID.String() - } - branchLink := ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() treeLink := branchLink rawLink := ctx.Repo.RepoLink + "/raw/" + ctx.Repo.BranchNameSubURL() @@ -74,25 +73,6 @@ func RefBlame(ctx *context.Context) { } } - // Show latest commit info of repository in table header, - // or of directory if not in root directory. - latestCommit := ctx.Repo.Commit - if len(ctx.Repo.TreePath) > 0 { - latestCommit, err = ctx.Repo.Commit.GetCommitByPath(ctx.Repo.TreePath) - if err != nil { - ctx.ServerError("GetCommitByPath", err) - return - } - } - ctx.Data["LatestCommit"] = latestCommit - ctx.Data["LatestCommitVerification"] = models.ParseCommitWithSignature(latestCommit) - ctx.Data["LatestCommitUser"] = models.ValidateCommitWithEmail(latestCommit) - - statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, ctx.Repo.Commit.ID.String(), models.ListOptions{}) - if err != nil { - log.Error("GetLatestCommitStatus: %v", err) - } - // Get current entry user currently looking at. entry, err := ctx.Repo.Commit.GetTreeEntryByPath(ctx.Repo.TreePath) if err != nil { @@ -102,9 +82,6 @@ func RefBlame(ctx *context.Context) { blob := entry.Blob() - ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(statuses) - ctx.Data["LatestCommitStatuses"] = statuses - ctx.Data["Paths"] = paths ctx.Data["TreeLink"] = treeLink ctx.Data["TreeNames"] = treeNames @@ -145,8 +122,33 @@ func RefBlame(ctx *context.Context) { blameParts = append(blameParts, *blamePart) } + // Get Topics of this repo + renderRepoTopics(ctx) + if ctx.Written() { + return + } + + commitNames, previousCommits := processBlameParts(ctx, blameParts) + if ctx.Written() { + return + } + + renderBlame(ctx, blameParts, commitNames, previousCommits) + + ctx.HTML(http.StatusOK, tplBlame) +} + +func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[string]models.UserCommit, map[string]string) { + // store commit data by SHA to look up avatar info etc commitNames := make(map[string]models.UserCommit) + // previousCommits contains links from SHA to parent SHA, + // if parent also contains the current TreePath. + previousCommits := make(map[string]string) + // and as blameParts can reference the same commits multiple + // times, we cache the lookup work locally commits := list.New() + commitCache := map[string]*git.Commit{} + commitCache[ctx.Repo.Commit.ID.String()] = ctx.Repo.Commit for _, part := range blameParts { sha := part.Sha @@ -154,14 +156,38 @@ func RefBlame(ctx *context.Context) { continue } - commit, err := ctx.Repo.GitRepo.GetCommit(sha) - if err != nil { - if git.IsErrNotExist(err) { - ctx.NotFound("Repo.GitRepo.GetCommit", err) - } else { - ctx.ServerError("Repo.GitRepo.GetCommit", err) + // find the blamePart commit, to look up parent & email address for avatars + commit, ok := commitCache[sha] + var err error + if !ok { + commit, err = ctx.Repo.GitRepo.GetCommit(sha) + if err != nil { + if git.IsErrNotExist(err) { + ctx.NotFound("Repo.GitRepo.GetCommit", err) + } else { + ctx.ServerError("Repo.GitRepo.GetCommit", err) + } + return nil, nil + } + commitCache[sha] = commit + } + + // find parent commit + if commit.ParentCount() > 0 { + psha := commit.Parents[0] + previousCommit, ok := commitCache[psha.String()] + if !ok { + previousCommit, _ = commit.Parent(0) + if previousCommit != nil { + commitCache[psha.String()] = previousCommit + } + } + // only store parent commit ONCE, if it has the file + if previousCommit != nil { + if haz1, _ := previousCommit.HasFile(ctx.Repo.TreePath); haz1 { + previousCommits[commit.ID.String()] = previousCommit.ID.String() + } } - return } commits.PushBack(commit) @@ -169,46 +195,39 @@ func RefBlame(ctx *context.Context) { commitNames[commit.ID.String()] = models.UserCommit{} } + // populate commit email addresses to later look up avatars. commits = models.ValidateCommitsWithEmails(commits) - for e := commits.Front(); e != nil; e = e.Next() { c := e.Value.(models.UserCommit) - commitNames[c.ID.String()] = c } - // Get Topics of this repo - renderRepoTopics(ctx) - if ctx.Written() { - return - } - - renderBlame(ctx, blameParts, commitNames) - - ctx.HTML(http.StatusOK, tplBlame) + return commitNames, previousCommits } -func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]models.UserCommit) { +func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]models.UserCommit, previousCommits map[string]string) { repoLink := ctx.Repo.RepoLink var lines = make([]string, 0) - - var commitInfo bytes.Buffer - var lineNumbers bytes.Buffer - var codeLines bytes.Buffer + rows := make([]*blameRow, 0) var i = 0 - for pi, part := range blameParts { + var commitCnt = 0 + for _, part := range blameParts { for index, line := range part.Lines { i++ lines = append(lines, line) - var attr = "" - if len(part.Lines)-1 == index && len(blameParts)-1 != pi { - attr = " bottom-line" + br := &blameRow{ + RowNumber: i, } + commit := commitNames[part.Sha] + previousSha := previousCommits[part.Sha] if index == 0 { + // Count commit number + commitCnt++ + // User avatar image commitSince := timeutil.TimeSinceUnix(timeutil.TimeStamp(commit.Author.When.Unix()), ctx.Data["Lang"].(string)) @@ -219,16 +238,14 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m avatar = string(templates.AvatarByEmail(commit.Author.Email, commit.Author.Name, 18, "mr-3")) } - commitInfo.WriteString(fmt.Sprintf(`
%s
%[5]s
%s
`, attr, avatar, repoLink, part.Sha, html.EscapeString(commit.CommitMessage), commitSince)) - } else { - commitInfo.WriteString(fmt.Sprintf(`
​
`, attr)) - } - - //Line number - if len(part.Lines)-1 == index && len(blameParts)-1 != pi { - lineNumbers.WriteString(fmt.Sprintf(``, i, i)) - } else { - lineNumbers.WriteString(fmt.Sprintf(``, i, i)) + br.Avatar = gotemplate.HTML(avatar) + br.RepoLink = repoLink + br.PartSha = part.Sha + br.PreviousSha = previousSha + br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, previousSha, ctx.Repo.TreePath) + br.CommitURL = fmt.Sprintf("%s/commit/%s", repoLink, part.Sha) + br.CommitMessage = html.EscapeString(commit.CommitMessage) + br.CommitSince = commitSince } if i != len(lines)-1 { @@ -236,16 +253,12 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m } fileName := fmt.Sprintf("%v", ctx.Data["FileName"]) line = highlight.Code(fileName, line) - line = `` + line + `` - if len(part.Lines)-1 == index && len(blameParts)-1 != pi { - codeLines.WriteString(fmt.Sprintf(`
  • %s
    • `, i, i, line)) - } else { - codeLines.WriteString(fmt.Sprintf(`
  • %s
    • `, i, i, line)) - } + + br.Code = gotemplate.HTML(line) + rows = append(rows, br) } } - ctx.Data["BlameContent"] = gotemplate.HTML(codeLines.String()) - ctx.Data["BlameCommitInfo"] = gotemplate.HTML(commitInfo.String()) - ctx.Data["BlameLineNums"] = gotemplate.HTML(lineNumbers.String()) + ctx.Data["BlameRows"] = rows + ctx.Data["CommitCnt"] = commitCnt } diff --git a/templates/repo/blame.tmpl b/templates/repo/blame.tmpl index 638683b25e..c7c497088a 100644 --- a/templates/repo/blame.tmpl +++ b/templates/repo/blame.tmpl @@ -23,11 +23,40 @@
    - - - - - + {{range $row := .BlameRows}} + + + + + + + {{end}}
    {{.BlameCommitInfo}}{{.BlameLineNums}}
      {{.BlameContent}}
    +
    +
    +
    + {{$row.Avatar}} +
    +
    + + {{$row.CommitMessage}} + +
    +
    + {{$row.CommitSince}} +
    +
    +
    +     		+ {{if $row.PreviousSha}} + + {{svg "octicon-versions"}} + + {{end}} + + + + {{$row.Code}} +
    diff --git a/web_src/js/index.js b/web_src/js/index.js index f1f41ce747..0693175a00 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -2283,20 +2283,24 @@ function initCodeView() { const $select = $(this); let $list; if ($('div.blame').length) { - $list = $('.code-view td.lines-code li'); + $list = $('.code-view td.lines-code.blame-code'); } else { $list = $('.code-view td.lines-code'); } selectRange($list, $list.filter(`[rel=${$select.attr('id')}]`), (e.shiftKey ? $list.filter('.active').eq(0) : null)); deSelect(); - showLineButton(); + + // show code view menu marker (don't show in blame page) + if ($('div.blame').length === 0) { + showLineButton(); + } }); $(window).on('hashchange', () => { let m = window.location.hash.match(/^#(L\d+)-(L\d+)$/); let $list; if ($('div.blame').length) { - $list = $('.code-view td.lines-code li'); + $list = $('.code-view td.lines-code.blame-code'); } else { $list = $('.code-view td.lines-code'); } @@ -2304,7 +2308,12 @@ function initCodeView() { if (m) { $first = $list.filter(`[rel=${m[1]}]`); selectRange($list, $first, $list.filter(`[rel=${m[2]}]`)); - showLineButton(); + + // show code view menu marker (don't show in blame page) + if ($('div.blame').length === 0) { + showLineButton(); + } + $('html, body').scrollTop($first.offset().top - 200); return; } @@ -2312,7 +2321,12 @@ function initCodeView() { if (m) { $first = $list.filter(`[rel=L${m[2]}]`); selectRange($list, $first); - showLineButton(); + + // show code view menu marker (don't show in blame page) + if ($('div.blame').length === 0) { + showLineButton(); + } + $('html, body').scrollTop($first.offset().top - 200); } }).trigger('hashchange'); @@ -2911,7 +2925,6 @@ function selectRange($list, $select, $from) { } else { $issue.attr('href', `${$issue.attr('href')}%23L${a}-L${b}`); } - return; } } diff --git a/web_src/less/_base.less b/web_src/less/_base.less index fb4e15e146..8a2279c727 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -106,6 +106,7 @@ --color-markup-code-block: #00000010; --color-button: #ffffff; --color-code-bg: #ffffff; + --color-code-sidebar-bg: #f5f5f5; --color-shadow: #00000030; --color-secondary-bg: #f4f4f4; --color-expand-button: #d8efff; @@ -1442,6 +1443,14 @@ a.ui.label:hover { margin-right: 0; } +.lines-blame-btn { + padding-left: 10px; + padding-right: 10px; + text-align: right !important; + background-color: var(--color-code-sidebar-bg); + width: 2%; +} + .lines-num { padding-left: 10px; padding-right: 10px; @@ -1507,7 +1516,7 @@ a.ui.label:hover { .blame .lines-num { padding: 0 !important; - background-color: #f5f5f5; + background-color: var(--color-code-sidebar-bg); } .blame .lines-code { @@ -1532,7 +1541,7 @@ a.ui.label:hover { vertical-align: top; color: #999999; padding: 0 !important; - background: #f5f5f5; + background: var(--color-code-sidebar-bg); width: 1%; -moz-user-select: none; -ms-user-select: none; @@ -1574,6 +1583,10 @@ a.ui.label:hover { } } +.top-line-blame { + border-top: 1px solid var(--color-secondary); +} + .lines-code, .lines-commit { .bottom-line { diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less index e638f3b461..74db8faaaa 100644 --- a/web_src/less/themes/theme-arc-green.less +++ b/web_src/less/themes/theme-arc-green.less @@ -101,6 +101,7 @@ --color-markup-code-block: #292d39; --color-button: #353846; --color-code-bg: #2a2e3a; + --color-code-sidebar-bg: #2e323e; --color-shadow: #00000060; --color-secondary-bg: #2a2e3a; --color-text-focus: #fff; @@ -430,11 +431,6 @@ td.blob-hunk { background-color: #bbbbbb !important; } -.lines-commit, -.blame .lines-num { - background: #2e323e !important; -} - .lines-num { color: var(--color-secondary-dark-6) !important; border-color: var(--color-secondary) !important;