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.

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

Conversation

rogerluo410
Copy link
Contributor

Feature:

  1. Fix bottom lines mismatched between commit info and code line in view blame page.
  2. Add previous blame links

Related PR:
#15148

Feature request issue:
#15109

Hi, @noerw, I noticed you created a PR for the request, perhaps this one could more helpful to what we ask for.

Performance:
blame

@jolheiser
Copy link
Member

Perhaps the versions octicon, to match GH?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 26, 2021
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Mar 26, 2021
@lunny lunny added this to the 1.15.0 milestone Mar 26, 2021
@noerw
Copy link
Member

noerw commented Mar 27, 2021

The linter complains:
routers/repo/blame.go:28:6: exported type BlameRow should have comment or be unexported

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@dc56fb7). Click here to learn what that means.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #15171   +/-   ##
=========================================
  Coverage          ?   43.56%           
=========================================
  Files             ?      668           
  Lines             ?    80604           
  Branches          ?        0           
=========================================
  Hits              ?    35113           
  Misses            ?    39784           
  Partials          ?     5707           
Impacted Files Coverage Δ
routers/repo/blame.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc56fb7...78ed60a. Read the comment docs.

@rogerluo410
Copy link
Contributor Author

The linter complains:
routers/repo/blame.go:28:6: exported type BlameRow should have comment or be unexported

Fixed

@rogerluo410
Copy link
Contributor Author

Perhaps the versions octicon, to match GH?

Updated svg

@zeripath
Copy link
Contributor

Please resolve conflicts.

Copy link
Member

@noerw noerw left a comment

Choose a reason for hiding this comment

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

One nit, otherwise looks good :)
Please merge current main branch

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

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

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"?

@6543
Copy link
Member

6543 commented Jun 27, 2021

@noerw @rogerluo410 thanks for the work -> merged as 9c6aeb4

@zeripath zeripath removed this from the 1.15.0 milestone Jun 28, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants