red-gate / XmlDoc2CmdletDoc

Create cmdlet XML help files from XML doc comments
Other
63 stars 24 forks source link

sorting parameters by position then by required and then by name in syntax generation #19

Closed rymir75 closed 8 years ago

rymir75 commented 8 years ago

Two cases when it's needed:

Additionaly MSDN doesn't guarantee that Type.GetMembers will return array of members in order of declaration https://msdn.microsoft.com/en-us/library/k2w5ey1e(v=vs.110).aspx

ChrisLambrou commented 8 years ago

Thank you for taking the time to check out the code and submit a PR. However, I don't really understand why the order that the parameters are defined in the .dll-Help.xml MAML help file has any bearing on either of the cases you've mentioned.

Two cases when it's needed:

when you use R# and sort class members by type, visibility and name

Are you referring to the the C# source editing feature of ReSharper, whereby it will order the fields and properties in its auto-completion lists, for example? If so, I don't see how controlling the ordering of the documentation in the MAML help file has any influence over this.

Are you referring to a PowerShell editing feature of ReSharper? If so, it's something I'm unaware of.

when you introduce a base cmdlet class with few base parameters (positioned)

Yes, this PR would sort the parameters irrespective of them being defined in the cmdlet class or a base class, but why does this matter? This would simply control the declaration order of the parameter elements in the MAML help file, but I'm not aware of any consumer of the file (e.g. the Get-Help cmdlet) that is sensitive to that ordering.

Could you please clarify in more detail exactly why the ordering of the parameters in the MAML help file is important, with a specific example?

rymir75 commented 8 years ago

I propose you an experiment. I'm sure that you have some MAML help file. Go to the syntaxItem element and change the order of positioned parameters e.g. move parameter with position 0 after a parameter with position 1, save it. Now, check how Get-Help renders syntax for changed cmdlet. Order of parameters in syntaxItem element has direct influence on generated syntax by Get-Help. It can mislead a user of the cmdlet.

This feature of R# is called "File Layout". If you have it configured that way that during code clean up you sort members of the class it can interchange properties e.g. property ParA (position 1) will be above property ParB (position 0). Similar effect will be if you have class hierarchy: "B" inherits from "A" and "A" inherits from "Cmdlet" and "A" has parameter ParA (position 0, type string) and "B" has parameter ParB (position 1, type string) then syntax for cmdlet will be: [-ParB] [-ParA] but should be exactly opposite.

I suppose that System.Type.GetMembers(BindingFlags) looks for members in the order they are declared and when we have deeper class hierarchy it first looks for members declared in the type and after it his parent and so on. Nevertheless MSDN documentation doesn't guarantee any order neither alphabetical nor declaration so we shouldn't depend on it.

I hope this clarifies my PR.

ChrisLambrou commented 8 years ago

Ah, I think I get it now. Get-Help detects the presence of the MAML help file, it displays its contents regardless of whether or not they actually match the API of the corresponding cmdlets. Honestly, the Get-Help cmdlet really isn't very good. I'm sorely tempted to dig out its source code and fix all the many, many problems I've found with it. Furthermore, the order parameters are declared in the source code may not match the explicit ordering defined by the Parameter attributes, so we should explicitly order them according to the Parameter attribute properties and the parameters' names, rather than the arbitrary order returned by Type.GetMembers.

Okay, will have another look at the code as there's one minor issue I wanted to dig into, and then hopefully merge it soon. Thanks again for this PR. :smile:

ChrisLambrou commented 8 years ago

Cheers! This has now been published to NuGet, and I've updated the contributors list in the project readme. :smile_cat:

rymir75 commented 8 years ago

Now, I can show off to my wife :smiley: It was a pleasure and my first contribution to open source.