Windowscommunitytoolkit: [Discussion] Do we use a .NET Markdown Parser Library instead of our own?

[michael-hawker]

Describe the problem this feature would solve

We have a lot of overhead for the MarkdownTextBlock supporting Markdown via our own Markdown parser. It may make more sense to refactor on top of a .NET Standard library that's maintained by a larger community, like Markdig.

This however, would require updating our MarkdownRenderer code to do all the construction from the AST.

Describe the solution

Take a dependency on a markdown parser instead of maintaining our own.

Additional context

We were thinking as part of #3062 to separate out the MarkdownTextBlock anyway, as it has a lot of dependencies, so this could also contribute to that effort as well to remove the need for the dependency on Microsoft.Toolkit.Parsers.

If we do this swap, we should convert all of our existing Unit Tests on the Parser to instead test the Renderer and check the translation between the Markdown text scenario and the UI Element tree construction.

This may also just mean we can get rid of the Microsoft.Toolkit.Parsers package entirely if we find another open source C# based RSS parser as we don't use that elsewhere in the toolkit currently.

Requirements

• [ ] We should leverage/convert our existing unit tests for render tests to make sure we make the correct UI component abstracts (vs current AST checks)
• [ ] We should work with Markdig or expose a helper for the original Span/String of the Markdown text of a component see Issue #2729
• [ ] TODO Grab other Markdown labelled issues to list here.

[msftbot[bot]]

Thanks for submitting a new feature request! I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

[michael-hawker]

@Kyaa-dost any ideas why the bot added these tags to this issue?

[Kyaa-dost]

@michael-hawker I was doing testing with the bot so this have occurred due to that. I have scanned through all the features in the bot and none should be causing this.

[MihaZupan]

+1 for Markdig. Feel free to ping me about it

[michael-hawker]

Looking at Unit Tests, appears the inline GitHub style e-mail parsing is not functioning fully (the original UnitTest had things backwards (as does GitHub)). See e-mail RFC spec section here.

Some e-mails not supported properly still by our parser include:

• a^[email protected]
• a.[email protected]
• "a..b"@test.com

[LokiMidgard]

I would like to see that the used parser could be exchangeable. E.g. hidden behind some interfaces. Including a default Implementation that is CommonMark compatible.

There are a bunch of different Markdown flavors out there already in use. If I start from scratch, I can use a flavor that is supported by this library. But if I consume the markdown from another source I don't control and it does not works with the implementation we choose I'm screwed.

Since this library is probably the way to go when rendering markdown in WinUi, it should be a little flexible.

[LokiMidgard]

If we do this swap, we should convert all of our existing Unit Tests on the Parser to instead test the Renderer and check the translation between the Markdown text scenario and the UI Element tree construction.

If you convert the existing Unit Tests, please consider DataRowAttribute for thinks like different Mail addresses. It was really painfull to find the mail addresses that were not working in the test. And every mail address after the failing one was not testst.

[michael-hawker]

@LokiMidgard as a quick fix for the existing unit tests I did in #3207 to better understand these problems, I did use the DataRowAttribute.

I know it's going to be a lot of work already to flip over to another implementation for the rendering, there is some tight coupling there, so I'm not sure how feasible it'd be to abstract again to support different parsers. Markdig does seem rather flexible with extensions, so I certainly think we should still have call-outs for customization, but I don't think a whole abstraction for parsing below the rending would be warranted?

@MihaZupan what has been your experience with the Markdown community on these subjects of the different Markdown flavors?

[michael-hawker]

I also want to bring up the issue of #268 again about HTML content (thinking snippets or other things), I just encountered this need again with Teams messages from the Microsoft Graph with our GraphPresenter control.

As part of this parser swap, could it make sense to have the renderer be more generalized to support a different parser for display or would these just be two parallel efforts?

[michael-hawker]

Actually just found this Xamarin solution for HTMLLabel, will test that out for my scenario. :)