RFC for native markdown parsing on console #116

adityapatwardhan · 2018-03-06T02:09:49Z

No description provided.

iSazonov · 2018-03-06T14:05:56Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+## Specification
+
+This RFC proposes to use VT100 escape sequences to render markdown content.
+`ConvertFrom-Markdown` cmdlet as part of `Microsoft.PowerShell.MarkdownRender` PowerShell module would consume a string or a file path and output a VT100 encoded string.


Why we introduce new module? We expect still many new cmdlets wil be created?

I agree, this can be moved to Microsoft.PowerShell.Utility

iSazonov · 2018-03-06T14:11:15Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+This RFC proposes to use VT100 escape sequences to render markdown content.
+`ConvertFrom-Markdown` cmdlet as part of `Microsoft.PowerShell.MarkdownRender` PowerShell module would consume a string or a file path and output a VT100 encoded string.
+Optionally, the output can be formatted as HTML.
+For converting strings to VT100 coded strings, we will be writing an extension to [markdig](https://github.com/lunet-io/markdig).


markdig already implement HTML renderer - nobody ask them about plans to implement VT100 renderer?

markdig has a good extendable architecture and they prefer extensions. We may publish a nuget package of the extension we write. so it can be used by others.

iSazonov · 2018-03-06T14:13:03Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+For converting strings to VT100 coded strings, we will be writing an extension to [markdig](https://github.com/lunet-io/markdig).
+The extension will insert VT100 escape sequences as appropriate.
+
+```PowerShell


The code block should be commented.

@iSazonov Could you please elaborate on this?

Seems it is out of context - text above is not related to the code.

Got it thanks.

iSazonov · 2018-03-06T14:21:21Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+- **AsHTML** : When selected, the output will be HTML.
+- **AsPlainText** : When selected, the output will be plain text.
+
+The default output will be a string encoded with VT100 escape sequences.


This cmdlet is not intended solely for the console output. So I guess users can expect another default (more popular HTML).

I prefer suggestion from @DerpMcDerp, I will update the RFC to state that, ConvertFrom-Markdown returns an object, and the default formatter renders it as markdown.

ConvertFrom-Markdown could return a PSObject with three properties

Property Tokens that holds the tokens (ast?) parsed by markdig

Property Html that holds the converted html text if -AsHTML is specified

Property VT100EncodedString that holds the escape sequences.

The PSObject has a special PSTypeName that can be recognized by formatting system, and render it using the VT100 escape sequences when

VT100EncodedString is not null, and, host support VT100 sequences

Otherwise, it will be rendered in a list view with the available properties.

I agree that we could return an object. And replace AsPlainText with AsVT100Encoded.
If we use simplified markdown scheme why we need property Tokens? It seems useless.
Having two results (Html and VT100EncodedString) at once is too costly. I suggest use methods or one property Value.

replace AsPlainText with AsVT100Encoded

I agree.

why we need property Tokens

I think we will get the parsed tokens/Asts anyway during the conversion. @DerpMcDerp can you give an example where the parsed markdown ASTs may be useful?

Having two results (Html and VT100EncodedString) at once is too costly

We don't generate two results in one go. By default, only Html is populated; if -AsVT100Encoded is specified, then VT100EncodedString is populated. This is similar to MeasureInfo objects produced from Measure-Object.

iSazonov · 2018-03-06T14:26:34Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+
+## Supported Markdown Elements
+
+We will be supporting a limited set of markdown elements in the initial version.


For the completeness of the RFC I would expect that we will describe the escape sequences for all markdown elements to have an accurate parity with that the markdig supports for HTML. (We are free implement its step-by-step).

The RFC describes escape sequences for all the markdown elements that will be supported in this version. Further work on the extension might have another RFC, if the changes are significant enough.

I agree if changing already implemented escapes will not be a breaking change.

iSazonov · 2018-03-06T14:29:40Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+
+## Open Questions
+
+- Investigate line wrapping behavior.


We have related open question - pager.

Thanks, I will add that to open question.

DerpMcDerp · 2018-03-06T19:02:53Z

I think it would be better for markdown to:

output a stream of objects.

enum Kind {
	Plain,
	Plain_,
	_Header1,
	Header1,
	Header1_,
	_LinkAlt,
	LinkAlt,
	LinkAlt_,
	_Link,
	Link,
	Link_
// etc.
}

struct Region {
	int Begin;
	int End;
}

class MarkdownToken {
	string Parent;
	Kind Kind;
	Region Region;
	
}

So:


Hey
# Foo

Would output:

Plain: "Hey"
Plain_: "\n"
_Header1: "# "
Header1: "Foo"
Header1_: "\n"

Remove -AsHTML and -AsPlainText and use ConvertFrom-Markdown to convert a markdown stream to html.
The default console formatter cmdlet should render the above output as formatted text
The console formatter cmdlet should not strip away formatting symbols by default. e.g.

## Foo

would render as:

ESC[4;93m## Foo 1ESC[0m

rather than:

ESC[4;93mFoo 1ESC[0m

daxian-dbw · 2018-03-12T19:57:41Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+
+- **Path** : Accepts an array of file paths with markdown content.
+- **LiteralPath** : Accepts an array of literal paths with markdown content.
+- **InputObject** : Accepts an InputObject of `System.IO.FileSystemInfo`, `string` type.


Why FileSystemInfo object? It won't be able to process directory, so FileInfo should be good here.

Agreed, FileInfo should be sufficient.

daxian-dbw · 2018-03-12T20:06:43Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+
+#### Headers
+
+VT100 escape sequences for headers are as follows:


I think the escape sequences for different headers should be configurable, because users may have different background colors for their hosts, and some pre-defined color won't work well with that background.

I believe we don't need to give the granularity that every markdown header color can be configured. Instead, we can provide 2 or 3 themes that would work for dark, light backgrounds respectively.

daxian-dbw · 2018-03-12T20:11:51Z

1-Draft/RFCNNNN-Native-Markdown-Rendering.md

+#### Links
+
+Links in paragraphs will keep the link label and append the link URL surrounded by parenthesis.
+The link will be colored to differentiate from plain text.


I suggest to have the text in [] quoted with "", otherwise there won't be a way to know what is the exact text that refers to the link. For example: this is a long text that refers to a link should be rendered as "this is a long text that refers to a link"(https://www.bing.com)

I like this idea. I will update.

adityapatwardhan · 2018-03-14T01:51:23Z

@iSazonov @DerpMcDerp @daxian-dbw your comments have been addressed.

iSazonov · 2018-03-14T06:39:16Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+Set-MarkdownOption [-Header1Color <ConsoleColor>] [-Header2Color <ConsoleColor>] [-Header3Color <ConsoleColor>] [-Header4Color <ConsoleColor>] [-Header5Color <ConsoleColor>] [-Header6Color <ConsoleColor>] [-CodeBlockForegroundColor <ConsoleColor>] [-CodeBlockBackgroudColor <ConsoleColor>] [-ImageAltTextForegroundColor <ConsoleColor>] [-LinkForegroundColor <ConsoleColor>] [-ItalicsForegroundColor <ConsoleColor>] [<CommonParameters>]
+
+Set-MarkdownOption [-Theme <MarkdownThemeType>] [<CommonParameters>]


I think about custom themes - using ThemeName as string is more general.
If we store the theme type/name in powershell.config.json I believe the cmdlet is unnecessary - I'd prefer to have one universal cndlet to edit powershell.config.json (out of the RFC).
I believe Set-MarkdownOption [-Header1Color <ConsoleColor>] ... is unnecessaty too - how often should users change a theme? I suppose very seldom. If we store a theme in a simple file (json/yaml?) - why we don't address the file format/place in the RFC? - users can manually edit it easily.

If we store the theme type/name in powershell.config.json

Having the theme setting in powershell.config.json doesn't seem necessary. I think having Set-MarkdownOption -Theme <name> in your profile would work.

If we store a theme in a simple file (json/yaml?) - why we don't address the file format/place in the RFC?

Do you mean to allow a user to alter a theme and create their own theme?

I modelled it after Set-PSReadLineOption. And as @daxian-dbw said, all the customization can be added to the profile.

Do you mean to allow a user to alter a theme and create their own theme?

Yes.

all the customization can be added to the profile.

I understand. I still think that customizing themes is a rare scenario and creating special cmdlets is redundant. It is also easier to create and distribute custom themes in a file than in a profile.

@iSazonov I have updated the RFC to supported importing / exporting the settings to/from a file.

iSazonov · 2018-03-14T06:43:23Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

As currently we discussed (@SteveL-MSFT ) tabs is problematic - we should ignore a console settings and do rendering in PowerShell consolehost internally.

I think "line wrapping" here refers to semantic line breaks?

After some investigation, we do not have to special case for this. This is parsed automatically. I will remove this.

SteveL-MSFT · 2018-03-14T21:05:53Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+```
+
+The properties can be individually to customized the rendering on console.


Grammar: The properties can individually customize the rendering on the console.

SteveL-MSFT · 2018-03-14T21:08:44Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+Code blocks will be rendered with a lighter background color and a dark foreground text.
+Background color will be applied for the console width.
+No syntax highlighting will be done.
+`ESC[500@` is used to have sufficiently wide background color.


Instead of wide, you mean contrasting?

I actually mean wide, so that the background spans the console width and not just the code string width.

adityapatwardhan · 2018-03-19T18:46:37Z

@SteveL-MSFT Your feedback has been addressed.

SteveL-MSFT · 2018-03-19T21:37:40Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+2. Adds a new dependency of `markdig` to `System.Management.Automation`.
+
+## Open Questions
+


Semantic linebreaks should be called out explicitly if supported or not

I have added a line the Paragraphs and line breaks section.

adityapatwardhan · 2018-03-26T22:03:51Z

@SteveL-MSFT This RFC is ready for review by committee.

joeyaiello

Added a couple open questions

joeyaiello · 2018-03-28T22:18:33Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+Set-MarkdownOption -InputObject <psobject> [<CommonParameters>]
+
+```


PSReadline 2.0 is moving away from the ConsoleColor and towards VT100 color sequences. If VT100 is already required for this Markdown support to work, might we want to do the same?

joeyaiello · 2018-03-28T22:19:19Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+```
+
+Export the current markdown settings to a file.


What kind of file are we exporting/importing the options as? .psd1?

It is a JSON file, I will update to clarify.

joeyaiello · 2018-03-28T22:40:29Z

@PowerShell/powershell-committee reviewed this today. A couple notes:

it'd be nice to see a full image example with all the different token renderings so we can see them next to each other
there should be a mention of the help system scenario given that it's our primary motivation for this work
the right verb should be Show instead of ConvertTo

daxian-dbw · 2018-03-28T22:47:00Z

@joeyaiello I'm afraid ConvertFrom- is still needed as the purpose is to convert a markdown to an object and write that object to the pipeline. The object has the properties Tokens (hold the asts generated from parsing the markdown file), HTML (hold the html content representing the markdown) and VT100EncodedString (hold the VT100 sequence representing the markdown) properties.

We can consider whether or not Show-Markdown should be added in through.

iSazonov

Leave a comment

iSazonov · 2018-03-29T05:33:24Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+The `Tokens` property has the AST for the markdown document from `markdig`.
+The `Html` property has the markdown document converted to `HTML`.
+The `VT100EncodedString` property has the markdown documented with `VT100` escape sequences.
+By default, the `Tokens`and `Html` properties will be populated.


As I uderstand we'll have Tokens always.

Yes that is correct.

I means we should change the sentence:

By default, the `Html` property will be populated. The `Tokens` property is populated in all cases.

Understood. Updated.

iSazonov · 2018-03-29T05:43:45Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+## Alternate Proposals and Considerations
+
+Introduce a new sigil to define a markdown string.
+This would be later rendered using formating files.


Have we plan to add [MarkDown] type to automatically format output strings?

[MarkDown] $var = " ... " $var $var | Out-Default

Adding a type accelerator [Markdown] is not in scope for this RFC.

My comment is not about the accelerator - it is about the type so we can use ETS and auto-formatting.

Yes, the cmdlet returns a strongly typed object. I will add a line about it.

adityapatwardhan · 2018-04-02T18:50:43Z

I have updated the RFC to address the committee's feedback.

iSazonov · 2018-04-03T04:34:39Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+```
+
+Import the markdown settings from the specified file and returns a PSObject of the options.
+This can be used as an `InputObject` for `Set-MarkdownOption`.


Seems we could add Import-MarkdownOption -ApplyOptions vs Import-MarkdownOption | Set-MarkdownOption.

iSazonov · 2018-04-03T04:38:03Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+#### Output Type
+
+The output type will be `MarkdownInfo` object with properties for `Html`, `VT100EncodedString` and `Tokens`.


With having MarkdownInfo I'd expect follow works (using ETS):

<command returns MarkdownInfo> | Out-Default <command returns MarkdownInfo> | Out-Markdown ``

We will have a default formatter. Maybe that calls Show-Markdown.

iSazonov · 2018-04-03T04:39:41Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+```
+
+Render the `VT100EncodedString` property of `MarkdownInfo` on console.


Related my previous comment - What about Out-Default, Out-Markdown and ETS?

With a default formatter, Out-Default will work.

iSazonov · 2018-04-03T04:41:00Z

2-Draft-Accepted/RFCNNNN-Native-Markdown-Rendering.md

+
+```
+
+Export the current markdown settings to a JSON file.


Have we default JSON file? Where?

We do not have a default JSON file. Values provided to Set-MarkdownOption via options or Import-MarkdownOption will be applied.

iSazonov · 2018-04-03T05:14:12Z

Maybe useful https://github.com/MicrosoftDocs/office-docs-powershell

joeyaiello · 2018-04-04T23:04:24Z

The @PowerShell/powershell-committee is generally okay with the functionality here, but we'd like a few details spec'd/documented here:

consider -PassThru for Set-MarkdownOption
process-persistent vs. runspace-persistent Markdown options
output type of Get-MarkdownOption

In the meantime, we're moving this to experimental to allow @adityapatwardhan to continue prototyping this work.

felixfbecker · 2018-09-10T20:22:59Z

What is the use case for having a VT100EncodedString property? If its sole purpose is for pretty output formatting in terminal hosts, it should be done through a formatting file (Format.ps1xml), not through the data object. I.e. this is the way this object should always be displayed, while the raw markdown or HTML is accessible through a property or by piping into a Convert cmdlet.

adityapatwardhan · 2018-09-10T23:32:12Z

@felixfbecker Eventually the help system will be updated to use and display markdown. This will be used for that.

RFC for native markdown parsing on console

03196ba

iSazonov reviewed Mar 6, 2018

View reviewed changes

daxian-dbw reviewed Mar 12, 2018

View reviewed changes

Addressed comments

b9daafb

iSazonov reviewed Mar 14, 2018

View reviewed changes

SteveL-MSFT requested changes Mar 14, 2018

View reviewed changes

Fixed grammar and updated open issues

c73f7a2

SteveL-MSFT requested changes Mar 19, 2018

View reviewed changes

Added semantic line break statement

4cac246

SteveL-MSFT approved these changes Mar 20, 2018

View reviewed changes

Added cmdlets to export/import settings

60bc1ca

SteveL-MSFT added the Review - Committee label Mar 27, 2018

joeyaiello suggested changes Mar 28, 2018

View reviewed changes

iSazonov reviewed Mar 29, 2018

View reviewed changes

Address comments from committee

15e33f1

iSazonov reviewed Apr 3, 2018

View reviewed changes

Move Markdown rendering RFC to experimental

fad1e81

joeyaiello approved these changes Apr 4, 2018

View reviewed changes

joeyaiello merged commit b276b9a into PowerShell:master Apr 4, 2018

joeyaiello removed the Review - Committee label Mar 4, 2019


		## Supported Markdown Elements

		We will be supporting a limited set of markdown elements in the initial version.


		#### Headers

		VT100 escape sequences for headers are as follows:


		Set-MarkdownOption [-Header1Color <ConsoleColor>] [-Header2Color <ConsoleColor>] [-Header3Color <ConsoleColor>] [-Header4Color <ConsoleColor>] [-Header5Color <ConsoleColor>] [-Header6Color <ConsoleColor>] [-CodeBlockForegroundColor <ConsoleColor>] [-CodeBlockBackgroudColor <ConsoleColor>] [-ImageAltTextForegroundColor <ConsoleColor>] [-LinkForegroundColor <ConsoleColor>] [-ItalicsForegroundColor <ConsoleColor>] [<CommonParameters>]

		Set-MarkdownOption [-Theme <MarkdownThemeType>] [<CommonParameters>]


		```

		The properties can be individually to customized the rendering on console.

		2. Adds a new dependency of `markdig` to `System.Management.Automation`.

		## Open Questions


		Set-MarkdownOption -InputObject <psobject> [<CommonParameters>]

		```


		#### Output Type

		The output type will be `MarkdownInfo` object with properties for `Html`, `VT100EncodedString` and `Tokens`.


		```

		Render the `VT100EncodedString` property of `MarkdownInfo` on console.

RFC for native markdown parsing on console #116

RFC for native markdown parsing on console #116

Uh oh!

Conversation

adityapatwardhan commented Mar 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DerpMcDerp commented Mar 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan commented Mar 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Mar 6, 2018 •

edited

Loading

adityapatwardhan Mar 19, 2018 •

edited

Loading