Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: link to previous blames in view blame page #15171

Closed
wants to merge 9 commits into from
Closed

Feature: link to previous blames in view blame page #15171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f11b40e
Fix blame bottom line and add blame prior button
rogerluo410 Mar 26, 2021
0499082
Jump to previous parent commit from the commit.
rogerluo410 Mar 26, 2021
7ee46f0
Lint code
rogerluo410 Mar 26, 2021
36d3b83
Merge branch 'master' into dev-update-blame
rogerluo410 Mar 26, 2021
9353120
Fix previous commit link
rogerluo410 Mar 26, 2021
caeffdb
Fix previous blame link
rogerluo410 Mar 26, 2021
c6170cd
Update svg.
rogerluo410 Mar 27, 2021
45b83dc
Fix the given file not exist in the previous commit.
rogerluo410 Mar 27, 2021
78ed60a
Fix blameRow struct not export
rogerluo410 Mar 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,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"
Expand Down
81 changes: 51 additions & 30 deletions routers/repo/blame.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package repo

import (
"bytes"
"container/list"
"fmt"
"html"
Expand All @@ -26,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
Expand Down Expand Up @@ -145,6 +158,7 @@ func RefBlame(ctx *context.Context) {
}

commitNames := make(map[string]models.UserCommit)
previousCommits := make(map[string]string)
commits := list.New()

for _, part := range blameParts {
Expand All @@ -163,6 +177,17 @@ func RefBlame(ctx *context.Context) {
return
}

// Get previous closest commit sha from the commit
previousCommit, err := commit.Parent(0)
if err == nil {
// Verify the commit
if haz, _ := commit.HasPreviousCommit(previousCommit.ID); haz {
if haz1, _ := previousCommit.HasFile(ctx.Repo.TreePath); haz1 {
previousCommits[commit.ID.String()] = previousCommit.ID.String()
}
}
}

commits.PushBack(commit)

commitNames[commit.ID.String()] = models.UserCommit{}
Expand All @@ -182,32 +207,34 @@ func RefBlame(ctx *context.Context) {
return
}

renderBlame(ctx, blameParts, commitNames)
renderBlame(ctx, blameParts, commitNames, previousCommits)

ctx.HTML(200, tplBlame)
}

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))

Expand All @@ -218,33 +245,27 @@ 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 {
line += "\n"
}
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["Codes"] = rows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this to "Rows"?

ctx.Data["CommitCnt"] = commitCnt
}
39 changes: 34 additions & 5 deletions templates/repo/blame.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 := .Codes}}
<tr class="{{if and (gt $.CommitCnt 1) ($row.CommitMessage)}}bottom-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>
Expand Down
44 changes: 28 additions & 16 deletions web_src/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2219,22 +2219,25 @@ 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();

// show code view menu marker
showCodeViewMenu();
// not show in blame page
if ($('div.blame').length === 0) {
showCodeViewMenu();
}
});

$(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');
}
Expand All @@ -2244,7 +2247,10 @@ function initCodeView() {
selectRange($list, $first, $list.filter(`[rel=${m[2]}]`));

// show code view menu marker
showCodeViewMenu();
// not show in blame page
if ($('div.blame').length === 0) {
showCodeViewMenu();
}

$('html, body').scrollTop($first.offset().top - 200);
return;
Expand All @@ -2255,7 +2261,10 @@ function initCodeView() {
selectRange($list, $first);

// show code view menu marker
showCodeViewMenu();
// not show in blame page
if ($('div.blame').length === 0) {
showCodeViewMenu();
}

$('html, body').scrollTop($first.offset().top - 200);
}
Expand Down Expand Up @@ -2824,13 +2833,14 @@ function selectRange($list, $select, $from) {

// add hashchange to permalink
const $issue = $('a.ref-in-new-issue');
const matched = $issue.attr('href').match(/%23L\d+$|%23L\d+-L\d+$/);
if (matched) {
$issue.attr('href', $issue.attr('href').replace($issue.attr('href').substr(matched.index), `%23L${a}-L${b}`));
} else {
$issue.attr('href', `${$issue.attr('href')}%23L${a}-L${b}`);
if ($issue && $issue.attr('href')) {
const matched = $issue.attr('href').match(/%23L\d+$|%23L\d+-L\d+$/);
if (matched) {
$issue.attr('href', $issue.attr('href').replace($issue.attr('href').substr(matched.index), `%23L${a}-L${b}`));
} else {
$issue.attr('href', `${$issue.attr('href')}%23L${a}-L${b}`);
}
}

return;
}
}
Expand All @@ -2839,11 +2849,13 @@ function selectRange($list, $select, $from) {

// add hashchange to permalink
const $issue = $('a.ref-in-new-issue');
const matched = $issue.attr('href').match(/%23L\d+$|%23L\d+-L\d+$/);
if (matched) {
$issue.attr('href', $issue.attr('href').replace($issue.attr('href').substr(matched.index), `%23${$select.attr('rel')}`));
} else {
$issue.attr('href', `${$issue.attr('href')}%23${$select.attr('rel')}`);
if ($issue && $issue.attr('href')) {
const matched = $issue.attr('href').match(/%23L\d+$|%23L\d+-L\d+$/);
if (matched) {
$issue.attr('href', $issue.attr('href').replace($issue.attr('href').substr(matched.index), `%23${$select.attr('rel')}`));
} else {
$issue.attr('href', `${$issue.attr('href')}%23${$select.attr('rel')}`);
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions web_src/less/_base.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,14 @@ a.ui.label:hover {
margin-right: 0;
}

.lines-blame-btn {
padding-left: 10px;
padding-right: 10px;
text-align: right !important;
background-color: #f5f5f5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This background color needs to overridden in web_src/less/themes/them-arc-green.less L464:

.blame .lines-num,
.blame .lines-blame-button {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should use css vars

Copy link
Member

@6543 6543 Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

silverwind did the transition to them for colors ...

So you dont have to touch our dark theme!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, the existing override for .blame .lines-num in theme-arc-green.less can probably dropped

width: 2%;
}

.lines-num {
padding-left: 10px;
padding-right: 10px;
Expand Down Expand Up @@ -1510,6 +1518,10 @@ a.ui.label:hover {
}
}

.bottom-line-blame {
border-bottom: 1px solid var(--color-secondary);
}

.lines-code,
.lines-commit {
.bottom-line {
Expand Down