Rendering "class" attribute

[zzwx]

Hi @yuin
I'm trying to append class="..." to all img tags and wondering if something like this would make sense to add (of course it's simply a hard-coded example for "class" attribute only) :

zzwx-forks@11441f5

This way users wouldn't have to completely rewrite render function in case something simple as adding a class is needed and they don't want to possibly break the code when the library gets updated.

This is my use case:

case *ast.Image:
  if entering {
    n.SetAttributeString("class", "img-fluid")
  }

[yuin]

You can do this without forking. You just embed html.Renderer in your NodeRenderer like the following:

type MyRenderer strutct {
    Renderer renderer.Renderer
    // etc
}

func NewMyRenderer() renderer.Renderer {
    return &MyRenderer{ html.NewRenderer() }
}

func (r *MyRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) {
    r.Renderer.RegisterFuncs(reg)
    reg.Register(ast.KindImage, /* your rendering function */ )
}

[tommiv]

@yuin I tried to use this snippet to customize rendering a little bit, but it looks like renderer.Renderer doesn't implement RegisterFuncs().

[zzwx]

@yuin that works, but it would be nice to avoid that in cases like this.

If one copy-pastes your original implementation and fixes it according to their need, he would have to constantly pay attention to your updated versions and make sure not to mess up. I'm trying to find a solution where people can plug into the default renderer and tweak it. Adding a class or classes would be probably the most popular one and you already have the Attributes built-in, which as far as I understand are only used for headings. What if we could use the same thing for all the rest of the tags?

[yuin]

I'm not planning to add class(attributes) rendering for other elmenets for now. Currently, html.Renderer.RenderAttributes assumes all attributes are used for html attributes and are strings.

But, for example, goldmark-highlighting uses attributes like hl_lines=["2-3",5] for code highliting options. This is aparently not an html attribute and not strings. So we should consider attribute rendering more carefully. Some white list or(and) black list are probably needed for attribute rendering.

[zzwx]

@yuin Please have a look at this test solution:

056f86e

This allows for this kind of usage - let's say we need to append a class and style:

func (c CustomGoldmarkRenderer) Render(w io.Writer, source []byte, n ast.Node) error {
	ast.Walk(n, func(n ast.Node, entering bool) (status ast.WalkStatus, err error) {
		switch v := n.(type) {
		case *ast.Document:
		case *ast.Heading:
		case *ast.Text:
		case *ast.Emphasis:
		case *ast.AutoLink:
		case *ast.Image:
			if entering {
				n.SetModifyHTMLAttribute(func(source []byte, node ast.Node, htmlAttributeName string, initialValue []byte) []byte {
					switch htmlAttributeName {
					case "class":
						return append(initialValue, []byte(" img-fluid shadow-sm")...)
					case "style":
						return append(initialValue, []byte(" margin: 2px")...)
					}
					return nil
				})
			}

It's not very elegant, but it passes the tests and the list of allowed attributes could be tweaked. May be you could come up with something like this?

[yuin]

@zzwx Thanks for suggestion. I can see some problems.

• We want to avoid type switching if we can because type switch has performance penalties
• ast.Node should be independent from specific output formats(i.e. we may can implement something like a LaTex renderer). So ast.Node should not have method like SetModifyHTMLAttribute .

You already can append attributes like class and style using parser.ASTTransformer and node.*Attribute methods.
Problem occurs when rendering attributes because now we do not have any clue about an attribute whether it should be rendered as an HTML attribute or not.

[zzwx]

@yuin

Please see if I understand correctly what you meant with your explanation. This is without patching the library:

type classTransformer struct {
}

func (c *classTransformer) Transform(node *ast.Document, reader text.Reader, pc parser.Context) {
	ast.Walk(node, func(n ast.Node, entering bool) (status ast.WalkStatus, err error) {
		if image, ok := n.(*ast.Image); ok {
			if entering {
				image.SetAttributeString("class", []byte("img-fluid shadow-sm"))
			}
		}
		return ast.WalkContinue, nil
	})
}

func NewClassTransformer() parser.ASTTransformer {
	return &classTransformer{}
}

type classTransformerRenderer struct {
	html.Config
}

func (r classTransformerRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) {
	reg.Register(ast.KindImage, r.renderClass)
}

func RenderAttributes(w util.BufWriter, node ast.Node) {
	for _, attr := range node.Attributes() {
		_, _ = w.WriteString(" ")
		_, _ = w.Write(attr.Name)
		_, _ = w.WriteString(`="`)
		_, _ = w.Write(util.EscapeHTML(attr.Value.([]byte)))
		_ = w.WriteByte('"')
	}
}

func (r classTransformerRenderer) renderClass(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
	if !entering {
		return ast.WalkContinue, nil
	}
	n := node.(*ast.Image)
	_, _ = w.WriteString("<img src=\"")
	if r.Unsafe || !html.IsDangerousURL(n.Destination) {
		_, _ = w.Write(util.EscapeHTML(util.URLEscape(n.Destination, true)))
	}
	_, _ = w.WriteString(`" alt="`)
	_, _ = w.Write(n.Text(source))
	_ = w.WriteByte('"')
	if n.Title != nil {
		_, _ = w.WriteString(` title="`)
		r.Writer.Write(w, n.Title)
		_ = w.WriteByte('"')
	}
	if n.Attributes() != nil {
		RenderAttributes(w, node)
	}
	if r.XHTML {
		_, _ = w.WriteString(" />")
	} else {
		_, _ = w.WriteString(">")
	}
	return ast.WalkSkipChildren, nil
}

func NewClassTransformerRenderer(opts ...html.Option) renderer.NodeRenderer {
	r := &classTransformerRenderer{
		Config: html.NewConfig(),
	}
	for _, opt := range opts {
		opt.SetHTMLOption(&r.Config)
	}
	return r
}

Now I basically copy-paste default implementation of rendering, and that's exactly what bothers me, since the only thing I add is this:

	if n.Attributes() != nil {
		RenderAttributes(w, node)
	}

(with the copy of your function, since there's no access to it either from outside).

I'm hoping you could see why this is a bit problematic to reinvent the wheel. Or most likely I'm just not seeing complete picture.

Imagine I want to do the same for Paragraph, or for other kind of elements? Will I end up copy-pasting the whole html.go?

Please advise!

[yuin]

@zzwx Your understanding is correct. There should be something good solutions, but now we can do is just like a your code. It seems that Hugo plans to use attributes as non-HTML attribute value, so if we simply add RenderAttributes to node rendering functions probably causes problems.

I maybe design APIs that is good and less performance degradation.

[zzwx]

@yuin Awesome. I think it would be great to provide some overriding capability for the purpose of overwriting or appending to what's been already added by the library itself (for instance, language-.... classes from renderCodeBlock). So the user has to receive current value and decide what to return. In theory still.

Just brainstorming: Do you think attributes could have a concept of types? Attribute of type "html" would be for the rendering purpose? It will break API as a downside. Or a default type could be what's been in use until now.

[yuin]

@zzwx It is not good. Writers can add attributes in Markdown documents like # header # {.class=header} . In this case writers can not add types for attributes.

[yuin]

@zzwx Now node renderers render attributes. Renderers use pre-defined white list to determine whether given attribute should be rendered as an HTML attribute or not.

[zzwx]

Great news @yuin! I'm gonna test it all and I already see it will work! Thanks for this amazing work you've done!