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

x/tools/present: "new" markdown syntax turns all sections into HTML #48841

Open
sbinet opened this issue Oct 7, 2021 · 4 comments
Open

x/tools/present: "new" markdown syntax turns all sections into HTML #48841

sbinet opened this issue Oct 7, 2021 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@sbinet
Copy link
Member

sbinet commented Oct 7, 2021

What version of Go are you using (go version)?

$ go version
go version devel go1.18-6f74ed06c5 Thu Oct 7 03:41:40 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes.
also with x/tools@v0.1.7

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/binet/.cache/go-build"
GOENV="/home/binet/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/binet/dev/go/gocode/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/binet/dev/go/gocode"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/binet/sdk/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/binet/sdk/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.18-6f74ed06c5 Thu Oct 7 03:41:40 2021 +0000"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build351486629=/tmp/go-build -gno-record-gcc-switches"

What did you do?

	var r = os.Stdin
	var input = flag.Arg(0)
	doc, err := present.Parse(r, input, 0)
	if err != nil {
		log.Fatal(err)
	}

What did you expect to see?

a present.Doc value with Sections filled with Elem []present.Elem fields of various present.Xyz types (present.Text, present.List, ...)

What did you see instead?

when a talk.slide file is using the "new" Markdown syntax, all Sections have only one Elem of type present.HTML.

each slide of the full talk has already been rendered to html using the underlying markdown package (yuin/goldmark), by passing the expected present.Xyz structure.

my sbinet/present-tex command was using the "data model" of present to render it to LaTeX+Beamer.

couldn't present.parseSections be modified to translate a goldmark AST into the "old" present data model instead of eagerly rendering everything to html ?

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Oct 7, 2021
@gopherbot gopherbot added this to the Unreleased milestone Oct 7, 2021
@sbinet
Copy link
Member Author

sbinet commented Oct 7, 2021

alternatively, one could perhaps add a field to present.Context to specify a goldmark.Renderer (defaulting to html)

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 7, 2021
@mknyszek
Copy link
Contributor

mknyszek commented Oct 7, 2021

CC @rsc via https://dev.golang.org/owners

@sbinet
Copy link
Member Author

sbinet commented Oct 8, 2021

thinking a bit more on this issue, and as (AFAICT) the original present AST can not represent the subset of CommonMark that the "new" format supports, it probably makes more sense to go the alternate way I've sketched above.

something along the lines of:

diff --git a/present/parse.go b/present/parse.go
index 4294ea5f..18c15fe9 100644
--- a/present/parse.go
+++ b/present/parse.go
@@ -273,6 +273,9 @@ func (l *Lines) nextNonEmpty() (text string, ok bool) {
 type Context struct {
        // ReadFile reads the file named by filename and returns the contents.
        ReadFile func(filename string) ([]byte, error)
+
+       // Render renders the given block of CommonMark as a present element.
+       Render func(p []byte) (Elem, error)
 }
 
 // ParseMode represents flags for the Parse function.
@@ -344,7 +347,10 @@ func (ctx *Context) Parse(r io.Reader, name string, mode ParseMode) (*Doc, error
 // Parse parses a document from r. Parse reads assets used by the presentation
 // from the file system using ioutil.ReadFile.
 func Parse(r io.Reader, name string, mode ParseMode) (*Doc, error) {
-       ctx := Context{ReadFile: ioutil.ReadFile}
+       ctx := Context{
+               ReadFile: ioutil.ReadFile,
+               Render:   renderAsHTML,
+       }
        return ctx.Parse(r, name, mode)
 }
 
@@ -512,11 +518,12 @@ func parseSections(ctx *Context, name, prefix string, lines *Lines, number []int
                                                block[i] = line
                                        }
                                }
-                               html, err := renderMarkdown([]byte(strings.Join(block, "\n")))
+
+                               elem, err := ctx.Render([]byte(strings.Join(block, "\n")))
                                if err != nil {
                                        return nil, err
                                }
-                               e = HTML{HTML: html}
+                               e = elem
 
                        default:
                                // Collect text lines.
@@ -698,16 +705,80 @@ func trimSpeakerNote(s string) string {
        return strings.TrimPrefix(s, ": ")
 }
 
-func renderMarkdown(input []byte) (template.HTML, error) {
+func renderAsHTML(input []byte) (Elem, error) {
        md := goldmark.New(goldmark.WithRendererOptions(html.WithUnsafe()))
        reader := text.NewReader(input)
        doc := md.Parser().Parse(reader)
        fixupMarkdown(doc)
        var b strings.Builder
        if err := md.Renderer().Render(&b, input, doc); err != nil {
-               return "", err
+               return nil, err
        }
-       return template.HTML(b.String()), nil
+       return HTML{HTML: template.HTML(b.String())}, nil
+}

sbinet added a commit to sbinet-staging/tools that referenced this issue Oct 11, 2021
This CL adds a new field to Context, to more easily access and customize
the CommonMark rendering process from external packages.

Fixes golang/go#48841.

Change-Id: I0bb91480bb1acbf66c117dc266b52ccb62b35c6f
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/355049 mentions this issue: present: introduce Context.Render func field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants