From 1ab16e48cccc086e7f97fb3ae8a293fe47a3a452 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Tue, 18 Apr 2023 03:05:19 +0800
Subject: [PATCH] Improve Wiki TOC (#24137)

The old code has a lot of technical debts, eg: `repo/wiki/view.tmpl` /
`Iterate`

This PR improves the Wiki TOC display and improves the code.

---------

Co-authored-by: delvh <dev.lh@web.de>
---
 modules/markup/markdown/convertyaml.go       |  2 +-
 modules/markup/markdown/goldmark.go          | 34 ++++----
 modules/markup/markdown/markdown.go          | 23 +++---
 modules/markup/markdown/renderconfig.go      | 81 ++++++++++----------
 modules/markup/markdown/renderconfig_test.go | 10 +--
 modules/markup/markdown/toc.go               |  8 +-
 modules/markup/renderer.go                   | 13 +++-
 modules/templates/helper.go                  |  7 --
 routers/web/base.go                          | 24 +++---
 routers/web/repo/wiki.go                     | 10 ++-
 templates/repo/wiki/view.tmpl                | 24 ++----
 web_src/css/repository.css                   | 11 +--
 12 files changed, 129 insertions(+), 118 deletions(-)

diff --git a/modules/markup/markdown/convertyaml.go b/modules/markup/markdown/convertyaml.go
index 6e90847e06..1675b68be2 100644
--- a/modules/markup/markdown/convertyaml.go
+++ b/modules/markup/markdown/convertyaml.go
@@ -34,7 +34,7 @@ func nodeToTable(meta *yaml.Node) ast.Node {
 
 func mappingNodeToTable(meta *yaml.Node) ast.Node {
 	table := east.NewTable()
-	alignments := []east.Alignment{}
+	alignments := make([]east.Alignment, 0, len(meta.Content)/2)
 	for i := 0; i < len(meta.Content); i += 2 {
 		alignments = append(alignments, east.AlignNone)
 	}
diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go
index 50b438219b..816e93b700 100644
--- a/modules/markup/markdown/goldmark.go
+++ b/modules/markup/markdown/goldmark.go
@@ -34,16 +34,17 @@ type ASTTransformer struct{}
 // Transform transforms the given AST tree.
 func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc parser.Context) {
 	firstChild := node.FirstChild()
-	createTOC := false
+	tocMode := ""
 	ctx := pc.Get(renderContextKey).(*markup.RenderContext)
 	rc := pc.Get(renderConfigKey).(*RenderConfig)
+
+	tocList := make([]markup.Header, 0, 20)
 	if rc.yamlNode != nil {
 		metaNode := rc.toMetaNode()
 		if metaNode != nil {
 			node.InsertBefore(node, firstChild, metaNode)
 		}
-		createTOC = rc.TOC
-		ctx.TableOfContents = make([]markup.Header, 0, 100)
+		tocMode = rc.TOC
 	}
 
 	attentionMarkedBlockquotes := make(container.Set[*ast.Blockquote])
@@ -59,15 +60,15 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 					v.SetAttribute(attr.Name, []byte(fmt.Sprintf("%v", attr.Value)))
 				}
 			}
-			text := n.Text(reader.Source())
+			txt := n.Text(reader.Source())
 			header := markup.Header{
-				Text:  util.BytesToReadOnlyString(text),
+				Text:  util.BytesToReadOnlyString(txt),
 				Level: v.Level,
 			}
 			if id, found := v.AttributeString("id"); found {
 				header.ID = util.BytesToReadOnlyString(id.([]byte))
 			}
-			ctx.TableOfContents = append(ctx.TableOfContents, header)
+			tocList = append(tocList, header)
 		case *ast.Image:
 			// Images need two things:
 			//
@@ -201,14 +202,15 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 		return ast.WalkContinue, nil
 	})
 
-	if createTOC && len(ctx.TableOfContents) > 0 {
-		lang := rc.Lang
-		if len(lang) == 0 {
-			lang = setting.Langs[0]
-		}
-		tocNode := createTOCNode(ctx.TableOfContents, lang)
-		if tocNode != nil {
+	showTocInMain := tocMode == "true" /* old behavior, in main view */ || tocMode == "main"
+	showTocInSidebar := !showTocInMain && tocMode != "false" // not hidden, not main, then show it in sidebar
+	if len(tocList) > 0 && (showTocInMain || showTocInSidebar) {
+		if showTocInMain {
+			tocNode := createTOCNode(tocList, rc.Lang, nil)
 			node.InsertBefore(node, firstChild, tocNode)
+		} else {
+			tocNode := createTOCNode(tocList, rc.Lang, map[string]string{"open": "open"})
+			ctx.SidebarTocNode = tocNode
 		}
 	}
 
@@ -373,7 +375,11 @@ func (r *HTMLRenderer) renderDocument(w util.BufWriter, source []byte, node ast.
 func (r *HTMLRenderer) renderDetails(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
 	var err error
 	if entering {
-		_, err = w.WriteString("<details>")
+		if _, err = w.WriteString("<details"); err != nil {
+			return ast.WalkStop, err
+		}
+		html.RenderAttributes(w, node, nil)
+		_, err = w.WriteString(">")
 	} else {
 		_, err = w.WriteString("</details>")
 	}
diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go
index f1ffea8872..d4a7195dc5 100644
--- a/modules/markup/markdown/markdown.go
+++ b/modules/markup/markdown/markdown.go
@@ -29,8 +29,8 @@ import (
 )
 
 var (
-	converter goldmark.Markdown
-	once      = sync.Once{}
+	specMarkdown     goldmark.Markdown
+	specMarkdownOnce sync.Once
 )
 
 var (
@@ -56,7 +56,7 @@ func (l *limitWriter) Write(data []byte) (int, error) {
 		if err != nil {
 			return n, err
 		}
-		return n, fmt.Errorf("Rendered content too large - truncating render")
+		return n, fmt.Errorf("rendered content too large - truncating render")
 	}
 	n, err := l.w.Write(data)
 	l.sum += int64(n)
@@ -73,10 +73,10 @@ func newParserContext(ctx *markup.RenderContext) parser.Context {
 	return pc
 }
 
-// actualRender renders Markdown to HTML without handling special links.
-func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
-	once.Do(func() {
-		converter = goldmark.New(
+// SpecializedMarkdown sets up the Gitea specific markdown extensions
+func SpecializedMarkdown() goldmark.Markdown {
+	specMarkdownOnce.Do(func() {
+		specMarkdown = goldmark.New(
 			goldmark.WithExtensions(
 				extension.NewTable(
 					extension.WithTableCellAlignMethod(extension.TableCellAlignAttribute)),
@@ -139,13 +139,18 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer)
 		)
 
 		// Override the original Tasklist renderer!
-		converter.Renderer().AddOptions(
+		specMarkdown.Renderer().AddOptions(
 			renderer.WithNodeRenderers(
 				util.Prioritized(NewHTMLRenderer(), 10),
 			),
 		)
 	})
+	return specMarkdown
+}
 
+// actualRender renders Markdown to HTML without handling special links.
+func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
+	converter := SpecializedMarkdown()
 	lw := &limitWriter{
 		w:     output,
 		limit: setting.UI.MaxDisplayFileSize * 3,
@@ -174,7 +179,7 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer)
 	buf = giteautil.NormalizeEOL(buf)
 
 	rc := &RenderConfig{
-		Meta: "table",
+		Meta: renderMetaModeFromString(string(ctx.RenderMetaAs)),
 		Icon: "table",
 		Lang: "",
 	}
diff --git a/modules/markup/markdown/renderconfig.go b/modules/markup/markdown/renderconfig.go
index f9c9cbc5f4..691df74312 100644
--- a/modules/markup/markdown/renderconfig.go
+++ b/modules/markup/markdown/renderconfig.go
@@ -7,32 +7,42 @@ import (
 	"fmt"
 	"strings"
 
+	"code.gitea.io/gitea/modules/markup"
+
 	"github.com/yuin/goldmark/ast"
 	"gopkg.in/yaml.v3"
 )
 
 // RenderConfig represents rendering configuration for this file
 type RenderConfig struct {
-	Meta     string
+	Meta     markup.RenderMetaMode
 	Icon     string
-	TOC      bool
+	TOC      string // "false": hide,  "side"/empty: in sidebar,  "main"/"true": in main view
 	Lang     string
 	yamlNode *yaml.Node
 }
 
+func renderMetaModeFromString(s string) markup.RenderMetaMode {
+	switch strings.TrimSpace(strings.ToLower(s)) {
+	case "none":
+		return markup.RenderMetaAsNone
+	case "table":
+		return markup.RenderMetaAsTable
+	default: // "details"
+		return markup.RenderMetaAsDetails
+	}
+}
+
 // UnmarshalYAML implement yaml.v3 UnmarshalYAML
 func (rc *RenderConfig) UnmarshalYAML(value *yaml.Node) error {
 	if rc == nil {
-		rc = &RenderConfig{
-			Meta: "table",
-			Icon: "table",
-			Lang: "",
-		}
+		return nil
 	}
+
 	rc.yamlNode = value
 
 	type commonRenderConfig struct {
-		TOC  bool   `yaml:"include_toc"`
+		TOC  string `yaml:"include_toc"`
 		Lang string `yaml:"lang"`
 	}
 	var basic commonRenderConfig
@@ -54,58 +64,45 @@ func (rc *RenderConfig) UnmarshalYAML(value *yaml.Node) error {
 
 	if err := value.Decode(&stringBasic); err == nil {
 		if stringBasic.Gitea != "" {
-			switch strings.TrimSpace(strings.ToLower(stringBasic.Gitea)) {
-			case "none":
-				rc.Meta = "none"
-			case "table":
-				rc.Meta = "table"
-			default: // "details"
-				rc.Meta = "details"
-			}
+			rc.Meta = renderMetaModeFromString(stringBasic.Gitea)
 		}
 		return nil
 	}
 
-	type giteaControl struct {
+	type yamlRenderConfig struct {
 		Meta *string `yaml:"meta"`
 		Icon *string `yaml:"details_icon"`
-		TOC  *bool   `yaml:"include_toc"`
+		TOC  *string `yaml:"include_toc"`
 		Lang *string `yaml:"lang"`
 	}
 
-	type complexGiteaConfig struct {
-		Gitea *giteaControl `yaml:"gitea"`
-	}
-	var complex complexGiteaConfig
-	if err := value.Decode(&complex); err != nil {
-		return fmt.Errorf("unable to decode into complexRenderConfig %w", err)
+	type yamlRenderConfigWrapper struct {
+		Gitea *yamlRenderConfig `yaml:"gitea"`
 	}
 
-	if complex.Gitea == nil {
+	var cfg yamlRenderConfigWrapper
+	if err := value.Decode(&cfg); err != nil {
+		return fmt.Errorf("unable to decode into yamlRenderConfigWrapper %w", err)
+	}
+
+	if cfg.Gitea == nil {
 		return nil
 	}
 
-	if complex.Gitea.Meta != nil {
-		switch strings.TrimSpace(strings.ToLower(*complex.Gitea.Meta)) {
-		case "none":
-			rc.Meta = "none"
-		case "table":
-			rc.Meta = "table"
-		default: // "details"
-			rc.Meta = "details"
-		}
+	if cfg.Gitea.Meta != nil {
+		rc.Meta = renderMetaModeFromString(*cfg.Gitea.Meta)
 	}
 
-	if complex.Gitea.Icon != nil {
-		rc.Icon = strings.TrimSpace(strings.ToLower(*complex.Gitea.Icon))
+	if cfg.Gitea.Icon != nil {
+		rc.Icon = strings.TrimSpace(strings.ToLower(*cfg.Gitea.Icon))
 	}
 
-	if complex.Gitea.Lang != nil && *complex.Gitea.Lang != "" {
-		rc.Lang = *complex.Gitea.Lang
+	if cfg.Gitea.Lang != nil && *cfg.Gitea.Lang != "" {
+		rc.Lang = *cfg.Gitea.Lang
 	}
 
-	if complex.Gitea.TOC != nil {
-		rc.TOC = *complex.Gitea.TOC
+	if cfg.Gitea.TOC != nil {
+		rc.TOC = *cfg.Gitea.TOC
 	}
 
 	return nil
@@ -116,9 +113,9 @@ func (rc *RenderConfig) toMetaNode() ast.Node {
 		return nil
 	}
 	switch rc.Meta {
-	case "table":
+	case markup.RenderMetaAsTable:
 		return nodeToTable(rc.yamlNode)
-	case "details":
+	case markup.RenderMetaAsDetails:
 		return nodeToDetails(rc.yamlNode, rc.Icon)
 	default:
 		return nil
diff --git a/modules/markup/markdown/renderconfig_test.go b/modules/markup/markdown/renderconfig_test.go
index f7f5e885a3..c53acdc77a 100644
--- a/modules/markup/markdown/renderconfig_test.go
+++ b/modules/markup/markdown/renderconfig_test.go
@@ -60,7 +60,7 @@ func TestRenderConfig_UnmarshalYAML(t *testing.T) {
 		},
 		{
 			"toc", &RenderConfig{
-				TOC:  true,
+				TOC:  "true",
 				Meta: "table",
 				Icon: "table",
 				Lang: "",
@@ -68,7 +68,7 @@ func TestRenderConfig_UnmarshalYAML(t *testing.T) {
 		},
 		{
 			"tocfalse", &RenderConfig{
-				TOC:  false,
+				TOC:  "false",
 				Meta: "table",
 				Icon: "table",
 				Lang: "",
@@ -78,7 +78,7 @@ func TestRenderConfig_UnmarshalYAML(t *testing.T) {
 			"toclang", &RenderConfig{
 				Meta: "table",
 				Icon: "table",
-				TOC:  true,
+				TOC:  "true",
 				Lang: "testlang",
 			}, `
 				include_toc: true
@@ -120,7 +120,7 @@ func TestRenderConfig_UnmarshalYAML(t *testing.T) {
 			"complex2", &RenderConfig{
 				Lang: "two",
 				Meta: "table",
-				TOC:  true,
+				TOC:  "true",
 				Icon: "smiley",
 			}, `
 	lang: one
@@ -155,7 +155,7 @@ func TestRenderConfig_UnmarshalYAML(t *testing.T) {
 				t.Errorf("Lang Expected %s Got %s", tt.expected.Lang, got.Lang)
 			}
 			if got.TOC != tt.expected.TOC {
-				t.Errorf("TOC Expected %t Got %t", tt.expected.TOC, got.TOC)
+				t.Errorf("TOC Expected %q Got %q", tt.expected.TOC, got.TOC)
 			}
 		})
 	}
diff --git a/modules/markup/markdown/toc.go b/modules/markup/markdown/toc.go
index 3715f031af..9602040931 100644
--- a/modules/markup/markdown/toc.go
+++ b/modules/markup/markdown/toc.go
@@ -13,10 +13,14 @@ import (
 	"github.com/yuin/goldmark/ast"
 )
 
-func createTOCNode(toc []markup.Header, lang string) ast.Node {
+func createTOCNode(toc []markup.Header, lang string, detailsAttrs map[string]string) ast.Node {
 	details := NewDetails()
 	summary := NewSummary()
 
+	for k, v := range detailsAttrs {
+		details.SetAttributeString(k, []byte(v))
+	}
+
 	summary.AppendChild(summary, ast.NewString([]byte(translation.NewLocale(lang).Tr("toc"))))
 	details.AppendChild(details, summary)
 	ul := ast.NewList('-')
@@ -40,7 +44,7 @@ func createTOCNode(toc []markup.Header, lang string) ast.Node {
 		}
 		li := ast.NewListItem(currentLevel * 2)
 		a := ast.NewLink()
-		a.Destination = []byte(fmt.Sprintf("#%s", url.PathEscape(header.ID)))
+		a.Destination = []byte(fmt.Sprintf("#%s", url.QueryEscape(header.ID)))
 		a.AppendChild(a, ast.NewString([]byte(header.Text)))
 		li.AppendChild(li, a)
 		ul.AppendChild(ul, li)
diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go
index e934aed925..f2477f1e9e 100644
--- a/modules/markup/renderer.go
+++ b/modules/markup/renderer.go
@@ -16,6 +16,16 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/yuin/goldmark/ast"
+)
+
+type RenderMetaMode string
+
+const (
+	RenderMetaAsDetails RenderMetaMode = "details" // default
+	RenderMetaAsNone    RenderMetaMode = "none"
+	RenderMetaAsTable   RenderMetaMode = "table"
 )
 
 type ProcessorHelper struct {
@@ -63,7 +73,8 @@ type RenderContext struct {
 	GitRepo          *git.Repository
 	ShaExistCache    map[string]bool
 	cancelFn         func()
-	TableOfContents  []Header
+	SidebarTocNode   ast.Node
+	RenderMetaAs     RenderMetaMode
 	InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page
 }
 
diff --git a/modules/templates/helper.go b/modules/templates/helper.go
index 27d6000daf..24687a4606 100644
--- a/modules/templates/helper.go
+++ b/modules/templates/helper.go
@@ -171,13 +171,6 @@ func NewFuncMap() []template.FuncMap {
 			}
 			return false
 		},
-		"Iterate": func(arg interface{}) (items []int64) {
-			count, _ := util.ToInt64(arg)
-			for i := int64(0); i < count; i++ {
-				items = append(items, i)
-			}
-			return items
-		},
 
 		// -----------------------------------------------------------------
 		// setting
diff --git a/routers/web/base.go b/routers/web/base.go
index da18a75643..79991d89db 100644
--- a/routers/web/base.go
+++ b/routers/web/base.go
@@ -143,31 +143,29 @@ func Recovery(ctx goctx.Context) func(next http.Handler) http.Handler {
 						"locale":     lc,
 					}
 
-					user := context.GetContextUser(req)
+					// TODO: this recovery handler is usually called without Gitea's web context, so we shouldn't touch that context too much
+					// Otherwise, the 500 page may cause new panics, eg: cache.GetContextWithData, it makes the developer&users couldn't find the original panic
+					user := context.GetContextUser(req) // almost always nil
 					if user == nil {
 						// Get user from session if logged in - do not attempt to sign-in
 						user = auth.SessionUser(sessionStore)
 					}
-					if user != nil {
-						store["IsSigned"] = true
-						store["SignedUser"] = user
-						store["SignedUserID"] = user.ID
-						store["SignedUserName"] = user.Name
-						store["IsAdmin"] = user.IsAdmin
-					} else {
-						store["SignedUserID"] = int64(0)
-						store["SignedUserName"] = ""
-					}
 
 					httpcache.SetCacheControlInHeader(w.Header(), 0, "no-transform")
 					w.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
 
-					if !setting.IsProd {
+					if !setting.IsProd || (user != nil && user.IsAdmin) {
 						store["ErrorMsg"] = combinedErr
 					}
+
+					defer func() {
+						if err := recover(); err != nil {
+							log.Error("HTML render in Recovery handler panics again: %v", err)
+						}
+					}()
 					err = rnd.HTML(w, http.StatusInternalServerError, "status/500", templates.BaseVars().Merge(store))
 					if err != nil {
-						log.Error("%v", err)
+						log.Error("HTML render in Recovery handler fails again: %v", err)
 					}
 				}
 			}()
diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go
index fe2becb7bb..0c5c5eed7d 100644
--- a/routers/web/repo/wiki.go
+++ b/routers/web/repo/wiki.go
@@ -298,7 +298,15 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 		ctx.Data["footerPresent"] = false
 	}
 
-	ctx.Data["toc"] = rctx.TableOfContents
+	if rctx.SidebarTocNode != nil {
+		sb := &strings.Builder{}
+		err = markdown.SpecializedMarkdown().Renderer().Render(sb, nil, rctx.SidebarTocNode)
+		if err != nil {
+			log.Error("Failed to render wiki sidebar TOC: %v", err)
+		} else {
+			ctx.Data["sidebarTocContent"] = sb.String()
+		}
+	}
 
 	// get commit count - wiki revisions
 	commitsCount, _ := wikiRepo.FileCommitsCount(wiki_service.DefaultBranch, pageFilename)
diff --git a/templates/repo/wiki/view.tmpl b/templates/repo/wiki/view.tmpl
index a1d1c04751..318006d96e 100644
--- a/templates/repo/wiki/view.tmpl
+++ b/templates/repo/wiki/view.tmpl
@@ -65,28 +65,16 @@
 				<p>{{.FormatWarning}}</p>
 			</div>
 		{{end}}
-		<div class="ui gt-mt-0 {{if or .sidebarPresent .toc}}grid equal width{{end}}">
-			<div class="ui {{if or .sidebarPresent .toc}}eleven wide column{{else}}gt-ml-0{{end}} segment markup wiki-content-main">
+		<div class="ui gt-mt-0 {{if or .sidebarPresent .sidebarTocContent}}grid equal width{{end}}">
+			<div class="ui {{if or .sidebarPresent .sidebarTocContent}}eleven wide column{{else}}gt-ml-0{{end}} segment markup wiki-content-main">
 				{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 				{{.content | Safe}}
 			</div>
-			{{if or .sidebarPresent .toc}}
-			<div class="column" style="padding-top: 0;">
-				{{if .toc}}
+			{{if or .sidebarPresent .sidebarTocContent}}
+			<div class="column gt-pt-0">
+				{{if .sidebarTocContent}}
 					<div class="ui segment wiki-content-toc">
-						<details open>
-							<summary>
-								<div class="ui header">{{.locale.Tr "toc"}}</div>
-							</summary>
-							{{$level := 0}}
-							{{range .toc}}
-								{{if lt $level .Level}}{{range Iterate (Eval .Level "-" $level)}}<ul>{{end}}{{end}}
-								{{if gt $level .Level}}{{range Iterate (Eval $level "-" .Level)}}</ul>{{end}}{{end}}
-								{{$level = .Level}}
-								<li><a href="#{{.ID}}">{{.Text}}</a></li>
-							{{end}}
-							{{range Iterate $level}}</ul>{{end}}
-						</details>
+						{{.sidebarTocContent | Safe}}
 					</div>
 				{{end}}
 				{{if .sidebarPresent}}
diff --git a/web_src/css/repository.css b/web_src/css/repository.css
index 5c90385628..05e50548d9 100644
--- a/web_src/css/repository.css
+++ b/web_src/css/repository.css
@@ -3261,14 +3261,15 @@ td.blob-excerpt {
   display: none;
 }
 
-.wiki-content-toc > ul > li {
-  margin-bottom: 4px;
-}
-
 .wiki-content-toc ul {
   margin: 0;
   list-style: none;
-  padding-left: 1em;
+  padding: 5px 0 5px 1em;
+}
+
+.wiki-content-toc ul ul {
+  border-left:  1px var(--color-secondary);
+  border-left-style: dashed;
 }
 
 /* fomantic's last-child selector does not work with hidden last child */