Skip to content

- Allow specification of markdown files to process with New-DocusaurusHelp. #201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 27, 2024

Conversation

mjr4077au
Copy link
Contributor

Hello there!

Within our module's build system, we have a bit of parsing that takes place to fix multi-line examples, remove superfluous PS C:\\\> parts within examples, and to also unescape backticks in descriptions and other parts of the comment-based help export.

This is an example of one of the mdx files generated via Docusaurus before we can get to parse the underlying platyPS export:
image

The extra PS C:\> parts are essential in the comment-based help so the example renders right from Get-Help, but of course it's not what we want in the export.

By being able to supply our own markdown files, we can have the perfect Docusaurus exports without having to reparse those files after the fact to try and correct the issues.
image

I wanted to see whether there'd be any interest in this minor extension upstream. If not, we can continue to maintain a friendly fork of the project to maintain the customisation on our end.

Copy link

vercel bot commented Nov 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docusaurus-powershell ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 27, 2024 11:34am

@bravo-kernel
Copy link
Contributor

Thank you for using and contributing. @fflaten could you please triple-check?

mjr4077au added a commit to PSAppDeployToolkit/PSAppDeployToolkit that referenced this pull request Nov 25, 2024
…build system.

Can use the public module once this PR is approved and the PR makes it into a release: alt3/Docusaurus.Powershell#201.
@bravo-kernel
Copy link
Contributor

If we proceed, this will be a supported feature. Do you think you will be able to provide a test case for it?

sintaxasn pushed a commit to PSAppDeployToolkit/PSAppDeployToolkit that referenced this pull request Nov 25, 2024
…build system.

Can use the public module once this PR is approved and the PR makes it into a release: alt3/Docusaurus.Powershell#201.
@mjr4077au
Copy link
Contributor Author

@bravo-kernel I understand and appreciate the concern. The function I was using as a demonstration is this one within our develop repository, and it can be seen that it has a 4-line example: https://github.com/PSAppDeployToolkit/PSAppDeployToolkit/blob/develop/src/PSAppDeployToolkit/Public/New-ADTValidateScriptErrorRecord.ps1

Without us explicitly providing the PS C:\> remark on each example line, the example doesn't render correctly in PowerShell 5 or 7. The below screenshots are with and without the remark, respectively.
image
image

Doing what we've done is widely known and accepted as the only solution to this problem

While this works great for Get-Help, it's not something platyPS knows how to handle. Our build system is based off Catesta, which is a popular solution and has almost 170 stars on GitHub. They provide a regex solution to parse the markdown exports to ensure multi-line examples stay within the triple backticks, and we've then extended it to remove the PS C:\> remarks so it looks correct at all times.

The potential alternative is to incorporate what we're doing in our build system into Docusaurus.Powershell so the generated markdown exports are able to be post-processed to repair this issue. I considered changing how Docusaurus.Powershell processes the files to be a larger change, and a potentially unwelcome one since if it's never come up before, maybe it's not an issue many, if any, are running into.

The code in our build system can be reviewed here: https://github.com/PSAppDeployToolkit/PSAppDeployToolkit/blob/7ebb7d227ca8d229ded51b48fee52ee0ebae80a2/src/PSAppDeployToolkit.build.ps1#L427-L439. If you feel something like this would be welcome to address the specific issue we have and make the Docusaurus output better for everyone, I'd be more than happy to submit something pertaining this.

@bravo-kernel
Copy link
Contributor

Thanks for the additional input, I think I understand better now and we might be able to implement it here.

You can try yourself by adding below new test case to the end of this integration test module.

        .EXAMPLE
        PS C:\>$paramName = "FilePath"
        PS C:\>$providedValue = "C:\InvalidPath"
        PS C:\>$exceptionMessage = "The specified path does not exist."
        PS C:\>New-ADTValidateScriptErrorRecord -ParameterName $paramName

        This example is already transformed into a code fenced block by Docusaurus.PowerShell.

Then run

invoke-pester .\Tests\Integration\CrossVersionCodeExamples\Integration.Tests.ps1

Now look inside %Temp\Alt3.Docusaurus.Powershell\CrossVersionCodeExamples.

You will see two files there:

  • Test-CrossVersionCodeExamples.PlatyPS.md
  • Test-CrossVersionCodeExamples.mdx

The MDX file is produced by the Alt3 module and already contains a properly configured fenced block with the following content.

$paramName = "FilePath"
PS C:\>$providedValue = "C:\InvalidPath"
PS C:\>$exceptionMessage = "The specified path does not exist."
PS C:\>New-ADTValidateScriptErrorRecord -ParameterName $paramName

This example is already transformed into a code fenced block.

You would like to see the three lines starting with PS C:\> stripped correct?

@fflaten
Copy link
Contributor

fflaten commented Nov 25, 2024

Thank you for using and contributing. @fflaten could you please triple-check?

Haven't tested it, but looks fine. Personally I'd rename the parameter to e.g. -PlatyPSMarkdownPath to be explicit, put it in a separate parameter set without -Module and parse module name from frontmatter ("Module Name: ...") to avoid user errors since $Module isn't validated in this flow.

However, I would like to propose an alternative solution.

Microsoft.PowerShell.PlatyPS (v1) is in preview and Docusaurus.PowerShell should migrate to it with a matching v2 preview. It has improved performance, many bugfixes and one of the key changes is an intermediate CommandHelp object that you can modify in PowerShell.

This will simplify how we fix examples, metadata (markdown frontmatter), links etc. New-DocusaurusHelp should also include a new CommandHelp[] input parameter so users can pre-process things like this as well.

It won't fix users stuck on platyPS v0.*, but I'm sure @mjr4077au would also like the improved DX of CommandHelp. 🙂

# concept
$commandHelp = New-CommandHelp -CommandInfo (Get-Command -Module PSAppDeployToolkit)
$commandHelp.Examples | % { $_.Remarks = $_.Remarks -replace '(?ms)^PS C:\\>' }
New-DocusaurusHelp -CommandHelp $commandHelp

@homotechsual
Copy link

Yeah new PlatyPS is going to change a lot of our customisations of this module too – perhaps a Docusaurus.PowerShell v2 might be able to find some way we can share common customisations – reading from a config file or something? We currently pre-process out the new PowerShell 7 common parameter and do a few other things like custom slugs using the "Subject" CBH field - cool and useful stuff but not stuff that feels broadly useful to others!

@mjr4077au
Copy link
Contributor Author

There's a lot to reply to here so I'll try and do my best 😅

@bravo-kernel the example output I provided above where it was broken was from Docusaurus, which would have been using platyPS 0.14.2. I'll provide it again for posterity:
image

Looking at the examples in https://github.com/alt3/Docusaurus.Powershell/blob/main/Tests/Integration/CrossVersionCodeExamples/CrossVersionCodeExamples.psm1, I believe your demonstration is only working because you've manually fenced off the code in the EXAMPLE sections with your own triple backticks? If that's the case, the problem with doing that is that it will break the output from Get-Help which we'd like to avoid:
image

Because I can post-process the platyPS exports to make them right, but I can't do anything to make Get-Help right except format the comment-based help in a way that it needs/expects, that's unfortunately the route I have to go down.

@fflaten thank you for your input. I've got absolutely nothing tying me to the old platyPS implementation, its just that they haven't put out a new preview of their next version for over 3 years. I hope it's everything that the current implementation is not because having to do fixes like this after the fact is asinine.

If there's still interest in what this PR proposes, I can change the parameter name to -PlatyPSMarkdownPath and ensure the module name is validated so there's no regression or issues.

@fflaten
Copy link
Contributor

fflaten commented Nov 25, 2024

its just that they haven't put out a new preview of their next version for over 3 years

It's not the same. They abandoned v2 and rewrote it from scratch with the new CommandHelp-architecture to make it extensible. They just released the first preview. https://devblogs.microsoft.com/powershell/announcing-platyps-100-preview1/

@mjr4077au
Copy link
Contributor Author

its just that they haven't put out a new preview of their next version for over 3 years

It's not the same. They abandoned v2 and rewrote it from scratch with the new CommandHelp-architecture to make it extensible. They just released the first preview. https://devblogs.microsoft.com/powershell/announcing-platyps-100-preview1/

Ah yes, I do recall that actually. One of our team members tested it and I believed it had issues parsing our dynamic parameters, but I'm yet to test it myself. I can't do too much with it just at the moment until it receives wider adoption, such as within projects like this that we're dependent on for our website. It's good that its coming though.

@bravo-kernel
Copy link
Contributor

Allright folks, let's keep this as simple as possible.

I agree with @fflaten and think this might be the time to create a v2 version of the module, perhaps making it more generic to cater e.g. Astro etc. as was the plan initially.

For your PR @mjr4077au, if you can update to follow below advice I will merge.

Personally I'd rename the parameter to e.g. -PlatyPSMarkdownPath to be explicit, put it in a separate parameter set without -Module and parse module name from frontmatter ("Module Name: ...") to avoid user errors since $Module isn't validated in this flow.

…-PlatyPSMarkdownPath`.

As requested, place it into a separate parameter set and munge out the module's name from the supplied files.
@mjr4077au
Copy link
Contributor Author

@bravo-kernel I've pushed the extra commit to address the requested changes. This has been tested locally on my end and works as expected.

I've also added one extra commit which I hope won't be an issue, but I couldn't build the module unless I was in the repository's root directory. I replaced the relative path in two spots of build-module.ps1 with $PSScriptRoot so the script can be called from any location.

@fflaten please let me know whether there's any concerns with the most recent commits.

… is mandatory within its parameter set.

Co-authored-by: Frode Flaten <[email protected]>
Copy link
Contributor

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the previous comments. One last from me.

…downPath` code to the first 10 files." to the correct intention of the reviewer.
…to be 1:1 before I started so Pester doesn't have a sad.
@bravo-kernel
Copy link
Contributor

Much much appreciated mr. @mjr4077au

@fflaten do you approve merging now?

@bravo-kernel
Copy link
Contributor

bravo-kernel commented Nov 27, 2024

@mjr4077au could you try running the build script with -GenerateDocs parameter so that the Alt3 website get-help page gets updated?

If it doesn't work or is a hassle, no worries. I will need to do a lot of updates anyways.

Copy link
Contributor

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks 🙏

@mjr4077au
Copy link
Contributor Author

@bravo-kernel that's done and pushed!

@bravo-kernel bravo-kernel merged commit 21d07b9 into alt3:main Nov 27, 2024
11 checks passed
@bravo-kernel
Copy link
Contributor

Amazing, just merged for a new release. Thank you kindly ♥

@bravo-kernel
Copy link
Contributor

sintaxasn pushed a commit to PSAppDeployToolkit/PSAppDeployToolkit that referenced this pull request Dec 4, 2024
…build system.

Can use the public module once this PR is approved and the PR makes it into a release: alt3/Docusaurus.Powershell#201.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants