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 <rogerluo410@gmail.com>
4 years ago · 9c6aeb47f7
parent 59c58553ba
commit 9c6aeb47f7
6 changed files with 166 additions and 101 deletions
--- 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"
--- 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(`<div class="blame-info%s"><div class="blame-data"><div class="blame-avatar">%s</div><div class="blame-message"><a href="%s/commit/%s" title="%[5]s">%[5]s</a></div><div class="blame-time">%s</div></div></div>`, attr, avatar, repoLink, part.Sha, html.EscapeString(commit.CommitMessage), commitSince))
-			} else {
-				commitInfo.WriteString(fmt.Sprintf(`<div class="blame-info%s">&#8203;</div>`, attr))
-			}
-
-			//Line number
-			if len(part.Lines)-1 == index && len(blameParts)-1 != pi {
-				lineNumbers.WriteString(fmt.Sprintf(`<span id="L%d" data-line-number="%d" class="bottom-line"></span>`, i, i))
-			} else {
-				lineNumbers.WriteString(fmt.Sprintf(`<span id="L%d" data-line-number="%d"></span>`, 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 = `<code class="code-inner">` + line + `</code>`
-			if len(part.Lines)-1 == index && len(blameParts)-1 != pi {
-				codeLines.WriteString(fmt.Sprintf(`<li class="L%d bottom-line" rel="L%d">%s</li>`, i, i, line))
-			} else {
-				codeLines.WriteString(fmt.Sprintf(`<li class="L%d" rel="L%d">%s</li>`, 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
 }
--- a/templates/repo/blame.tmpl
+++ b/templates/repo/blame.tmpl
@ -23,11 +23,40 @@
 		<div class="file-view code-view">
 			<table>
 				<tbody>
-					<tr>
-						<td class="lines-commit">{{.BlameCommitInfo}}</td>
-						<td class="lines-num">{{.BlameLineNums}}</td>
-						<td class="lines-code"><code class="chroma"><ol class="linenums">{{.BlameContent}}</ol></code></td>
-					</tr>
+					{{range $row := .BlameRows}}
+						<tr class="{{if and (gt $.CommitCnt 1) ($row.CommitMessage)}}top-line-blame{{end}}">
+							<td class="lines-commit">
+								<div class="blame-info">
+									<div class="blame-data">
+										<div class="blame-avatar">
+											{{$row.Avatar}}
+										</div>
+										<div class="blame-message">
+											<a href="{{$row.CommitURL}}" title="{{$row.CommitMessage}}">
+												{{$row.CommitMessage}}
+											</a>
+										</div>
+										<div class="blame-time">
+											{{$row.CommitSince}}
+										</div>
+									</div>
+								</div>
+							</td>
+							<td class="lines-blame-btn">
+								{{if $row.PreviousSha}}
+									<a href="{{$row.PreviousShaURL}}" class="poping up" data-content='{{$.i18n.Tr "repo.blame_prior"}}' data-variation="tiny inverted">
+										{{svg "octicon-versions"}}
+									</a>
+								{{end}}
+							</td>
+							<td class="lines-num">
+								<span id="L{{$row.RowNumber}}" data-line-number="{{$row.RowNumber}}"></span>
+							</td>
+							<td rel="L{{$row.RowNumber}}" rel="L{{$row.RowNumber}}" class="lines-code blame-code chroma">
+								<code class="code-inner pl-3">{{$row.Code}}</code>
+							</td>
+						</tr>
+					{{end}}
 				</tbody>
 			</table>
 		</div>
--- 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;
    }
  }
--- 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 {
--- 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;