MarkdownRender
MarkdownRender copied to clipboard
Microsoft.PowerShell.MarkdownRender.MarkdownInfo.ToString() should be the HTML
Summary of the new feature / enhancement
As a user, I want to be able to more seamlessly work with Markdown in PowerShell.
Unfortunately, when I try to ToString() a Markdown object, I get:
Microsoft.PowerShell.MarkdownRender.MarkdownInfo
This makes easily embedding markdown inside of a string more painful than it needs to be, and also complicates logic for -joining multiple markdowns
Proposed technical implementation details (optional)
The .ToString()
method the MarkdownInfo class should be the converted HTML.
While I think it's a good idea to have .ToString()
return a meaningful representation, note that the logic of the Microsoft.PowerShell.MarkdownRender.MarkdownInfo
-returning ConvertFrom-Markdown
cmdlet is to either fill in the .Html
(by default) or the .VT100EncodedString
property (if requested via -AsVT100EncodedString
).
Thus, it's probably better to do something like [operand order updated after feedback]:
public override string ToString() {
return this.Html ?? this.VT100EncodedString ?? string.Empty;
}
@mklement0 I'm definitely open to the idea of a flag, as I think both are useful.
However / and, I think that should be the job of a local formatter (which should probably call Show-Markdown)
I think HTML construction is the more common / intended use case and should be the default.
I also think ToString() could have multiple overloads ;-)
I think HTML construction is the more common / intended use case and should be the default.
It is the default, but the problem is that .Html
isn't guaranteed to have a value, and if it doesn't (if -AsVT100EncodedString
was used), with your approach would it would string-interpolate to the empty string - and I don't think that's desirable.
Defaulting to whichever property is filled in to me seems like the preferable behavior.
As for multiple overloads: That would be useful in the abstract - say .ToString(MarkdownConversionType.Html)
, .ToString(MarkdownConversionType.VT100)
, and (not currently an option) .ToString(MarkdownConversionType.PlainText)
- but at least currently makes no sense, as long as only ever one property is filled in.
From what I can tell, while you can currently publicly instantiate a MarkdownInfo
instance, you cannot fill it yourself, and therefore have no way of requesting that both .Html
and .VT100EncodedString
be filled - possibly on demand.
@mklement0 I get what you're saying here, and I think the best approach would be to make the ToString()
conditional depending on which had content.
Defaulting to whichever property is filled in to me seems like the preferable behavior.
PR updated. Thanks for the feedback!
Glad to hear it, @StartAutomating - in my earlier comment I had the operand order backwards (though it would have worked as along as the either-or logic of the properties being filled in is in place), so I've updated it to avoid confusion.