Setting ImplicitParagraph has no effect #339

New Issue

claunia · 2026-01-29T14:34:14Z

claunia commented

2026-01-29 14:34:14 +00:00

Originally created by @sklivvz on GitHub (Jan 5, 2020).

As far as I understand, setting ImplicitParagraph to false should prevent markdig from wrapping rendered HTML in a <p>.

However, the following test does not behave as expected

            var pipeline = new MarkdownPipelineBuilder().Build();
            var writer = new StringWriter();
            var renderer = new Markdig.Renderers.HtmlRenderer(writer);
            renderer.ImplicitParagraph = false;
            pipeline.Setup(renderer);
            var foo = Markdown.ToHtml("test", pipeline);

            // foo is "<p>test</p>\n" and not "test"

Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable -- the user should not be forced to go through 5 lines of setup to set a basic option, such as when rendering a snippet. This also makes them basically undiscoverable.

Originally created by @sklivvz on GitHub (Jan 5, 2020). As far as I understand, setting `ImplicitParagraph` to `false` should prevent markdig from wrapping rendered HTML in a `<p>`. However, the following test does not behave as expected ```csharp var pipeline = new MarkdownPipelineBuilder().Build(); var writer = new StringWriter(); var renderer = new Markdig.Renderers.HtmlRenderer(writer); renderer.ImplicitParagraph = false; pipeline.Setup(renderer); var foo = Markdown.ToHtml("test", pipeline); // foo is "<p>test</p>\n" and not "test" ``` Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable -- the user should not be forced to go through 5 lines of setup to set a basic option, such as when rendering a snippet. This also makes them basically undiscoverable.

claunia added the enhancement PR Welcome! labels 2026-01-29 14:34:14 +00:00

claunia commented

2026-01-29 14:34:15 +00:00

@xoofx commented on GitHub (Jan 5, 2020):

Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable

I don't remember the details of this particular parameter, but these parameters on the Renderer class are often render states used by the renderers and not always settable by end-user. So their discover-ability was absolutely not a concern, unless you want to develop a new custom renderer.

Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable -- the user should not be forced to go through 5 lines of setup to set a basic option, such as when rendering a snippet. This also makes them basically undiscoverable.

Well, the situation could have been worse, I could have decided back in the days to make them completely internal. But at least, the API is giving you a chance to have an access for it, so that's not that bad right?

I'm no longer in the details of the API to propose anything right now, nor I have dedicated personal time to track this, so if you could take the time to make a proposal, and a PR, that would be helpful.

@xoofx commented on GitHub (Jan 5, 2020): > Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable I don't remember the details of this particular parameter, but these parameters on the Renderer class are often render states used by the renderers and not always settable by end-user. So their discover-ability was absolutely not a concern, unless you want to develop a new custom renderer. > Furthermore, this parameter (and the couple more similar) are only interesting at render time and as such, they should be directly settable -- the user should not be forced to go through 5 lines of setup to set a basic option, such as when rendering a snippet. This also makes them basically undiscoverable. Well, the situation could have been worse, I could have decided back in the days to make them completely internal. But at least, the API is giving you a chance to have an access for it, so that's not that bad right? I'm no longer in the details of the API to propose anything right now, nor I have dedicated personal time to track this, so if you could take the time to make a proposal, and a PR, that would be helpful.

claunia commented

2026-01-29 14:34:16 +00:00

@MihaZupan commented on GitHub (Jan 5, 2020):

ImplicitParagraph is false by default, if you don't want paragraph tags, set it to true.

Also Markdown.ToHtml creates its own renderer. If you create your own HtmlRenderer you have to render on it explicitly like so:

var pipeline = new MarkdownPipelineBuilder().Build();

var document = Markdown.Parse("test", pipeline);

StringWriter writer = new StringWriter();
var renderer = new HtmlRenderer(writer);
renderer.ImplicitParagraph = true; // note this change
pipeline.Setup(renderer);

renderer.Render(document); // using the renderer directly
writer.Flush();
string html = writer.ToString();
// test

the user should not be forced to go through 5 lines of setup to set a basic option

I agree it's not user-friendly rn, you are pretty much copying the implementation of ToHtml. I was thinking of adding another helper method, something like:

public class Markdown
{
    public static string ToHtml(string markdown, MarkdownPipeline pipeline, Action<HtmlRenderer> setupAction)
}

So this example would become

var pipeline = new MarkdownPipelineBuilder().Build();
string html = Markdown.ToHtml("test", pipeline, renderer => renderer.ImplicitParagraph = true);

@MihaZupan commented on GitHub (Jan 5, 2020): `ImplicitParagraph` is false by default, if you don't want paragraph tags, set it to true. Also `Markdown.ToHtml` creates its own renderer. If you create your own `HtmlRenderer` you have to render on it explicitly like so: ```c# var pipeline = new MarkdownPipelineBuilder().Build(); var document = Markdown.Parse("test", pipeline); StringWriter writer = new StringWriter(); var renderer = new HtmlRenderer(writer); renderer.ImplicitParagraph = true; // note this change pipeline.Setup(renderer); renderer.Render(document); // using the renderer directly writer.Flush(); string html = writer.ToString(); // test ``` > the user should not be forced to go through 5 lines of setup to set a basic option I agree it's not user-friendly rn, you are pretty much copying the implementation of `ToHtml`. I was thinking of adding another helper method, something like: ```c# public class Markdown { public static string ToHtml(string markdown, MarkdownPipeline pipeline, Action<HtmlRenderer> setupAction) } ``` So this example would become ```c# var pipeline = new MarkdownPipelineBuilder().Build(); string html = Markdown.ToHtml("test", pipeline, renderer => renderer.ImplicitParagraph = true); ```

claunia commented

2026-01-29 14:34:16 +00:00

@sklivvz commented on GitHub (Jan 5, 2020):

OK, so first of all thanks for clarifying the problem and its solution. I agree that a helper method would improve the situation, perhaps with some extra docs.

This worked for me, although it needs to be moved to the right class, tested etc.

    public static class Mark {
        public static string ToHtml(string markdown, MarkdownPipeline pipeline, Action<HtmlRenderer> setupAction)
        {
            var writer = new StringWriter();
            var renderer = new Markdig.Renderers.HtmlRenderer(writer);
            setupAction(renderer);
            renderer.Render(Markdown.Parse(markdown));
            return writer.ToString();
        }
    }

If you like the approach I'll push a PR tomorrow

Thanks,

@sklivvz commented on GitHub (Jan 5, 2020): OK, so first of all thanks for clarifying the problem and its solution. I agree that a helper method would improve the situation, perhaps with some extra docs. This worked for me, although it needs to be moved to the right class, tested etc. ```csharp public static class Mark { public static string ToHtml(string markdown, MarkdownPipeline pipeline, Action<HtmlRenderer> setupAction) { var writer = new StringWriter(); var renderer = new Markdig.Renderers.HtmlRenderer(writer); setupAction(renderer); renderer.Render(Markdown.Parse(markdown)); return writer.ToString(); } } ``` If you like the approach I'll push a PR tomorrow Thanks,

claunia commented

2026-01-29 14:34:16 +00:00

@MihaZupan commented on GitHub (Jan 5, 2020):

If @xoofx agrees with the API shape, the implementation would probably be like so:
(note that I changed the parameters)

Might be cleaner to reverse configureRenderer and pipeline and make pipeline non-default as I would imagine most callers will specify it and having an Action as a last parameter is usually more readable.

public static string ToHtml(string markdown, Action<HtmlRenderer> configureRenderer, MarkdownPipeline pipeline = null, MarkdownParserContext context = null)
{
    if (markdown is null) throw new ArgumentNullException(nameof(markdown));
    if (configureRenderer is null) throw new ArgumentNullException(nameof(configureRenderer));

    pipeline ??= new MarkdownPipelineBuilder().Build();
    pipeline = CheckForSelfPipeline(pipeline, markdown);

    var writer = new StringWriter();
    var renderer = new HtmlRenderer(writer);
    pipeline.Setup(renderer);

    configureRenderer(renderer);

    var document = Parse(markdown, pipeline, context);
    renderer.Render(document);
    writer.Flush();

    return writer.ToString();
}

@MihaZupan commented on GitHub (Jan 5, 2020): If @xoofx agrees with the API shape, the implementation would probably be like so: (note that I changed the parameters) Might be cleaner to reverse `configureRenderer` and `pipeline` and make pipeline non-default as I would imagine most callers will specify it and having an `Action` as a last parameter is usually more readable. ```c# public static string ToHtml(string markdown, Action<HtmlRenderer> configureRenderer, MarkdownPipeline pipeline = null, MarkdownParserContext context = null) { if (markdown is null) throw new ArgumentNullException(nameof(markdown)); if (configureRenderer is null) throw new ArgumentNullException(nameof(configureRenderer)); pipeline ??= new MarkdownPipelineBuilder().Build(); pipeline = CheckForSelfPipeline(pipeline, markdown); var writer = new StringWriter(); var renderer = new HtmlRenderer(writer); pipeline.Setup(renderer); configureRenderer(renderer); var document = Parse(markdown, pipeline, context); renderer.Render(document); writer.Flush(); return writer.ToString(); } ```

claunia commented

2026-01-29 14:34:17 +00:00

@audigex commented on GitHub (Oct 14, 2020):

I feel like this is all massively overcomplicated for the end user, for something that seems so basic and fundamental. Clearly there are a lot of situations where the user is going to want to render markdown inline.

It seems like this is all just a very roundabout way of handling something that should be done with the existence of ToInlineHtml() or ToHtml(bool inline = false)

@audigex commented on GitHub (Oct 14, 2020): I feel like this is all massively overcomplicated for the end user, for something that seems so basic and fundamental. Clearly there are a lot of situations where the user is going to want to render markdown inline. It seems like this is all just a very roundabout way of handling something that should be done with the existence of `ToInlineHtml()` or `ToHtml(bool inline = false)`

claunia commented

2026-01-29 14:34:17 +00:00

@MihaZupan commented on GitHub (Oct 14, 2020):

I don't know if this specific case is so common that a dedicated overload would be necessary.

Most users requiring fine-grained control will likely want to adjust more than one thing and will already be doing the pipeline setup and rendering steps manually.

@MihaZupan commented on GitHub (Oct 14, 2020): I don't know if this specific case is so common that a dedicated overload would be necessary. Most users requiring fine-grained control will likely want to adjust more than one thing and will already be doing the pipeline setup and rendering steps manually.

claunia commented

2026-01-29 14:34:18 +00:00

@kinetiq commented on GitHub (Sep 19, 2023):

@MihaZupan For what it's worth, I need it and I'm frustrated. I imagine this must be very common (especially considering the upvotes on the comment asking for an overload).

I've been poking around trying to find a solution that isn't an overblown hack but I'm giving up - I'm going to go do it the ugly way. :)

@kinetiq commented on GitHub (Sep 19, 2023): @MihaZupan For what it's worth, I need it and I'm frustrated. I imagine this must be very common (especially considering the upvotes on the comment asking for an overload). I've been poking around trying to find a solution that isn't an overblown hack but I'm giving up - I'm going to go do it the ugly way. :)

claunia commented

2026-01-29 14:34:18 +00:00

@xoofx commented on GitHub (Sep 19, 2023):

@MihaZupan For what it's worth, I need it and I'm frustrated. I imagine this must be very common (especially considering the upvotes on the comment asking for an overload).

Let me reiterate from my original response, that if someone wants such a feature, this person will have to make a PR for it with tests. 🙂

@xoofx commented on GitHub (Sep 19, 2023): > @MihaZupan For what it's worth, I need it and I'm frustrated. I imagine this must be very common (especially considering the upvotes on the comment asking for an overload). Let me reiterate from my original response, that if someone wants such a feature, this person will have to make a PR for it with tests. 🙂

claunia commented

2026-01-29 14:34:18 +00:00

@vsvsav commented on GitHub (Oct 7, 2024):

I don't know if this specific case is so common that a dedicated overload would be necessary.

Most users requiring fine-grained control will likely want to adjust more than one thing and will already be doing the pipeline setup and rendering steps manually.

Nope. I need it too. Additional wrapping the final text is not always a good idea, because if it is text inside a label+checkbox, the text will start on a new line.

@vsvsav commented on GitHub (Oct 7, 2024): > I don't know if this specific case is so common that a dedicated overload would be necessary. > > Most users requiring fine-grained control will likely want to adjust more than one thing and will already be doing the pipeline setup and rendering steps manually. Nope. I need it too. Additional wrapping the final text is not always a good idea, because if it is text inside a label+checkbox, the text will start on a new line.

claunia commented

2026-01-29 14:34:19 +00:00

@IIARROWS commented on GitHub (Mar 2, 2025):

Same here... I have an already existing structure, I have to put markdown text inside a span (just for emphasis like bold and italics), which of course cannot contain a paragraph.

Right now I'm calling Markdig.Markdown.ToHtml passing a string and a custom pipeline.

I think the proposal from sklivvz works nice. Hiding boilerplate from the caller is really helpful.

As far as I care, it should just be part of the pipeline. Add something like "UseNoWrapper" or whatever, seems really logical to me, it just have to prevent placing the paragraph tags at the beginning and end segment.
If the string then has other tags like titles, it's a user problem.

@IIARROWS commented on GitHub (Mar 2, 2025): Same here... I have an already existing structure, I have to put markdown text inside a span (just for emphasis like bold and italics), which of course cannot contain a paragraph. Right now I'm calling `Markdig.Markdown.ToHtml` passing a string and a custom pipeline. I think the proposal from [sklivvz](https://github.com/xoofx/markdig/issues/385#issuecomment-570953239) works nice. Hiding boilerplate from the caller is really helpful. As far as I care, it should just be part of the pipeline. Add something like "UseNoWrapper" or whatever, seems really logical to me, it just have to prevent placing the paragraph tags at the beginning and end segment. If the string then has other tags like titles, it's a user problem.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/markdig#339