From 7e17424c7e189f91d4f1b35e2504141b1e8116f6 Mon Sep 17 00:00:00 2001 From: jaqra <48099350+jaqra@users.noreply.github.com> Date: Tue, 10 Sep 2019 12:03:30 +0300 Subject: [PATCH] Make link last commit massages in repository home page and commit tables (#8006) * Make link last commit massages in repository home page and commit tables * Use RenderCommitMessageLink instead surround with a * deleted __debug_bin file * Exclude email to link from latest commit title * Exclude email processor from commit table Co-Authored-By: mrsdizzie * Add class parameter to a html element creator functions. Make links underline dashed that are not commit * fix tests * Show dashed underline when also not hovered --- modules/markup/html.go | 73 +++++++++++++++++++++++----- modules/markup/html_internal_test.go | 36 ++++++++------ modules/templates/helper.go | 33 ++++++++++--- public/css/index.css | 8 +++ public/less/_repository.less | 36 ++++++++++++++ templates/repo/commits_table.tmpl | 3 +- templates/repo/view_list.tmpl | 3 +- 7 files changed, 154 insertions(+), 38 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index ff5b1a76e6..f07993bc4c 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -211,6 +211,40 @@ func RenderCommitMessage( return ctx.postProcess(rawHTML) } +var commitMessageSubjectProcessors = []processor{ + fullIssuePatternProcessor, + fullSha1PatternProcessor, + linkProcessor, + mentionProcessor, + issueIndexPatternProcessor, + crossReferenceIssueIndexPatternProcessor, + sha1CurrentPatternProcessor, +} + +// RenderCommitMessageSubject will use the same logic as PostProcess and +// RenderCommitMessage, but will disable the shortLinkProcessor and +// emailAddressProcessor, will add a defaultLinkProcessor if defaultLink is set, +// which changes every text node into a link to the passed default link. +func RenderCommitMessageSubject( + rawHTML []byte, + urlPrefix, defaultLink string, + metas map[string]string, +) ([]byte, error) { + ctx := &postProcessCtx{ + metas: metas, + urlPrefix: urlPrefix, + procs: commitMessageSubjectProcessors, + } + if defaultLink != "" { + // we don't have to fear data races, because being + // commitMessageSubjectProcessors of fixed len and cap, every time we + // append something to it the slice is realloc+copied, so append always + // generates the slice ex-novo. + ctx.procs = append(ctx.procs, genDefaultLinkProcessor(defaultLink)) + } + return ctx.postProcess(rawHTML) +} + // RenderDescriptionHTML will use similar logic as PostProcess, but will // use a single special linkProcessor. func RenderDescriptionHTML( @@ -296,12 +330,17 @@ func (ctx *postProcessCtx) textNode(node *html.Node) { } } -func createLink(href, content string) *html.Node { +func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), Attr: []html.Attribute{{Key: "href", Val: href}}, } + + if class != "" { + a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) + } + text := &html.Node{ Type: html.TextNode, Data: content, @@ -311,12 +350,17 @@ func createLink(href, content string) *html.Node { return a } -func createCodeLink(href, content string) *html.Node { +func createCodeLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), Attr: []html.Attribute{{Key: "href", Val: href}}, } + + if class != "" { + a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) + } + text := &html.Node{ Type: html.TextNode, Data: content, @@ -364,7 +408,7 @@ func mentionProcessor(_ *postProcessCtx, node *html.Node) { } // Replace the mention with a link to the specified user. mention := node.Data[m[2]:m[3]] - replaceContent(node, m[2], m[3], createLink(util.URLJoin(setting.AppURL, mention[1:]), mention)) + replaceContent(node, m[2], m[3], createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) } func shortLinkProcessor(ctx *postProcessCtx, node *html.Node) { @@ -541,11 +585,11 @@ func fullIssuePatternProcessor(ctx *postProcessCtx, node *html.Node) { if matchOrg == ctx.metas["user"] && matchRepo == ctx.metas["repo"] { // TODO if m[4]:m[5] is not nil, then link is to a comment, // and we should indicate that in the text somehow - replaceContent(node, m[0], m[1], createLink(link, id)) + replaceContent(node, m[0], m[1], createLink(link, id, "issue")) } else { orgRepoID := matchOrg + "/" + matchRepo + id - replaceContent(node, m[0], m[1], createLink(link, orgRepoID)) + replaceContent(node, m[0], m[1], createLink(link, orgRepoID, "issue")) } } @@ -573,9 +617,9 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { } else { ctx.metas["index"] = id[1:] } - link = createLink(com.Expand(ctx.metas["format"], ctx.metas), id) + link = createLink(com.Expand(ctx.metas["format"], ctx.metas), id, "issue") } else { - link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", id[1:]), id) + link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", id[1:]), id, "issue") } replaceContent(node, match[2], match[3], link) } @@ -591,7 +635,7 @@ func crossReferenceIssueIndexPatternProcessor(ctx *postProcessCtx, node *html.No repo, issue := parts[0], parts[1] replaceContent(node, m[2], m[3], - createLink(util.URLJoin(setting.AppURL, repo, "issues", issue), ref)) + createLink(util.URLJoin(setting.AppURL, repo, "issues", issue), ref, issue)) } // fullSha1PatternProcessor renders SHA containing URLs @@ -642,7 +686,7 @@ func fullSha1PatternProcessor(ctx *postProcessCtx, node *html.Node) { text += " (" + hash + ")" } - replaceContent(node, start, end, createCodeLink(urlFull, text)) + replaceContent(node, start, end, createCodeLink(urlFull, text, "commit")) } // sha1CurrentPatternProcessor renders SHA1 strings to corresponding links that @@ -672,7 +716,7 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { } replaceContent(node, m[2], m[3], - createCodeLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "commit", hash), base.ShortSha(hash))) + createCodeLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "commit", hash), base.ShortSha(hash), "commit")) } // emailAddressProcessor replaces raw email addresses with a mailto: link. @@ -682,7 +726,7 @@ func emailAddressProcessor(ctx *postProcessCtx, node *html.Node) { return } mail := node.Data[m[2]:m[3]] - replaceContent(node, m[2], m[3], createLink("mailto:"+mail, mail)) + replaceContent(node, m[2], m[3], createLink("mailto:"+mail, mail, "mailto")) } // linkProcessor creates links for any HTTP or HTTPS URL not captured by @@ -693,7 +737,7 @@ func linkProcessor(ctx *postProcessCtx, node *html.Node) { return } uri := node.Data[m[0]:m[1]] - replaceContent(node, m[0], m[1], createLink(uri, uri)) + replaceContent(node, m[0], m[1], createLink(uri, uri, "link")) } func genDefaultLinkProcessor(defaultLink string) processor { @@ -707,7 +751,10 @@ func genDefaultLinkProcessor(defaultLink string) processor { node.Type = html.ElementNode node.Data = "a" node.DataAtom = atom.A - node.Attr = []html.Attribute{{Key: "href", Val: defaultLink}} + node.Attr = []html.Attribute{ + {Key: "href", Val: defaultLink}, + {Key: "class", Val: "default-link"}, + } node.FirstChild, node.LastChild = ch, ch } } diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index bb47ba3b2c..2824ce3e68 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -20,18 +20,22 @@ const Repo = "gogits/gogs" const AppSubURL = AppURL + Repo + "/" // alphanumLink an HTML link to an alphanumeric-style issue -func alphanumIssueLink(baseURL string, name string) string { - return link(util.URLJoin(baseURL, name), name) +func alphanumIssueLink(baseURL, class, name string) string { + return link(util.URLJoin(baseURL, name), class, name) } // numericLink an HTML to a numeric-style issue -func numericIssueLink(baseURL string, index int) string { - return link(util.URLJoin(baseURL, strconv.Itoa(index)), fmt.Sprintf("#%d", index)) +func numericIssueLink(baseURL, class string, index int) string { + return link(util.URLJoin(baseURL, strconv.Itoa(index)), class, fmt.Sprintf("#%d", index)) } // link an HTML link -func link(href, contents string) string { - return fmt.Sprintf("%s", href, contents) +func link(href, class, contents string) string { + if class != "" { + class = " class=\"" + class + "\"" + } + + return fmt.Sprintf("%s", href, class, contents) } var numericMetas = map[string]string{ @@ -89,13 +93,13 @@ func TestRender_IssueIndexPattern2(t *testing.T) { test := func(s, expectedFmt string, indices ...int) { links := make([]interface{}, len(indices)) for i, index := range indices { - links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), index) + links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", index) } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &postProcessCtx{metas: localMetas}) for i, index := range indices { - links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", index) + links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", "issue", index) } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &postProcessCtx{metas: numericMetas}) @@ -158,7 +162,7 @@ func TestRender_IssueIndexPattern4(t *testing.T) { test := func(s, expectedFmt string, names ...string) { links := make([]interface{}, len(names)) for i, name := range names { - links[i] = alphanumIssueLink("https://someurl.com/someUser/someRepo/", name) + links[i] = alphanumIssueLink("https://someurl.com/someUser/someRepo/", "issue", name) } expected := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expected, &postProcessCtx{metas: alphanumericMetas}) @@ -197,17 +201,17 @@ func TestRender_AutoLink(t *testing.T) { // render valid issue URLs test(util.URLJoin(setting.AppSubURL, "issues", "3333"), - numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), 3333)) + numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", 3333)) // render valid commit URLs tmp := util.URLJoin(AppSubURL, "commit", "d8a994ef243349f321568f9e36d5c3f444b99cae") - test(tmp, "d8a994ef24") + test(tmp, "d8a994ef24") tmp += "#diff-2" - test(tmp, "d8a994ef24 (diff-2)") + test(tmp, "d8a994ef24 (diff-2)") // render other commit URLs tmp = "https://external-link.gitea.io/go-gitea/gitea/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2" - test(tmp, "d8a994ef24 (diff-2)") + test(tmp, "d8a994ef24 (diff-2)") } func TestRender_FullIssueURLs(t *testing.T) { @@ -228,11 +232,11 @@ func TestRender_FullIssueURLs(t *testing.T) { test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6", "Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6") test("Look here http://localhost:3000/person/repo/issues/4", - `Look here person/repo#4`) + `Look here person/repo#4`) test("http://localhost:3000/person/repo/issues/4#issuecomment-1234", - `person/repo#4`) + `person/repo#4`) test("http://localhost:3000/gogits/gogs/issues/4", - `#4`) + `#4`) } func TestRegExp_issueNumericPattern(t *testing.T) { diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 147df3a788..ba61dd5eef 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -118,13 +118,14 @@ func NewFuncMap() []template.FuncMap { "EscapePound": func(str string) string { return strings.NewReplacer("%", "%25", "#", "%23", " ", "%20", "?", "%3F").Replace(str) }, - "PathEscapeSegments": util.PathEscapeSegments, - "URLJoin": util.URLJoin, - "RenderCommitMessage": RenderCommitMessage, - "RenderCommitMessageLink": RenderCommitMessageLink, - "RenderCommitBody": RenderCommitBody, - "RenderNote": RenderNote, - "IsMultilineCommitMessage": IsMultilineCommitMessage, + "PathEscapeSegments": util.PathEscapeSegments, + "URLJoin": util.URLJoin, + "RenderCommitMessage": RenderCommitMessage, + "RenderCommitMessageLink": RenderCommitMessageLink, + "RenderCommitMessageLinkSubject": RenderCommitMessageLinkSubject, + "RenderCommitBody": RenderCommitBody, + "RenderNote": RenderNote, + "IsMultilineCommitMessage": IsMultilineCommitMessage, "ThemeColorMetaTag": func() string { return setting.UI.ThemeColorMetaTag }, @@ -322,6 +323,24 @@ func RenderCommitMessageLink(msg, urlPrefix, urlDefault string, metas map[string return template.HTML(msgLines[0]) } +// RenderCommitMessageLinkSubject renders commit message as a XXS-safe link to +// the provided default url, handling for special links without email to links. +func RenderCommitMessageLinkSubject(msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML { + cleanMsg := template.HTMLEscapeString(msg) + // we can safely assume that it will not return any error, since there + // shouldn't be any special HTML. + fullMessage, err := markup.RenderCommitMessageSubject([]byte(cleanMsg), urlPrefix, urlDefault, metas) + if err != nil { + log.Error("RenderCommitMessageSubject: %v", err) + return "" + } + msgLines := strings.Split(strings.TrimSpace(string(fullMessage)), "\n") + if len(msgLines) == 0 { + return template.HTML("") + } + return template.HTML(msgLines[0]) +} + // RenderCommitBody extracts the body of a commit message without its title. func RenderCommitBody(msg, urlPrefix string, metas map[string]string) template.HTML { cleanMsg := template.HTMLEscapeString(msg) diff --git a/public/css/index.css b/public/css/index.css index a68547ffb5..2cc7b42cef 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -466,6 +466,10 @@ footer .ui.left,footer .ui.right{line-height:40px} } .repository.file.list #repo-files-table thead th{padding-top:8px;padding-bottom:5px;font-weight:400} .repository.file.list #repo-files-table thead .ui.avatar{margin-bottom:5px} +.repository.file.list #repo-files-table thead .commit-summary a{text-decoration:underline;-webkit-text-decoration-style:dashed;text-decoration-style:dashed} +.repository.file.list #repo-files-table thead .commit-summary a:hover{-webkit-text-decoration-style:solid;text-decoration-style:solid} +.repository.file.list #repo-files-table thead .commit-summary a.default-link{text-decoration:none} +.repository.file.list #repo-files-table thead .commit-summary a.default-link:hover{text-decoration:underline;-webkit-text-decoration-style:solid;text-decoration-style:solid} .repository.file.list #repo-files-table tbody .octicon{margin-left:3px;margin-right:5px;color:#777} .repository.file.list #repo-files-table tbody .octicon.octicon-mail-reply{margin-right:10px} .repository.file.list #repo-files-table tbody .octicon.octicon-file-directory,.repository.file.list #repo-files-table tbody .octicon.octicon-file-submodule,.repository.file.list #repo-files-table tbody .octicon.octicon-file-symlink-directory{color:#1e70bf} @@ -829,6 +833,10 @@ footer .ui.left,footer .ui.right{line-height:40px} .stats-table .table-cell.tiny{height:.5em} tbody.commit-list{vertical-align:baseline} .commit-list .message-wrapper{overflow:hidden;text-overflow:ellipsis;max-width:calc(100% - 50px);display:inline-block;vertical-align:middle} +.commit-list .commit-summary a{text-decoration:underline;-webkit-text-decoration-style:dashed;text-decoration-style:dashed} +.commit-list .commit-summary a:hover{-webkit-text-decoration-style:solid;text-decoration-style:solid} +.commit-list .commit-summary a.default-link{text-decoration:none} +.commit-list .commit-summary a.default-link:hover{text-decoration:underline;-webkit-text-decoration-style:solid;text-decoration-style:solid} .commit-list .commit-status-link{display:inline-block;vertical-align:middle} .commit-body{white-space:pre-wrap} .git-notes.top{text-align:left} diff --git a/public/less/_repository.less b/public/less/_repository.less index d6572fc306..a64763b393 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -277,6 +277,24 @@ .ui.avatar { margin-bottom: 5px; } + + .commit-summary a { + text-decoration: underline; + text-decoration-style: dashed; + + &:hover { + text-decoration-style: solid; + } + + &.default-link { + text-decoration: none; + + &:hover { + text-decoration: underline; + text-decoration-style: solid; + } + } + } } tbody { @@ -2188,6 +2206,24 @@ tbody.commit-list { vertical-align: middle; } +.commit-list .commit-summary a { + text-decoration: underline; + text-decoration-style: dashed; + + &:hover { + text-decoration-style: solid; + } + + &.default-link { + text-decoration: none; + + &:hover { + text-decoration: underline; + text-decoration-style: solid; + } + } +} + .commit-list .commit-status-link { display: inline-block; vertical-align: middle; diff --git a/templates/repo/commits_table.tmpl b/templates/repo/commits_table.tmpl index 6561c0f258..e11bbee0e8 100644 --- a/templates/repo/commits_table.tmpl +++ b/templates/repo/commits_table.tmpl @@ -71,7 +71,8 @@ - {{RenderCommitMessage .Message $.RepoLink $.Repository.ComposeMetas}} + {{ $commitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Username $.Reponame .ID }} + {{RenderCommitMessageLinkSubject .Message $.RepoLink $commitLink $.Repository.ComposeMetas}} {{if IsMultilineCommitMessage .Message}} diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl index c35f5d7e35..4c96109e8b 100644 --- a/templates/repo/view_list.tmpl +++ b/templates/repo/view_list.tmpl @@ -28,7 +28,8 @@ {{end}} {{template "repo/commit_status" .LatestCommitStatus}} - {{RenderCommitMessage .LatestCommit.Message $.RepoLink $.Repository.ComposeMetas}} + {{ $commitLink:= printf "%s/commit/%s" .RepoLink .LatestCommit.ID }} + {{RenderCommitMessageLinkSubject .LatestCommit.Message $.RepoLink $commitLink $.Repository.ComposeMetas}} {{if IsMultilineCommitMessage .LatestCommit.Message}}