Skip to content

Commit

Permalink
markup/goldmark: Add removeSurroundingParagraph for Markdown images
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
bep committed Dec 5, 2022
1 parent 535ea8c commit 63126c6
Show file tree
Hide file tree
Showing 9 changed files with 469 additions and 24 deletions.
6 changes: 6 additions & 0 deletions markup/converter/hooks/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ type LinkContext interface {
PlainText() string
}

type ImageLinkContext interface {
LinkContext
IsBlock() bool
Ordinal() int
}

type CodeblockContext interface {
AttributesProvider
text.Positioner
Expand Down
11 changes: 8 additions & 3 deletions markup/goldmark/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -33,6 +33,10 @@ import (
"github.com/yuin/goldmark/text"
)

const (
internalAttrPrefix = "_h__"
)

// Provider is the package entry point.
var Provider converter.ProviderProvider = provide{}

Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down
8 changes: 6 additions & 2 deletions markup/goldmark/goldmark_config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
113 changes: 113 additions & 0 deletions markup/goldmark/images/integration_test.go
Original file line number Diff line number Diff line change
@@ -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>")
})

}
77 changes: 77 additions & 0 deletions markup/goldmark/images/transform.go
Original file line number Diff line number Diff line change
@@ -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

})

}
113 changes: 113 additions & 0 deletions markup/goldmark/links/integration_test.go
Original file line number Diff line number Diff line change
@@ -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>")
})

}

0 comments on commit 63126c6

Please sign in to comment.