Skip to content

ADD: Context to Renderer#578

Closed
grishat wants to merge 1 commit intoxoofx:masterfrom
grishat:add-context-to-renderer
Closed

ADD: Context to Renderer#578
grishat wants to merge 1 commit intoxoofx:masterfrom
grishat:add-context-to-renderer

Conversation

@grishat
Copy link
Contributor

@grishat grishat commented Dec 3, 2021

Added MarkdownParserContext to Renderer.

I wrote my own LinkExtension render TryLinkInlineRenderer, and need context access from HtmlRenderer, that available only in parser.

    public void Setup(MarkdownPipeline pipeline, IMarkdownRenderer renderer)
    {
        if (renderer is HtmlRenderer htmlRenderer)
        {
            var inlineRenderer = htmlRenderer.ObjectRenderers.FindExact<LinkInlineRenderer>();
            if (inlineRenderer != null)
            {
                inlineRenderer.TryWriters.Remove(TryLinkInlineRenderer);
                inlineRenderer.TryWriters.Add(TryLinkInlineRenderer);
            }
        }
    }

    private bool TryLinkInlineRenderer(HtmlRenderer renderer, LinkInline link)
    {
        ...
       var propValue = (string?)renderer.Context?.Properties["PropName"];
        ...
    }

I think the context should be available both in the parser and in the render. This required minimal changes.
What do you think?

Copy link
Collaborator

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me to extend the context to rendering

pipeline ??= DefaultPipeline;

using var rentedRenderer = pipeline.RentHtmlRenderer();
using var rentedRenderer = pipeline.RentHtmlRenderer(null, context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
using var rentedRenderer = pipeline.RentHtmlRenderer(null, context);
using var rentedRenderer = pipeline.RentHtmlRenderer(writer: null, context);

private readonly MarkdownParserContext? _context;

public HtmlRendererCache(MarkdownPipeline pipeline, bool customWriter = false)
public HtmlRendererCache(MarkdownPipeline pipeline, bool customWriter = false, MarkdownParserContext? context = null)
Copy link
Collaborator

@MihaZupan MihaZupan Dec 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MarkdownParserContext may be different for each parsing operation and shouldn't be cached between uses of the renderer.

Instead of storing the context on HtmlRendererCache, set it after renting it like we do with the writer and then null it out in Reset.

@grishat
Copy link
Contributor Author

grishat commented Dec 4, 2021

I get you, it turned out to be a little more complicated than I thought.
I found walkaround for me. I think I need to close this PR. I can't run the tests either on my local machine.

@grishat grishat closed this Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants