@mklement0 @iSazonov

Had some time to look at this again and I see a main challenge.

If we want to combine all input from MarkdownInfo and string into one string, it tricky since MarkdownInfo only exposes Html or VT100EncodedString, and if you call MarkdownRenderer.Convert() on a VT100EncodedString, it will give strange results:

> $mdText = "**Bold**"
> $mdVT100String = $mdText | ConvertFrom-Markdown -AsVT100EncodedString
> $mdVT100String | Format-List

Html               :
VT100EncodedString : Bold # This is bold in terminal, as expected

Tokens             : {Markdig.Syntax.ParagraphBlock}
> [Microsoft.PowerShell.MarkdownRender.MarkdownConverter]::Convert($mdVT100String.VT100EncodedString, [Microsoft.PowerShell.MarkdownRender.MarkdownConversionType]::VT100, [Microsoft.PowerShell.MarkdownRender.PSMarkdownOptionInfo]::new()) | Format-List

Html               :
VT100EncodedString : mBoldm # This has strange characters, almost looking like it was unescaped or something


Tokens             : {Markdig.Syntax.ParagraphBlock}

Which shows some strange VT100EncodedString which seems to be changed. Almost seems that it tries to re-encode the string again, which I guess makes sense.

This above makes it challenging since after combining all strings, you will need to run MarkdownRenderer.Convert() on the single string and not be able to differentiate.

One option I was looking into was seeing if you can get the original markdown from AST with Tokens, but that proved to be quite difficult to parse.

Another option was perhaps running the conversion line by line for string inputs and ignoring MarkdownInfo and adding them as is, but that will get more difficult because Convert seems to add quite of bit of extra new lines so they will need to be normalized. This is probably quite challenging to get right though, could introduce more regression.

With above findings I can see why this was never done in the first place. Perhaps existing approach of just handling strings is not a bad idea, I do consider mixing types in input to be an edge case.

@ArmaanMcleod, I'm a bit confused: [Microsoft.PowerShell.MarkdownRender.MarkdownConverter]::Convert() expects Markdown as input - i.e. plain text - so it's not surprising that it doesn't handle a string with VT escape sequences meaningfully, and I don't think it should be expected to.

That said, that it effectively breaks such escape sequences instead of passing them through is definitely problematic (seemingly, it removes the [ characters, so that "`e[1mBold`e[0m" turns into "`e1mBold`e0m"), but that strikes me as a separate problem.

I agree with @mklement0 I can't think of a realistic scenario for such a transformation.

Thanks @mklement0 & @iSazonov

In this case I don't think it makes sense to try and combine MarkdownInfo strings with string and pass everything to MarkdownRenderer.Convert(), so I've made changes to just collect all input of type string. I don't think you can do both unless there was a way to get original markdown string from MarkdownInfo. Otherwise as shown above we will be passing already VT100 escaped sequences into MarkdownRenderer.Convert() and get strange results.

This PR has 13 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Label      : Extra Small
Size       : +13 -0
Percentile : 5.2%

Total files changed: 2

Change summary by file extension:
.cs : +6 -0
.ps1 : +7 -0

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author