renderer: add support for extending node renderers #16

Merged
benclmnt merged 4 commits from f-extend-node-renderers into main 2024-02-08 05:56:29 +00:00
benclmnt commented 2022-12-26 14:00:33 +00:00 (Migrated from github.com)

Hi, I added support for extending node renderer similar to goldmark's HTML renderer.

An example usage:

markdownRenderer := markdown.NewRenderer()
markdownRenderer.AddOptions(
	renderer.WithNodeRenderers(util.Prioritized(&latex.BlockRenderer{
		RendererType: latex.RendererTypeMarkdown,
	}, 502)),
)
md := goldmark.New(
	goldmark.WithRenderer(markdownRenderer),
)	
Hi, I added support for extending node renderer similar to goldmark's HTML renderer. An example usage: ```go markdownRenderer := markdown.NewRenderer() markdownRenderer.AddOptions( renderer.WithNodeRenderers(util.Prioritized(&latex.BlockRenderer{ RendererType: latex.RendererTypeMarkdown, }, 502)), ) md := goldmark.New( goldmark.WithRenderer(markdownRenderer), ) ```
coveralls commented 2023-04-06 15:57:23 +00:00 (Migrated from github.com)

Pull Request Test Coverage Report for Build 7825492430

  • 0 of 102 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 7551523042: 0.0%
Covered Lines: 452
Relevant Lines: 452

💛 - Coveralls
## Pull Request Test Coverage Report for [Build 7825492430](https://coveralls.io/builds/65574627) * **0** of **102** **(100.0%)** changed or added relevant lines in **2** files are covered. * No unchanged relevant lines lost coverage. * Overall coverage remained the same at **100.0%** --- | Totals | [![Coverage Status](https://coveralls.io/builds/65574627/badge)](https://coveralls.io/builds/65574627) | | :-- | --: | | Change from base [Build 7551523042](https://coveralls.io/builds/65115124): | 0.0% | | Covered Lines: | 452 | | Relevant Lines: | 452 | --- ##### 💛 - [Coveralls](https://coveralls.io)
teekennedy (Migrated from github.com) reviewed 2023-04-06 15:58:32 +00:00
@ -126,10 +146,10 @@ func (r *Renderer) renderBlockSeparator(node ast.Node, entering bool) ast.WalkSt
func (r *Renderer) renderAutoLink(node ast.Node, entering bool) ast.WalkStatus {
teekennedy (Migrated from github.com) commented 2023-04-06 15:58:31 +00:00

I was starting to worry about the performance of setting up all these renderer function chains. Nice change!

I was starting to worry about the performance of setting up all these renderer function chains. Nice change!
teekennedy (Migrated from github.com) requested changes 2023-04-06 22:16:43 +00:00
teekennedy (Migrated from github.com) left a comment

Other than my one requested change this looks good!

Other than my one requested change this looks good!
@ -128,3 +148,2 @@
if entering {
r.rc.writer.Write([]byte("<"))
r.rc.writer.Write(n.URL(r.rc.source))
r.rc.writer.WriteBytes([]byte("<"))
teekennedy (Migrated from github.com) commented 2023-04-06 22:15:52 +00:00

The only thing I suggest changing is to add a length check here. Since you're keeping maxKind up to date in the Register method, I don't think an index out of range issue is likely to come up in practice. If it does happen, it'll result in a panic.

The existing version of the code would turn this into a no-op by calling the function returned by chainRenderers(...) with an empty slice. We can maintain that behavior here by returning ast.WalkContinue.

The only thing I suggest changing is to add a length check here. Since you're keeping `maxKind` up to date in the `Register` method, I don't think an index out of range issue is likely to come up in practice. If it _does_ happen, it'll result in a panic. The existing version of the code would turn this into a no-op by calling the function returned by `chainRenderers(...)` with an empty slice. We can maintain that behavior here by returning `ast.WalkContinue`.
teekennedy (Migrated from github.com) approved these changes 2024-02-08 05:56:18 +00:00
Sign in to join this conversation.
No description provided.