xoofx / markdig

A fast, powerful, CommonMark compliant, extensible Markdown processor for .NET
BSD 2-Clause "Simplified" License
4.41k stars 456 forks source link

Add ConfigureHtmlRenderer to MarkdownPipelineBuilder #802

Open yngvedh opened 5 months ago

yngvedh commented 5 months ago

I needed to set up image url rewriting in a project of mine and found it a bit tedious not having a way to set it up in the pipeline. So I added the ability to configure the HtmlRenderer from the pipeline builder. As an added bonus, this works with the renderer cache as well.

I did not add any tests but updated the BaseUrl test to configure the html renderer using the pipeline builder. I did not update changelog.md since it already has been neglected for some time. I have not spent any time figuring out whether any of the documentation should be updated, please advise. I could not find any existing PR or issue on this exact issue so I took the liberty to raise this draft PR.

UPDATE: I added a bare bones builder for HtmlRenderer letting me set the one field I need as a POC. It is basically a factory for TextRendererBase which I also refactored the cache mechanism to use so that too should work for any text renderer. This goes for the ToHtml method since it uses the caching mechanism to get hold of its renderer.

xoofx commented 5 months ago

Thanks. It looks ok at a quick glance.

yngvedh commented 5 months ago

Great, I'll see if I can find the time to flesh it out with more properties and renderers.

yngvedh commented 5 months ago

So I added builders for all kinds of renderers along with all fields. I also added doc comments and updated some tests.

Now, I'm not 100% sure all those fields should be available through the builder. In particular the EnableHtmlFor* options because they seem to only be used internally by the different renderers/extensions. Please let me know whether I should drop some of them.

I'm also not 100% sure all those renderers should be available through the builder at either so let me know if any of them shouldn't and I'll have them removed.

MihaZupan commented 5 months ago

A couple observations:

xoofx commented 5 months ago

Indeed, seeing more changes coming, I agree the points from @MihaZupan