godoc: convert Markdown files to HTML during serving

For golang.org today, Markdown is converted to HTML during
the static file embedding, but that precludes using Markdown with
"live serving".

Moving the code here lets godoc itself do the conversion and
therefore works with live serving. It is also more consistent with
re-executing templates during serving for Template:true files.

When a file is .md but also has Template: true, templates apply
first, so that templates can generate Markdown.
This is reversed from what x/website was doing (Markdown before templates)
but that decision was mostly forced by doing it during static
embedding and not necessarily the right one.
There's no reason to force switching to raw HTML just because
you want to use a template.
(A template can of course still generate HTML.)

Change-Id: I7db6d54b43e45803e965df7a1ab2f26293285cfd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251343
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

godoc/markdown.go

@@ -0,0 +1,31 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package godoc
+
+import (
+	"bytes"
+
+	"github.com/yuin/goldmark"
+	"github.com/yuin/goldmark/parser"
+	"github.com/yuin/goldmark/renderer/html"
+)
+
+// renderMarkdown converts a limited and opinionated flavor of Markdown (compliant with
+// CommonMark 0.29) to HTML for the purposes of Go websites.
+//
+// The Markdown source may contain raw HTML,
+// but Go templates have already been processed.
+func renderMarkdown(src []byte) ([]byte, error) {
+	// parser.WithHeadingAttribute allows custom ids on headings.
+	// html.WithUnsafe allows use of raw HTML, which we need for tables.
+	md := goldmark.New(
+		goldmark.WithParserOptions(parser.WithHeadingAttribute()),
+		goldmark.WithRendererOptions(html.WithUnsafe()))
+	var buf bytes.Buffer
+	if err := md.Convert(src, &buf); err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
+}

godoc/meta.go

@@ -26,12 +26,15 @@ var (
 // ----------------------------------------------------------------------------
 // Documentation Metadata
 
-// TODO(adg): why are some exported and some aren't? -brad
 type Metadata struct {
+	// These fields can be set in the JSON header at the top of a doc.
 	Title    string
 	Subtitle string
-	Template bool   // execute as template
-	Path     string // canonical path for this page
+	Template bool     // execute as template
+	Path     string   // canonical path for this page
+	AltPaths []string // redirect these other paths to this page
+
+	// These are internal to the implementation.
 	filePath string // filesystem path relative to goroot
 }
 
@@ -58,7 +61,7 @@ func extractMetadata(b []byte) (meta Metadata, tail []byte, err error) {
 	return
 }
 
-// UpdateMetadata scans $GOROOT/doc for HTML files, reads their metadata,
+// UpdateMetadata scans $GOROOT/doc for HTML and Markdown files, reads their metadata,
 // and updates the DocMetadata map.
 func (c *Corpus) updateMetadata() {
 	metadata := make(map[string]*Metadata)
@@ -79,7 +82,7 @@ func (c *Corpus) updateMetadata() {
 				scan(name) // recurse
 				continue
 			}
-			if !strings.HasSuffix(name, ".html") {
+			if !strings.HasSuffix(name, ".html") && !strings.HasSuffix(name, ".md") {
 				continue
 			}
 			// Extract metadata from the file.
@@ -93,15 +96,23 @@ func (c *Corpus) updateMetadata() {
 				log.Printf("updateMetadata: %s: %v", name, err)
 				continue
 			}
+			// Present all .md as if they were .html,
+			// so that it doesn't matter which one a page is written in.
+			if strings.HasSuffix(name, ".md") {
+				name = strings.TrimSuffix(name, ".md") + ".html"
+			}
 			// Store relative filesystem path in Metadata.
 			meta.filePath = name
 			if meta.Path == "" {
-				// If no Path, canonical path is actual path.
-				meta.Path = meta.filePath
+				// If no Path, canonical path is actual path with .html removed.
+				meta.Path = strings.TrimSuffix(name, ".html")
 			}
 			// Store under both paths.
 			metadata[meta.Path] = &meta
 			metadata[meta.filePath] = &meta
+			for _, path := range meta.AltPaths {
+				metadata[path] = &meta
+			}
 		}
 	}
 	scan("/doc")

godoc/server.go

@@ -695,7 +695,15 @@ func (p *Presentation) serveDirectory(w http.ResponseWriter, r *http.Request, ab
 
 func (p *Presentation) ServeHTMLDoc(w http.ResponseWriter, r *http.Request, abspath, relpath string) {
 	// get HTML body contents
+	isMarkdown := false
 	src, err := vfs.ReadFile(p.Corpus.fs, abspath)
+	if err != nil && strings.HasSuffix(abspath, ".html") {
+		if md, errMD := vfs.ReadFile(p.Corpus.fs, strings.TrimSuffix(abspath, ".html")+".md"); errMD == nil {
+			src = md
+			isMarkdown = true
+			err = nil
+		}
+	}
 	if err != nil {
 		log.Printf("ReadFile: %s", err)
 		p.ServeError(w, r, relpath, err)
@@ -738,6 +746,18 @@ func (p *Presentation) ServeHTMLDoc(w http.ResponseWriter, r *http.Request, absp
 		src = buf.Bytes()
 	}
 
+	// Apply markdown as indicated.
+	// (Note template applies before Markdown.)
+	if isMarkdown {
+		html, err := renderMarkdown(src)
+		if err != nil {
+			log.Printf("executing markdown %s: %v", relpath, err)
+			p.ServeError(w, r, relpath, err)
+			return
+		}
+		src = html
+	}
+
 	// if it's the language spec, add tags to EBNF productions
 	if strings.HasSuffix(abspath, "go_spec.html") {
 		var buf bytes.Buffer
@@ -797,7 +817,8 @@ func (p *Presentation) serveFile(w http.ResponseWriter, r *http.Request) {
 		if redirect(w, r) {
 			return
 		}
-		if index := pathpkg.Join(abspath, "index.html"); util.IsTextFile(p.Corpus.fs, index) {
+		index := pathpkg.Join(abspath, "index.html")
+		if util.IsTextFile(p.Corpus.fs, index) || util.IsTextFile(p.Corpus.fs, pathpkg.Join(abspath, "index.md")) {
 			p.ServeHTMLDoc(w, r, index, index)
 			return
 		}

godoc/server_test.go

@@ -73,6 +73,17 @@ func F()
 	}
 }
 
+func testServeBody(t *testing.T, p *Presentation, path, body string) {
+	t.Helper()
+	r := &http.Request{URL: &url.URL{Path: path}}
+	rw := httptest.NewRecorder()
+	p.ServeFile(rw, r)
+	if rw.Code != 200 || !strings.Contains(rw.Body.String(), body) {
+		t.Fatalf("GET %s: expected 200 w/ %q: got %d w/ body:\n%s",
+			path, body, rw.Code, rw.Body)
+	}
+}
+
 func TestRedirectAndMetadata(t *testing.T) {
 	c := NewCorpus(mapfs.New(map[string]string{
 		"doc/y/index.html": "Hello, y.",
@@ -87,13 +98,13 @@ Hello, x.
 		Corpus:    c,
 		GodocHTML: template.Must(template.New("").Parse(`{{printf "%s" .Body}}`)),
 	}
-	r := &http.Request{URL: &url.URL{}}
 
 	// Test that redirect is sent back correctly.
 	// Used to panic. See golang.org/issue/40665.
 	for _, elem := range []string{"x", "y"} {
 		dir := "/doc/" + elem + "/"
-		r.URL.Path = dir + "index.html"
+
+		r := &http.Request{URL: &url.URL{Path: dir + "index.html"}}
 		rw := httptest.NewRecorder()
 		p.ServeFile(rw, r)
 		loc := rw.Result().Header.Get("Location")
@@ -101,12 +112,19 @@ Hello, x.
 			t.Errorf("GET %s: expected 301 -> %q, got %d -> %q", r.URL.Path, dir, rw.Code, loc)
 		}
 
-		r.URL.Path = dir
-		rw = httptest.NewRecorder()
-		p.ServeFile(rw, r)
-		if rw.Code != 200 || !strings.Contains(rw.Body.String(), "Hello, "+elem) {
-			t.Fatalf("GET %s: expected 200 w/ Hello, %s: got %d w/ body:\n%s",
-				r.URL.Path, elem, rw.Code, rw.Body)
-		}
+		testServeBody(t, p, dir, "Hello, "+elem)
 	}
 }
+
+func TestMarkdown(t *testing.T) {
+	p := &Presentation{
+		Corpus: NewCorpus(mapfs.New(map[string]string{
+			"doc/test.md":  "**bold**",
+			"doc/test2.md": `{{"*template*"}}`,
+		})),
+		GodocHTML: template.Must(template.New("").Parse(`{{printf "%s" .Body}}`)),
+	}
+
+	testServeBody(t, p, "/doc/test.html", "<strong>bold</strong>")
+	testServeBody(t, p, "/doc/test2.html", "<em>template</em>")
+}