Missing <command:syntaxItem> in the output maml file
I have a markdown help file ( Get-Test.md.txt ) created with New-MarkdownHelp that has 2 parameter sets. Calling New-ExternalHelp results in a MAML that has only 1 <command:syntaxItem>. After a little effort to isolate it I found that it is only reproduced when all parameter definitions in the .md file mention the second parameter sets or (All). If I change a parameter set name from (All) to "Default" the MAML file is generated correctly and contains 2 <command:syntaxItem>.
PlatyPS version 0.11.1
I tried it on the attached md file and it generates 2 param sets as expected. Can you attach the problematic markdown?
when all parameter definitions in the .md file mention the second parameter sets or (All).
So if I try
function foo
{
param(
[Parameter(ParameterSetName="p1")]
[string]$foo,
[string]$bar
)
}
Then Get-Help puts it all into a single parameter set.
PS /Users/vors/Downloads> Get-Help foo
NAME
foo
SYNTAX
foo [-foo <string>] [-bar <string>] [<CommonParameters>]
ALIASES
None
REMARKS
None
Sorry. I've uploaded wrong file. Get-Test.md.txt So the file uploaded above results in only 1 syntax item with 1 required parameter and 1 optional parameter which is not correct. It should generate additional syntax item with 1 optional parameter.
Did you modify the markdown parameter sets or syntax? It seems that if you have a single parameter set declared on 1 parameter and no parameter set on another parameter, powershell will just assume you have a single parameter set.
You can see that for yourself:
function foo
{
param(
[Parameter(ParameterSetName="p1")]
[string]$foo,
[string]$bar
)
}
PS /Users/vors/Downloads> (Get-Command foo).ParameterSets | ft
Name IsDefault Parameters
---- --------- ----------
p1 False {foo, bar, Verbose, Debug...}
The problem with the single parameter set(as showed in the example md) is that the default parameter set may have only optional parameters and the second parameter set may have a required parameter. So the generated (single) syntax item now has a mandatory parameter which means the parameter is always mandatory. This is not correct since the default parameter set has no mandatory parameters. This is not reproduced with the UI help editor.
I've generated the md file by using New-MarkDownHelp from binary module (c#). Since the problem exists with a relatively big cmdlet I've isolated the issue by removing and renaming some parts of the MD.
Oh I see your point. Looks like powershell doesn't handle this case separately:
function foo
{
param(
[Parameter(ParameterSetName="p1", Mandatory=$true)]
[string]$foo,
[Parameter(Mandatory=$false)]
[string]$bar
)
}
PS /Users/vors/Downloads> foo -bar 123
cmdlet foo at command pipeline position 1
Supply values for the following parameters:
foo:
PS /Users/vors/Downloads> (Get-Command foo).ParameterSets | ft
Name IsDefault Parameters
---- --------- ----------
p1 False {foo, bar, Verbose, Debug...}
I see that PS has a problem too. I'm not sure how this is related to MD to MAML conversion since the markdown already contains 2 syntax items and the result MAML file contains only 1 syntax item.
See my latest example that illustrates the problem correctly. The cmdlet has 2 parameter sets but the "Default" parameter set syntax item is not generated. Get-Test.md.txt
I've modified your example to create a repro script:
function foo
{
param(
[Parameter(ParameterSetName="Default")]
[Parameter(ParameterSetName="p1")]
[string]$bar,
[Parameter(ParameterSetName="p1", Mandatory=$true)]
[string]$foo
)
}
New-MarkdownHelp -Command foo -OutputFolder ./
New-ExternalHelp -Path .\foo.md -OutputPath ./
Note the result MAML file contains 1 syntax item with 2 paramters. The default parameter set syntax item is missing.
Ah I finally see what you mean :)
Note that Default is not special name for that matter, it will work with any other name.
Can you please take a look at the PR above that contains the fix?
oh forgot to click "publish the review" button
Oh, I got it all wrong. :) I now see the problem in the code. Let me summarize:
Problem: The "GatherSyntax()" doesn't have enough information to generate the correct syntax items since it relies only on information in the "## PARAMETERS" section. This section may not contain all parameter set names because if parameter is in all parameter sets it is marked as "Parameter Sets: (All)".
Possible solutions:
- Option 1 (Recommended): Generate MAML syntax items based on the information in the MD. Is there a reason the code is not implemented that way?
- Option 2: Collect information about parameter sets from "parameters" + "syntax" sections in the MD file. Use it in GatherSyntax() to generate correctly syntax items.
- Option 3: Drop "(All)" moniker and expand to all parameter set names when generating the MD file. This will eliminate any possible problems. Example: "Parameter Sets: (All)" -> "Parameter Sets: ParameterSet1, ParameterSet2".
Please share thoughts on the Options listed above. Thanks, f0rt
Ah that's right!
We cannot drop (All) because this is what PowerShell uses when there are no parameterSets specified. I.e.
function foo
{
param(
[Parameter(ParameterSetName="Default")]
[Parameter(ParameterSetName="p1")]
[string]$bar,
[Parameter(ParameterSetName="p1", Mandatory=$true)]
[string]$foo,
[string]$alll
)
}
Get-Help foo would give us
PARAMETERS
-alll <string>
Required? false
Position? Named
Accept pipeline input? false
Parameter set name (All)
Aliases None
Dynamic? false
-bar <string>
Required? false
Position? Named
Accept pipeline input? false
Parameter set name p1, Default
Aliases None
Dynamic? false
-foo <string>
Required? true
Position? Named
Accept pipeline input? false
Parameter set name p1
Aliases None
Dynamic? false
We should distinguish (All) that arises when there no parameter sets specified from the full enumeration of all parameter sets.
What about Option1?
Syntax section is currently write-only and that's how it's designed to be. This is done to avoid dealing with problems when it's a) misformatted b) inconsistent with parameters metadata section
So we're going with Option 2? If not please propose solution and I'll implement. Thanks.
Wait, I'm confused. What's the difference between 1 and 2?
I was proposing to do a variant of 3, where we distinguish cases when we have implicit (All) when any parameter names are omitted and an explicit enumeration of all param sets. The later one should be expanded as suggested, the former should be kept as is.
- In Option 1 the syntax items are copied directly from SYNTAX section in the MD file. In Option 2 the syntax items are generated again based on the parameter set names in the SYNTAX section and the information in the PARAMETERS section. I guess we are dropping these since the SYNTAX section is "write-only".
I was proposing to do a variant of 3, where we distinguish cases when we have implicit (All) when any parameter names are omitted and an explicit enumeration of all param sets. The later one should be expanded as suggested, the former should be kept as is.
This will work only if the users create/update the MD file with the PlatyPS cmdlets. If they do it manually it will be a little confusing. I guess should be documented somehow (comment section in the MD file etc.). Anyway, they solution sounds good enough for me. Let me know if you are still ok with it and I'll start implementing it.
If they do it manually
Parameters metadata, in general, should not be touched manually. It could in theory, but Update-MarkdownHelp should be doing a much better job for free.
Yes, that plan sounds good to me. Thank you for taking a stab at it!
Hi!
I have replaced "(All)" with ParameterSets names separated by comma. (I fixed it in Markdownv2Renderer.cs, Method : AddParameter) Only two unit tests failed because of the expected result (They expected "(All)"), The names of unit tests which failed are: RenderesAllParameterSetMoniker and RendererProduceMarkdownV2Output I fixed them to expect ParameterSets names.
I have two questions:
Is it OK to be fixed that way? Do you need "(All)" for some specific case which is not covered by Unit tests?
I would like to add some additional unit tests. Are you ok?
Best Regards, Ivanka Zaycheva
Hi @vzay !
Yes, thats the right way to proceed, but we should only unroll the (All) into the explicit list of all parameter names when it's that way in the source code.
That would be consistent with how default generated help works (see https://github.com/PowerShell/platyPS/issues/401#issuecomment-430472533)
Basically there is a logic somewhere in platyPS that checks that we cover all parameter cases and turns them into (All) and we should delete it.
+1 - Azure PowerShell team is running into this issue as well. Let me know what I can do to push this fix along :)
Based on review of the conversation, it appears that this is by design. Wait for Microsoft.PowerShell.PlatyPS v1 and see if you think there needs to be a change in what it produces.