Have ModuleBuilder support comment-based help that is located prior to function-block

[johlju]

How ModuleBuilder currently works, and how we add comment-based help before the function-statement according to our style-guideline, the command in PlatyPS does not recognize the comment-based help and neither does the command Get-Help. This is because SetModuleContent does not add a blank line between the previous function (script file) and the next functions comment-based help (script file). If adding a blank line both PlatyPS and Get-Help will work.

I would be happy to send in a PR to resolve this. But would be an acceptable solution?

1. Make the function SetModuleContent always add a blank line between script files.
2. Make the function SetModuleContent add a line between script files when an optional parameter is passed to Build-Module.
3. Make the function SetModuleContent add a "script prefix file" prior to each script file when an optional parameter ScriptPrefix is passed to Build-Module containing the path to a script prefix file, e.g ScriptPrefix.ps1.
4. ... an even better option?

Code in SetModuleContent that will be affected: https://github.com/PoshCode/ModuleBuilder/blob/d183d880dd7d6be057facf3e26a8cf82aef96d1a/Source/Private/SetModuleContent.ps1#L49-L51

[johlju]

@Jaykul when you have time, could I get your input on this?

[Jaykul]

I'm totally fine with adding a blank line between each file all the time... Until you said this, I would have said it can't make any difference ;-)

The only problem is ... if you add it before the #Region will your CBH work? If we add it between the #Region and the first line of the file, it's going to screw up the calculation of source line numbers in a way that would be nearly impossible to reconcile with modules produced before the change, right?

[johlju]

We would have to make this change opt-in for modules that were built before the change to work. But I didn't think anyone would have that scenario, using an "old" built module to calculate coverage against potentially newer code. But it is a viable scenario so might be better to make this one opt-in?

I thought about code coverage even when this change is in place, and not entirely sure how to solve it, if you have suggestions I appreciate it.

Another thing about code-coverage. I will have to look into the tests for the functions ConvertTo-SourceLineNumber and ConvertFrom-SourceLineNumber and add a test that adds a space (and that fails at first) to see how it should be resolved in the code below. I'm guessing if we always add a blank line inside the #Region then we could aways use the offset - 1 (or +1 depending on how it calculates). But maybe it just works as-is. 🤔

https://github.com/PoshCode/ModuleBuilder/blob/d183d880dd7d6be057facf3e26a8cf82aef96d1a/Source/Public/ConvertTo-SourceLineNumber.ps1#L68-L84

https://github.com/PoshCode/ModuleBuilder/blob/d183d880dd7d6be057facf3e26a8cf82aef96d1a/Source/Public/ConvertFrom-SourceLineNumber.ps1#L55-L69

I will do the work for this then. I aim for it being opt-in unless you say we ignore the viable scenario above.

[kilasuit]

@johlju on this

How ModuleBuilder currently works, and how we add comment-based help before the function-statement according to our style-guideline,

This is a bad practise in adding CBH before a function definition which IMO the style guide you are using should change this recommendation because it breaks many other discoverability mechanisms within PowerShell both interactively & non-interactively & I actually would really like to see that option be removed in a future release as it breaks too many user expectations downstream and is not a good way of writing any comment based help which I partially started on this in this blog post, Beware where you place Comment based Help, which I need to update, it also lead me to making the PesterHelpers Module as you can't programmatically determine if a function has help or not, from the functions definition which is useful for different types of analysis.

Just my 2p worth, on why you shouldn't use CBH above a function definition, and is perhaps a design mistake to cater for those coming from other languages.

[johlju]

Thanks for your input. I haven't found a problem with it using AST and GetHelpContent() method as of yet.

Using CBH above the function statement is one of the valid ways of adding comment-based help.

[Jaykul]

I prefer the help at the top, but inside ... but since putting it outside (or at the bottom) works (as far as I can tell), I did not mean to break it.

I think, @johlju that you could just put a "-1`n" instead of a 0 in the "#Region '$SourceName' 0" on line 49 of SetModuleContent:

https://github.com/PoshCode/ModuleBuilder/blob/d183d880dd7d6be057facf3e26a8cf82aef96d1a/Source/Private/SetModuleContent.ps1#L49

And then add an optional "-" in the LineNumber pattern, like: (?<LineNumber>-?\d+) on line 52 of ConvertTo-SourceLineNumber (and anywhere else we have a similar regex?):

https://github.com/PoshCode/ModuleBuilder/blob/d183d880dd7d6be057facf3e26a8cf82aef96d1a/Source/Public/ConvertTo-SourceLineNumber.ps1#L52

Since line -1 would always be an empty line, there would be "no risk" (practically speaking) of the line number functions ever actually returning it, and it would be clear (at least to me) that the empty line was added by us.

What do you think?

[johlju]

@Jaykul that helped a lot! It was much easier than I thought. I sent in PR https://github.com/PoshCode/ModuleBuilder/pull/127 for review.