From 63126c6359a693345a3a81b22e0f95660b248044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sat, 3 Dec 2022 12:33:48 +0100 Subject: [PATCH] markup/goldmark: Add removeSurroundingParagraph for Markdown images * Removes any surrounding paragraph nodes * And transfers any attributes from the surrounding paragraph down to the image node * Adds IsBlock and Ordinal (zero based) field to the image context passed to the image render hooks IsBlock is set to true if `wrapStandAloneImageWithinParagraph = false` and the image's parent node has only one child. Closes #8362 Fixes #10492 Fixes #10494 Fixes #10501 --- markup/converter/hooks/hooks.go | 6 ++ markup/goldmark/convert.go | 11 +- markup/goldmark/goldmark_config/config.go | 8 +- markup/goldmark/images/integration_test.go | 113 +++++++++++++++++++++ markup/goldmark/images/transform.go | 77 ++++++++++++++ markup/goldmark/links/integration_test.go | 113 +++++++++++++++++++++ markup/goldmark/links/transform.go | 73 +++++++++++++ markup/goldmark/render_hooks.go | 89 ++++++++++++---- markup/internal/attributes/attributes.go | 3 + 9 files changed, 469 insertions(+), 24 deletions(-) create mode 100644 markup/goldmark/images/integration_test.go create mode 100644 markup/goldmark/images/transform.go create mode 100644 markup/goldmark/links/integration_test.go create mode 100644 markup/goldmark/links/transform.go diff --git a/markup/converter/hooks/hooks.go b/markup/converter/hooks/hooks.go index a8666bdf0..a59da939e 100644 --- a/markup/converter/hooks/hooks.go +++ b/markup/converter/hooks/hooks.go @@ -37,6 +37,12 @@ type LinkContext interface { PlainText() string } +type ImageLinkContext interface { + LinkContext + IsBlock() bool + Ordinal() int +} + type CodeblockContext interface { AttributesProvider text.Positioner diff --git a/markup/goldmark/convert.go b/markup/goldmark/convert.go index ba85831b0..a179cd233 100644 --- a/markup/goldmark/convert.go +++ b/markup/goldmark/convert.go @@ -17,12 +17,12 @@ package goldmark import ( "bytes" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/markup/goldmark/codeblocks" + "github.com/gohugoio/hugo/markup/goldmark/images" "github.com/gohugoio/hugo/markup/goldmark/internal/extensions/attributes" "github.com/gohugoio/hugo/markup/goldmark/internal/render" - "github.com/gohugoio/hugo/identity" - "github.com/gohugoio/hugo/markup/converter" "github.com/gohugoio/hugo/markup/tableofcontents" "github.com/yuin/goldmark" @@ -33,6 +33,10 @@ import ( "github.com/yuin/goldmark/text" ) +const ( + internalAttrPrefix = "_h__" +) + // Provider is the package entry point. var Provider converter.ProviderProvider = provide{} @@ -92,6 +96,8 @@ func newMarkdown(pcfg converter.ProviderConfig) goldmark.Markdown { parserOptions []parser.Option ) + extensions = append(extensions, images.New(cfg.Parser.WrapStandAloneImageWithinParagraph)) + if mcfg.Highlight.CodeFences { extensions = append(extensions, codeblocks.New()) } @@ -131,7 +137,6 @@ func newMarkdown(pcfg converter.ProviderConfig) goldmark.Markdown { if cfg.Parser.Attribute.Title { parserOptions = append(parserOptions, parser.WithAttribute()) } - if cfg.Parser.Attribute.Block { extensions = append(extensions, attributes.New()) } diff --git a/markup/goldmark/goldmark_config/config.go b/markup/goldmark/goldmark_config/config.go index a3238091b..ff0b6bbef 100644 --- a/markup/goldmark/goldmark_config/config.go +++ b/markup/goldmark/goldmark_config/config.go @@ -36,8 +36,9 @@ var Default = Config{ Unsafe: false, }, Parser: Parser{ - AutoHeadingID: true, - AutoHeadingIDType: AutoHeadingIDTypeGitHub, + AutoHeadingID: true, + AutoHeadingIDType: AutoHeadingIDTypeGitHub, + WrapStandAloneImageWithinParagraph: true, Attribute: ParserAttribute{ Title: true, Block: false, @@ -88,6 +89,9 @@ type Parser struct { // Enables custom attributes. Attribute ParserAttribute + + // Whether to wrap stand-alone images within a paragraph or not. + WrapStandAloneImageWithinParagraph bool } type ParserAttribute struct { diff --git a/markup/goldmark/images/integration_test.go b/markup/goldmark/images/integration_test.go new file mode 100644 index 000000000..e8d1b880e --- /dev/null +++ b/markup/goldmark/images/integration_test.go @@ -0,0 +1,113 @@ +package images_test + +import ( + "strings" + "testing" + + "github.com/gohugoio/hugo/hugolib" +) + +func TestDisableWrapStandAloneImageWithinParagraph(t *testing.T) { + t.Parallel() + + filesTemplate := ` +-- config.toml -- +[markup.goldmark.renderer] + unsafe = false +[markup.goldmark.parser] +wrapStandAloneImageWithinParagraph = CONFIG_VALUE +[markup.goldmark.parser.attribute] + block = true + title = true +-- content/p1.md -- +--- +title: "p1" +--- + +This is an inline image: ![Inline Image](/inline.jpg). Some more text. + +![Block Image](/block.jpg) +{.b} + + +-- layouts/_default/single.html -- +{{ .Content }} +` + + t.Run("With Hook, no wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "false") + files = files + `-- layouts/_default/_markup/render-image.html -- +{{ if .IsBlock }} +
+ {{ .Text }} +
+{{ else }} + {{ .Text }} +{{ end }} +` + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", + "This is an inline image: \n\t\"Inline\n. Some more text.

", + "
\n\t\"Block", + ) + }) + + t.Run("With Hook, wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "true") + files = files + `-- layouts/_default/_markup/render-image.html -- +{{ if .IsBlock }} +
+ {{ .Text }} +
+{{ else }} + {{ .Text }} +{{ end }} +` + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", + "This is an inline image: \n\t\"Inline\n. Some more text.

", + "

\n\t\"Block\n

", + ) + }) + + t.Run("No Hook, no wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "false") + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", "

This is an inline image: \"Inline. Some more text.

\n\"Block") + }) + + t.Run("No Hook, wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "true") + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", "

\"Block

") + }) + +} diff --git a/markup/goldmark/images/transform.go b/markup/goldmark/images/transform.go new file mode 100644 index 000000000..4eb793ae1 --- /dev/null +++ b/markup/goldmark/images/transform.go @@ -0,0 +1,77 @@ +package images + +import ( + "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" +) + +type ( + imagesExtension struct { + wrapStandAloneImageWithinParagraph bool + } +) + +const ( + // Used to signal to the rendering step that an image is used in a block context. + // Dont's change this; the prefix must match the internalAttrPrefix in the root goldmark package. + AttrIsBlock = "_h__isBlock" + AttrOrdinal = "_h__ordinal" +) + +func New(wrapStandAloneImageWithinParagraph bool) goldmark.Extender { + return &imagesExtension{wrapStandAloneImageWithinParagraph: wrapStandAloneImageWithinParagraph} +} + +func (e *imagesExtension) Extend(m goldmark.Markdown) { + m.Parser().AddOptions( + parser.WithASTTransformers( + util.Prioritized(&Transformer{wrapStandAloneImageWithinParagraph: e.wrapStandAloneImageWithinParagraph}, 300), + ), + ) +} + +type Transformer struct { + wrapStandAloneImageWithinParagraph bool +} + +// Transform transforms the provided Markdown AST. +func (t *Transformer) Transform(doc *ast.Document, reader text.Reader, pctx parser.Context) { + var ordinal int + ast.Walk(doc, func(node ast.Node, enter bool) (ast.WalkStatus, error) { + if !enter { + return ast.WalkContinue, nil + } + + if n, ok := node.(*ast.Image); ok { + parent := n.Parent() + n.SetAttributeString(AttrOrdinal, ordinal) + ordinal++ + + if !t.wrapStandAloneImageWithinParagraph { + isBlock := parent.ChildCount() == 1 + if isBlock { + n.SetAttributeString(AttrIsBlock, true) + } + + if isBlock && parent.Kind() == ast.KindParagraph { + for _, attr := range parent.Attributes() { + // Transfer any attribute set down to the image. + // Image elements does not support attributes on its own, + // so it's safe to just set without checking first. + n.SetAttribute(attr.Name, attr.Value) + } + grandParent := parent.Parent() + grandParent.ReplaceChild(grandParent, parent, n) + } + } + + } + + return ast.WalkContinue, nil + + }) + +} diff --git a/markup/goldmark/links/integration_test.go b/markup/goldmark/links/integration_test.go new file mode 100644 index 000000000..20d4d74b1 --- /dev/null +++ b/markup/goldmark/links/integration_test.go @@ -0,0 +1,113 @@ +package images_test + +import ( + "strings" + "testing" + + "github.com/gohugoio/hugo/hugolib" +) + +func TestDisableWrapStandAloneImageWithinParagraph(t *testing.T) { + t.Parallel() + + filesTemplate := ` +-- config.toml -- +[markup.goldmark.renderer] + unsafe = false +[markup.goldmark.parser] +wrapStandAloneImageWithinParagraph = CONFIG_VALUE +[markup.goldmark.parser.attribute] + block = true + title = true +-- content/p1.md -- +--- +title: "p1" +--- + +This is an inline image: ![Inline Image](/inline.jpg). Some more text. + +![Block Image](/block.jpg) +{.b} + + +-- layouts/_default/single.html -- +{{ .Content }} +` + + t.Run("With Hook, no wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "false") + files = files + `-- layouts/_default/_markup/render-image.html -- +{{ if .IsBlock }} +
+ {{ .Text }}|{{ .Ordinal }} +
+{{ else }} + {{ .Text }}|{{ .Ordinal }} +{{ end }} +` + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", + "This is an inline image: \n\t\"Inline\n. Some more text.

", + "
\n\t\"Block", + ) + }) + + t.Run("With Hook, wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "true") + files = files + `-- layouts/_default/_markup/render-image.html -- +{{ if .IsBlock }} +
+ {{ .Text }} +
+{{ else }} + {{ .Text }} +{{ end }} +` + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", + "This is an inline image: \n\t\"Inline\n. Some more text.

", + "

\n\t\"Block\n

", + ) + }) + + t.Run("No Hook, no wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "false") + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", "

This is an inline image: \"Inline. Some more text.

\n\"Block") + }) + + t.Run("No Hook, wrap", func(t *testing.T) { + files := strings.ReplaceAll(filesTemplate, "CONFIG_VALUE", "true") + b := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + NeedsOsFS: false, + }, + ).Build() + + b.AssertFileContent("public/p1/index.html", "

\"Block

") + }) + +} diff --git a/markup/goldmark/links/transform.go b/markup/goldmark/links/transform.go new file mode 100644 index 000000000..b4f6b6dc5 --- /dev/null +++ b/markup/goldmark/links/transform.go @@ -0,0 +1,73 @@ +package images + +import ( + "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" +) + +type ( + linksExtension struct { + wrapStandAloneImageWithinParagraph bool + } +) + +const ( + // Used to signal to the rendering step that an image is used in a block context. + // Dont's change this; the prefix must match the internalAttrPrefix in the root goldmark package. + AttrIsBlock = "_h__isBlock" +) + +func New(wrapStandAloneImageWithinParagraph bool) goldmark.Extender { + return &linksExtension{wrapStandAloneImageWithinParagraph: wrapStandAloneImageWithinParagraph} +} + +func (e *linksExtension) Extend(m goldmark.Markdown) { + m.Parser().AddOptions( + parser.WithASTTransformers( + util.Prioritized(&Transformer{wrapStandAloneImageWithinParagraph: e.wrapStandAloneImageWithinParagraph}, 300), + ), + ) +} + +type Transformer struct { + wrapStandAloneImageWithinParagraph bool +} + +// Transform transforms the provided Markdown AST. +func (t *Transformer) Transform(doc *ast.Document, reader text.Reader, pctx parser.Context) { + ast.Walk(doc, func(node ast.Node, enter bool) (ast.WalkStatus, error) { + if !enter { + return ast.WalkContinue, nil + } + + if n, ok := node.(*ast.Image); ok { + parent := n.Parent() + + if !t.wrapStandAloneImageWithinParagraph { + isBlock := parent.ChildCount() == 1 + if isBlock { + n.SetAttributeString(AttrIsBlock, true) + } + + if isBlock && parent.Kind() == ast.KindParagraph { + for _, attr := range parent.Attributes() { + // Transfer any attribute set down to the image. + // Image elements does not support attributes on its own, + // so it's safe to just set without checking first. + n.SetAttribute(attr.Name, attr.Value) + } + grandParent := parent.Parent() + grandParent.ReplaceChild(grandParent, parent, n) + } + } + + } + + return ast.WalkContinue, nil + + }) + +} diff --git a/markup/goldmark/render_hooks.go b/markup/goldmark/render_hooks.go index e28f816d6..f36f9f4e6 100644 --- a/markup/goldmark/render_hooks.go +++ b/markup/goldmark/render_hooks.go @@ -20,6 +20,7 @@ import ( "github.com/gohugoio/hugo/common/types/hstring" "github.com/gohugoio/hugo/markup/converter/hooks" "github.com/gohugoio/hugo/markup/goldmark/goldmark_config" + "github.com/gohugoio/hugo/markup/goldmark/images" "github.com/gohugoio/hugo/markup/goldmark/internal/render" "github.com/gohugoio/hugo/markup/internal/attributes" @@ -52,16 +53,13 @@ type linkContext struct { title string text hstring.RenderedString plainText string + *attributes.AttributesHolder } func (ctx linkContext) Destination() string { return ctx.destination } -func (ctx linkContext) Resolved() bool { - return false -} - func (ctx linkContext) Page() any { return ctx.page } @@ -78,6 +76,20 @@ func (ctx linkContext) Title() string { return ctx.title } +type imageLinkContext struct { + linkContext + ordinal int + isBlock bool +} + +func (ctx imageLinkContext) IsBlock() bool { + return ctx.isBlock +} + +func (ctx imageLinkContext) Ordinal() int { + return ctx.ordinal +} + type headingContext struct { page any level int @@ -151,14 +163,36 @@ func (r *hookedRenderer) renderImage(w util.BufWriter, source []byte, node ast.N text := ctx.Buffer.Bytes()[pos:] ctx.Buffer.Truncate(pos) + var ( + isBlock bool + ordinal int + ) + if b, ok := n.AttributeString(images.AttrIsBlock); ok && b.(bool) { + isBlock = true + } + if n, ok := n.AttributeString(images.AttrOrdinal); ok { + ordinal = n.(int) + } + + // We use the attributes to signal from the parser whether the image is in + // a block context or not. + // We may find a better way to do that, but for now, we'll need to remove any + // internal attributes before rendering. + attrs := r.filterInternalAttributes(n.Attributes()) + err := lr.RenderLink( w, - linkContext{ - page: ctx.DocumentContext().Document, - destination: string(n.Destination), - title: string(n.Title), - text: hstring.RenderedString(text), - plainText: string(n.Text(source)), + imageLinkContext{ + linkContext: linkContext{ + page: ctx.DocumentContext().Document, + destination: string(n.Destination), + title: string(n.Title), + text: hstring.RenderedString(text), + plainText: string(n.Text(source)), + AttributesHolder: attributes.New(attrs, attributes.AttributesOwnerGeneral), + }, + ordinal: ordinal, + isBlock: isBlock, }, ) @@ -167,6 +201,17 @@ func (r *hookedRenderer) renderImage(w util.BufWriter, source []byte, node ast.N return ast.WalkContinue, err } +func (r *hookedRenderer) filterInternalAttributes(attrs []ast.Attribute) []ast.Attribute { + n := 0 + for _, x := range attrs { + if !bytes.HasPrefix(x.Name, []byte(internalAttrPrefix)) { + attrs[n] = x + n++ + } + } + return attrs[:n] +} + // Fall back to the default Goldmark render funcs. Method below borrowed from: // https://github.com/yuin/goldmark/blob/b611cd333a492416b56aa8d94b04a67bf0096ab2/renderer/html/html.go#L404 func (r *hookedRenderer) renderImageDefault(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { @@ -186,6 +231,10 @@ func (r *hookedRenderer) renderImageDefault(w util.BufWriter, source []byte, nod r.Writer.Write(w, n.Title) _ = w.WriteByte('"') } + if n.Attributes() != nil { + attrs := r.filterInternalAttributes(n.Attributes()) + attributes.RenderASTAttributes(w, attrs...) + } if r.XHTML { _, _ = w.WriteString(" />") } else { @@ -224,11 +273,12 @@ func (r *hookedRenderer) renderLink(w util.BufWriter, source []byte, node ast.No err := lr.RenderLink( w, linkContext{ - page: ctx.DocumentContext().Document, - destination: string(n.Destination), - title: string(n.Title), - text: hstring.RenderedString(text), - plainText: string(n.Text(source)), + page: ctx.DocumentContext().Document, + destination: string(n.Destination), + title: string(n.Title), + text: hstring.RenderedString(text), + plainText: string(n.Text(source)), + AttributesHolder: attributes.Empty, }, ) @@ -292,10 +342,11 @@ func (r *hookedRenderer) renderAutoLink(w util.BufWriter, source []byte, node as err := lr.RenderLink( w, linkContext{ - page: ctx.DocumentContext().Document, - destination: url, - text: hstring.RenderedString(label), - plainText: label, + page: ctx.DocumentContext().Document, + destination: url, + text: hstring.RenderedString(label), + plainText: label, + AttributesHolder: attributes.Empty, }, ) diff --git a/markup/internal/attributes/attributes.go b/markup/internal/attributes/attributes.go index 688740983..a67b51acb 100644 --- a/markup/internal/attributes/attributes.go +++ b/markup/internal/attributes/attributes.go @@ -126,6 +126,9 @@ func (a Attribute) ValueString() string { return cast.ToString(a.Value) } +// Empty holds no attributes. +var Empty = &AttributesHolder{} + type AttributesHolder struct { // What we get from Goldmark. attributes []Attribute