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
This commit is contained in:
Bjørn Erik Pedersen 2022-12-03 12:33:48 +01:00
parent 535ea8cc9b
commit 63126c6359
9 changed files with 469 additions and 24 deletions

View file

@ -37,6 +37,12 @@ type LinkContext interface {
PlainText() string PlainText() string
} }
type ImageLinkContext interface {
LinkContext
IsBlock() bool
Ordinal() int
}
type CodeblockContext interface { type CodeblockContext interface {
AttributesProvider AttributesProvider
text.Positioner text.Positioner

View file

@ -17,12 +17,12 @@ package goldmark
import ( import (
"bytes" "bytes"
"github.com/gohugoio/hugo/identity"
"github.com/gohugoio/hugo/markup/goldmark/codeblocks" "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/extensions/attributes"
"github.com/gohugoio/hugo/markup/goldmark/internal/render" "github.com/gohugoio/hugo/markup/goldmark/internal/render"
"github.com/gohugoio/hugo/identity"
"github.com/gohugoio/hugo/markup/converter" "github.com/gohugoio/hugo/markup/converter"
"github.com/gohugoio/hugo/markup/tableofcontents" "github.com/gohugoio/hugo/markup/tableofcontents"
"github.com/yuin/goldmark" "github.com/yuin/goldmark"
@ -33,6 +33,10 @@ import (
"github.com/yuin/goldmark/text" "github.com/yuin/goldmark/text"
) )
const (
internalAttrPrefix = "_h__"
)
// Provider is the package entry point. // Provider is the package entry point.
var Provider converter.ProviderProvider = provide{} var Provider converter.ProviderProvider = provide{}
@ -92,6 +96,8 @@ func newMarkdown(pcfg converter.ProviderConfig) goldmark.Markdown {
parserOptions []parser.Option parserOptions []parser.Option
) )
extensions = append(extensions, images.New(cfg.Parser.WrapStandAloneImageWithinParagraph))
if mcfg.Highlight.CodeFences { if mcfg.Highlight.CodeFences {
extensions = append(extensions, codeblocks.New()) extensions = append(extensions, codeblocks.New())
} }
@ -131,7 +137,6 @@ func newMarkdown(pcfg converter.ProviderConfig) goldmark.Markdown {
if cfg.Parser.Attribute.Title { if cfg.Parser.Attribute.Title {
parserOptions = append(parserOptions, parser.WithAttribute()) parserOptions = append(parserOptions, parser.WithAttribute())
} }
if cfg.Parser.Attribute.Block { if cfg.Parser.Attribute.Block {
extensions = append(extensions, attributes.New()) extensions = append(extensions, attributes.New())
} }

View file

@ -36,8 +36,9 @@ var Default = Config{
Unsafe: false, Unsafe: false,
}, },
Parser: Parser{ Parser: Parser{
AutoHeadingID: true, AutoHeadingID: true,
AutoHeadingIDType: AutoHeadingIDTypeGitHub, AutoHeadingIDType: AutoHeadingIDTypeGitHub,
WrapStandAloneImageWithinParagraph: true,
Attribute: ParserAttribute{ Attribute: ParserAttribute{
Title: true, Title: true,
Block: false, Block: false,
@ -88,6 +89,9 @@ type Parser struct {
// Enables custom attributes. // Enables custom attributes.
Attribute ParserAttribute Attribute ParserAttribute
// Whether to wrap stand-alone images within a paragraph or not.
WrapStandAloneImageWithinParagraph bool
} }
type ParserAttribute struct { type ParserAttribute struct {

View file

@ -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 }}
<figure class="{{ .Attributes.class }}">
<img src="{{ .Destination | safeURL }}" alt="{{ .Text }}" />
</figure>
{{ else }}
<img src="{{ .Destination | safeURL }}" alt="{{ .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<img src=\"/inline.jpg\" alt=\"Inline Image\" />\n. Some more text.</p>",
"<figure class=\"b\">\n\t<img src=\"/block.jpg\" alt=\"Block Image\" />",
)
})
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 }}
<figure class="{{ .Attributes.class }}">
<img src="{{ .Destination | safeURL }}" alt="{{ .Text }}" />
</figure>
{{ else }}
<img src="{{ .Destination | safeURL }}" alt="{{ .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<img src=\"/inline.jpg\" alt=\"Inline Image\" />\n. Some more text.</p>",
"<p class=\"b\">\n\t<img src=\"/block.jpg\" alt=\"Block Image\" />\n</p>",
)
})
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", "<p>This is an inline image: <img src=\"/inline.jpg\" alt=\"Inline Image\">. Some more text.</p>\n<img src=\"/block.jpg\" alt=\"Block Image\" class=\"b\">")
})
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", "<p class=\"b\"><img src=\"/block.jpg\" alt=\"Block Image\"></p>")
})
}

View file

@ -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
})
}

View file

@ -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 }}
<figure class="{{ .Attributes.class }}">
<img src="{{ .Destination | safeURL }}" alt="{{ .Text }}|{{ .Ordinal }}" />
</figure>
{{ else }}
<img src="{{ .Destination | safeURL }}" alt="{{ .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<img src=\"/inline.jpg\" alt=\"Inline Image|0\" />\n. Some more text.</p>",
"<figure class=\"b\">\n\t<img src=\"/block.jpg\" alt=\"Block Image|1\" />",
)
})
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 }}
<figure class="{{ .Attributes.class }}">
<img src="{{ .Destination | safeURL }}" alt="{{ .Text }}" />
</figure>
{{ else }}
<img src="{{ .Destination | safeURL }}" alt="{{ .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<img src=\"/inline.jpg\" alt=\"Inline Image\" />\n. Some more text.</p>",
"<p class=\"b\">\n\t<img src=\"/block.jpg\" alt=\"Block Image\" />\n</p>",
)
})
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", "<p>This is an inline image: <img src=\"/inline.jpg\" alt=\"Inline Image\">. Some more text.</p>\n<img src=\"/block.jpg\" alt=\"Block Image\" class=\"b\">")
})
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", "<p class=\"b\"><img src=\"/block.jpg\" alt=\"Block Image\"></p>")
})
}

View file

@ -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
})
}

View file

@ -20,6 +20,7 @@ import (
"github.com/gohugoio/hugo/common/types/hstring" "github.com/gohugoio/hugo/common/types/hstring"
"github.com/gohugoio/hugo/markup/converter/hooks" "github.com/gohugoio/hugo/markup/converter/hooks"
"github.com/gohugoio/hugo/markup/goldmark/goldmark_config" "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/goldmark/internal/render"
"github.com/gohugoio/hugo/markup/internal/attributes" "github.com/gohugoio/hugo/markup/internal/attributes"
@ -52,16 +53,13 @@ type linkContext struct {
title string title string
text hstring.RenderedString text hstring.RenderedString
plainText string plainText string
*attributes.AttributesHolder
} }
func (ctx linkContext) Destination() string { func (ctx linkContext) Destination() string {
return ctx.destination return ctx.destination
} }
func (ctx linkContext) Resolved() bool {
return false
}
func (ctx linkContext) Page() any { func (ctx linkContext) Page() any {
return ctx.page return ctx.page
} }
@ -78,6 +76,20 @@ func (ctx linkContext) Title() string {
return ctx.title 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 { type headingContext struct {
page any page any
level int level int
@ -151,14 +163,36 @@ func (r *hookedRenderer) renderImage(w util.BufWriter, source []byte, node ast.N
text := ctx.Buffer.Bytes()[pos:] text := ctx.Buffer.Bytes()[pos:]
ctx.Buffer.Truncate(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( err := lr.RenderLink(
w, w,
linkContext{ imageLinkContext{
page: ctx.DocumentContext().Document, linkContext: linkContext{
destination: string(n.Destination), page: ctx.DocumentContext().Document,
title: string(n.Title), destination: string(n.Destination),
text: hstring.RenderedString(text), title: string(n.Title),
plainText: string(n.Text(source)), 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 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: // Fall back to the default Goldmark render funcs. Method below borrowed from:
// https://github.com/yuin/goldmark/blob/b611cd333a492416b56aa8d94b04a67bf0096ab2/renderer/html/html.go#L404 // 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) { 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) r.Writer.Write(w, n.Title)
_ = w.WriteByte('"') _ = w.WriteByte('"')
} }
if n.Attributes() != nil {
attrs := r.filterInternalAttributes(n.Attributes())
attributes.RenderASTAttributes(w, attrs...)
}
if r.XHTML { if r.XHTML {
_, _ = w.WriteString(" />") _, _ = w.WriteString(" />")
} else { } else {
@ -224,11 +273,12 @@ func (r *hookedRenderer) renderLink(w util.BufWriter, source []byte, node ast.No
err := lr.RenderLink( err := lr.RenderLink(
w, w,
linkContext{ linkContext{
page: ctx.DocumentContext().Document, page: ctx.DocumentContext().Document,
destination: string(n.Destination), destination: string(n.Destination),
title: string(n.Title), title: string(n.Title),
text: hstring.RenderedString(text), text: hstring.RenderedString(text),
plainText: string(n.Text(source)), 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( err := lr.RenderLink(
w, w,
linkContext{ linkContext{
page: ctx.DocumentContext().Document, page: ctx.DocumentContext().Document,
destination: url, destination: url,
text: hstring.RenderedString(label), text: hstring.RenderedString(label),
plainText: label, plainText: label,
AttributesHolder: attributes.Empty,
}, },
) )

View file

@ -126,6 +126,9 @@ func (a Attribute) ValueString() string {
return cast.ToString(a.Value) return cast.ToString(a.Value)
} }
// Empty holds no attributes.
var Empty = &AttributesHolder{}
type AttributesHolder struct { type AttributesHolder struct {
// What we get from Goldmark. // What we get from Goldmark.
attributes []Attribute attributes []Attribute