fix: coalesce duplicate class attributes in CodeBlock and Heading#356
fix: coalesce duplicate class attributes in CodeBlock and Heading#356kjk merged 2 commits intogomarkdown:masterfrom
Conversation
When a CodeBlock or Heading node has both a language/special class and a custom class attribute set via the AST, the renderer produced two separate class="..." attributes in the HTML output. Browsers only use the first one, so the custom class was silently dropped. Add coalesceClassAttrs helper that merges multiple class attributes into a single one, and apply it in both CodeBlock and HeadingEnter. Fixes gomarkdown#209.
There was a problem hiding this comment.
Pull request overview
This PR fixes duplicate class="..." attributes emitted for CodeBlock and Heading nodes when both a renderer-generated class (e.g., language-* / special) and an AST-provided class are present, by coalescing them into a single class attribute (per #209).
Changes:
- Add
coalesceClassAttrshelper to merge multipleclass="..."attributes into one. - Apply the helper in
Renderer.HeadingEnterandRenderer.CodeBlock. - Add unit tests for the helper plus an integration test reproducing the reported issue; update expected mmark test output.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
html/renderer.go |
Coalesces duplicate class attributes for headings and code blocks via new helper. |
html/coalesce_test.go |
Unit tests for coalesceClassAttrs across common and edge cases. |
html_renderer_test.go |
Integration test ensuring language + custom class are merged into one attribute. |
testdata/mmark.test |
Updates expected HTML output to reflect coalesced heading classes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ast.WalkFunc(doc, func(node ast.Node, entering bool) ast.WalkStatus { | ||
| if cb, ok := node.(*ast.CodeBlock); ok { | ||
| cb.Attribute = &ast.Attribute{ | ||
| Classes: [][]byte{[]byte("my-class")}, | ||
| } | ||
| } | ||
| return ast.GoToNext |
There was a problem hiding this comment.
The AST walk in this test sets cb.Attribute on both the entering and exiting visits because it doesn't check the entering flag. Adding if !entering { return ast.GoToNext } (or equivalent) would avoid redundant mutation and make the intent clearer.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks! |
Fixes #209.
Problem: When a
CodeBlockorHeadingnode has both a language/special class and a custom class attribute set via the AST, the renderer produced two separateclass="..."attributes:Browsers only use the first
classattribute, so the custom class was silently dropped.Fix: Add
coalesceClassAttrshelper that merges multipleclassattributes into one, applied in bothCodeBlockandHeadingEnter. This resolves the TODO comments at both call sites.After:
Validation:
go test ./...passes (excluding pre-existing failures inmd/TestRenderCodeBlockandexamples/)go vet ./html/cleantestdata/mmark.testexpected output to reflect correct coalesced class