-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Improve and unify commit author rendering #36094
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
base: main
Are you sure you want to change the base?
Conversation
|
I don't think showing email in the UI is a good idea, maybe only for authorized users |
|
It was already shown previously as a tooltip on the avatar in the commit view, but was not consistent. |
|
And yes, I agree it should be hidden for anonymous users, but imho thats a separate topic and we need to validate all places where we pass these addresses to the frontend. |
|
I think I could actually try to fix this at least in these 3 places, make a subtemplate for "avatar and username" and do the logged in check there, wip until then. |
|
Since the repository is public, it can be accessed freely. Anyone can clone it and obtain the email information locally anyway. |
|
I guess it has a small benefit against scrape bots. |
|
Yes, GitHub also does not show emails in git for the very same reason most probably |
|
This proves harder than I thought because these templates are such a mess, there is |
|
Shared template added but there is a unresolved issue on latest commit page and commit view:
|
|
Fixed above by not passing these invalid The E-Mail adress is also no longer rendered into HTML for signed-out users. |
| @@ -0,0 +1,56 @@ | |||
| {{/* Template Attributes: | |||
| * User: The user associated with the commit, if any | |||
| * Commit: The commit | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you should pass so many different structs.
Commit is not really used. What you need is a "git user" or "gitea user".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit is used for .Commit.Author vs .Author. Maybe it can be unified, not sure. The three pages all pass different context variables.
| {{if and .User.FullName DefaultShowFullName}} | ||
| {{$username = .User.FullName}} | ||
| {{else if .User.Name}} | ||
| {{$username = .User.Name}} | ||
| {{end}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if *User will also be compatible with .Author.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if
*Userwill also be compatible with.Author.
No. More complicated.
| "Commit" .LatestCommit | ||
| "AvatarSize" 24 | ||
| "AuthorBold" true | ||
| "IsSigned" $.IsSigned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to pass "IsSigned"
Use ctx.RootData.IsSigned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, didn't know that existed.


Uh oh!
There was an error while loading. Please reload this page.