PowerShell / platyPS

Write PowerShell External Help in Markdown
MIT License
783 stars 155 forks source link

New-MarkdownHelp Generates Markdown that violates many MD rules #345

Closed PlagueHO closed 5 years ago

PlagueHO commented 6 years ago

Steps to reproduce

Execute New-MarkdownHelp against any modules with cmdlets including help docs. Attempts should be made to meet (or suppress) standard MD rules as these all get highlighted in VSCode is rule violations.

image

Expected behavior

# New-CosmosDbContext

---
external help file: CosmosDB-help.xml
Module Name: CosmosDB
online version:
schema: 2.0.0
---

## SYNOPSIS

Create a context object containing the information required
to connect to a CosmosDB.

## SYNTAX

### Context (Default)

` ` `
New-CosmosDbContext -Account <String> [-Database <String>] -Key <SecureString> [-KeyType <String>]
 [<CommonParameters>]
` ` `

Actual behavior

---
external help file: CosmosDB-help.xml
Module Name: CosmosDB
online version:
schema: 2.0.0
---

# New-CosmosDbContext

## SYNOPSIS
Create a context object containing the information required
to connect to a CosmosDB.

## SYNTAX

### Context (Default)
` ` `
New-CosmosDbContext -Account <String> [-Database <String>] -Key <SecureString> [-KeyType <String>]
 [<CommonParameters>]
` ` `

Environment data

v0.9.0

Note: Who is the maintainer for this module? Is it still under active development?

vors commented 6 years ago

Hi @PlagueHO , thank you for the report.

standard MD rules

What is standart MD rules? We are mostly using CommonMark spec for our markdown guidelines with few extensions. All the problems reported by this VS Code markdownlinter don't violate the spec and seems quite arbitrary in my opinion.

  1. The top-level metadata is yaml_metadata_block which is a common extension of markdown.

  2. Blanks around the headers is not something specified by the spec, see https://spec.commonmark.org/0.28/#atx-heading

  3. Same for the blanks around fenced blocks https://spec.commonmark.org/0.28/#fenced-code-blocks There is even an explicit statement that blank lines are not required:

Other blocks can also occur before and after fenced code blocks without an intervening blank line

The rationality why it was chosen to avoid extra new lines is to make the resulting document tighter and more convenient to edit / read in raw form. @BernieWhite actually implemented a feature #321 recently that preserves most of the blank line formatting, when Update-MarkdownHelp is called. If you like a different line spacing, you can reformat markdown and it will be preserved.

Note: Who is the maintainer for this module? Is it still under active development?

Mostly me with lots of help from @BernieWhite , also PS team is maintaining it. I would not say it's under very active development just now, but it's surely not abandoned. Contributions are always welcome.

PlagueHO commented 6 years ago

Thanks for the great response @vors - it is much appreciated. And the work you guys do on this module is awesome.

To give you some background: I have been looking into using this module to generate some of the Markdown and other documentation for the DSC Resource Kit resources. However, the DSC Resource Kit resources has some checks that run in the CI that validate against these Markdown rules. But now I see these aren't the same set of rules (sorry about that - my bad :grin:).

Would it be worth implementing a switch on Update-MarkdownHelp to allow controlling of the markdown generation rules (or an alternate markdown template perhaps?). Either way as you're open to contributions, I'm happy to contribute this. Just want to make sure it was in alignment with what the longer term plans for this module is. E.g. I want to make sure I'm going in the right direction as per your plans for this.

vors commented 6 years ago

I think Update-MarkdownHelp should just try to preserve the format as is, but adding some options to New-MarkdownHelp is a good idea. Maybe a simple switch -NewLinePadding would be a good start, but if you have ideas how to make it more generically with some template, that would be great too.

DSC Resource Kit resources has some checks that run in the CI that validate against these Markdown rules

It's really up to you guys, how you want to work on https://github.com/PowerShell/DscResources . But as original author of https://github.com/PowerShell/DscResource.Tests I'd suggest to relax these draconic markdown linter rules :D .

There are many more useful things you can do with markdown:

I just want to highlight that there are many more reasonable and useful inveriants to auto-check.

Failing the CI build because somebody writes

# Foo
Bar

And not

# Foo

Bar

Doesn't seem fair to me.

PlagueHO commented 6 years ago

Yes, you're right - a switch to New-MarkdownHelp would be a much better approach. You raise some excellent points too about "how many rules is too many" and hopefully we haven't gone too mad with "validation" :grin:

The good thing is there is an "opt-in" process where each resource module can opt-in to the "soft" tests they want to apply. MD validation is only one type of "soft" test. @TravisEz13 added the ability to suppress individual markdown rules in this test as well, so it is a bit flexible. So some resource modules are still pretty loose and apply only the "harder" tests (MOF validation, file format, etc).

I love your suggestions:

Anyway, some great points and something to think about (for me).

BernieWhite commented 6 years ago

Addition of a new parameter might be the only case where this might be handy.

But yes generally Update-MarkdownHelp should preserve existing formatting.

FISHMANPET commented 5 years ago

Came here because I was curious about all the warnings the markdownlinter was throwing in VSCode as well. I came to have markdownlinter installed because I contributed to the Microsoft Powershell docs, and their repo recommends you install the markdownlinter extension and includes a configuration file within the repo to specify how exactly the rules are applied. For what it's worth, the official Powershell docs require blanks around headers. So I'd support some kind of options to New-MarkdownHelp that expands the functionality... somehow. I agree that maybe they're arbitrary and not very useful, but there will probably be other newbies like me that will take this same journey and end up wondering why platyps is giving me all these yellow exclamation marks.

I don't really have any specific recommendations but just thought I'd share a perspective that might further explain this

For reference: https://github.com/MicrosoftDocs/PowerShell-Docs/blob/staging/contributing/4-MARKDOWN-SPECIFICS.md https://github.com/DavidAnson/markdownlint#optionsconfig

vors commented 5 years ago

Good point, should we suggest alternations to the https://github.com/MicrosoftDocs/PowerShell-Docs/blob/staging/contributing/4-MARKDOWN-SPECIFICS.md instead of trying to adopt the platyPS? Trying to fix the tool to some (semi-arbitrary in my opinion) set of markdown lints seems like a fixing the problem at the wrong level to me.

vors commented 5 years ago

For historical context, platyPS pre-dates the PowerShell-Docs repo, which is the platyPS consumer. So it's odd to expect to alternate the tool to me.

vors commented 5 years ago

closing old issues as won't fix