Skip to content

'<#' does not trigger insert of comment based help ('##' does) #1228

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

Closed
csandfeld opened this issue Mar 15, 2018 · 26 comments
Closed

'<#' does not trigger insert of comment based help ('##' does) #1228

csandfeld opened this issue Mar 15, 2018 · 26 comments
Labels

Comments

@csandfeld
Copy link

System Details

- Operating system name and version: Windows 10 Enterprise Version 1709 (OS Build 16299.309)
- VS Code version: 1.21.1
- PowerShell extension version: 1.6.0.0
- Output from `$PSVersionTable`:
PSVersion                      5.1.16299.251
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.16299.251
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Issue Description

Typing <# right above/below a function statement, no longer triggers insert of comment based help. Typing ## does work, but I like the block comment better for the comment based help.

See GIF for details:
2018-03-15_12-49-47

Attached Logs

vscode-ps-logs.zip

@rkeithhill
Copy link
Contributor

This happens to me at times but usually I'm glad it fails because most of those times I'm commenting out a function and the auto-expansion to a help template is really bothersome.

@Cirzen
Copy link

Cirzen commented Mar 20, 2018

Keith makes a good point. I love the ability to auto insert the help comments, especially with the parameter names already filled out.
I'm not keen on the hash marker on every line though, I think it's uglier than the multi-line comment alternative, and it gets annoying having to insert '#' and set the indent correctly. Would an alternate snippet shortcut be better?

@GABeech
Copy link

GABeech commented Mar 20, 2018

In addition to the '#' style being uglier the recommended style is to use '<#' block comments.

@TylerLeonhardt
Copy link
Member

So it seems that this a pretty opinionated feature. However, we have some conflicting experiences.

  • If we allow <# to create block comments, then we hurt those that want to comment out functions.
  • If we don't have snippet to create block comments, then we hurt those that like that style of help.

Based on this, I wonder if I can convince those that want a block comment snippet if we can change the trigger from <# to something else like block help, help, <##, or something else... and we add new docs to say how to add new user snippets that would allow you to set <# if you so choose.

That way, both parties have their experience.

Thoughts?

@Cirzen
Copy link

Cirzen commented Mar 20, 2018

Happy with any particular shortcut.
"help" is already an alias though, so my instinct would be to shy away from that. I quite like <##
The comment based help is equally applicable for entire ".ps1" files as well as functions, so I would add the request that it should work for both.

@GABeech
Copy link

GABeech commented Mar 20, 2018

@tylerl0706 any reason to not make it a config option?

Could be as simple as "powershell.autoGenerateHelp": "off|block|comment|on"

off= don't generate
block = block style ('<# #>')
comment = comment style ('##')
on = enable both.

Default it to on, and people can tweak the config to suit their needs and wants.

@SeeminglyScience
Copy link
Collaborator

Slightly more ambiguous solution:

Make the completion provider API available from PowerShell/C# (like was done with code lens and symbols). Then refactor the existing HelpCompletionProvider to implement that API and allow it to be removed/adjusted from PowerShell.

@brettmillerb
Copy link
Contributor

I think the issue here is that the last update broke the way that it worked and typing <# used to generate the help but it doesn't work anymore.

The preferences are also a good discussion but something broke in the last update.

@TylerLeonhardt
Copy link
Member

@SeeminglyScience oh I see this is not just a snippet. Got it. Then you're right, completion provider should be available from PowerShell/C#. That way we can also do Register-EditorCompletion or something.

Btw everyone, there is a snippet for <# help called: comment-help!
https://github.com/PowerShell/vscode-powershell/blob/master/snippets/PowerShell.json#L557-L577

Please use that as a work around until this issue is fixed!

@brettmillerb
Copy link
Contributor

The <# used to generate help and include help for already declared parameters. Was awfully handy 👍

@csandfeld
Copy link
Author

Exactly @brettmillerb - it used to work when then typing <# but no longer does (I should have been more clear about that when opening the issue).

And as you mentioned above, it was more handy than a regular snippet, as it generated help for already declared parameters - something the snippet does not.

Looking at HelpCompletion.ts it still appear to be setup as a trigger, but for whatever reason it does not work in the latest version.

@GABeech
Copy link

GABeech commented Mar 21, 2018

After Doing a bit of digging. It seems like the following is happening:

when you type <# it auto-completes to <##> (no space, not sure if this is relevant),

Taking a peek at what is happening in the onEvent function in HelpCompletion.ts, this auto complete is showing up as two text changes:

1: <
2: ##>

This causes the matcher to not match and thus not insert the generated help.

@TylerLeonhardt
Copy link
Member

Great find @GABeech!

@ClintWeaver
Copy link

Would it be possible to create a Command Pallet action to trigger the generation of comment-based help. While we're at it, I would also recommend creating a Settings option for the default location of comment-based help. I'm sure people are very passionate about where the correct place is for that block of text.

If I had a say in the matter, the default should be between the function name and the param block, if only to allow for better code collapsing.

function Test-Function {
<#
  .SYNOPSIS
    Blah
#>
[CmdletBinding()]
param()

@rkeithhill
Copy link
Contributor

OK, I found the culprit and unfortunately it is VSCode. In the file C:\Program Files (x86)\Microsoft VS Code\resources\app\extensions\powershell\language-configuration.json there is this decl:

	"autoClosingPairs": [
		["{", "}"],
		["[", "]"],
		["(", ")"],
		{ "open": "\"", "close": "\"", "notIn": ["string"]},
		{ "open": "'", "close": "'", "notIn": ["string", "comment"]},
		["/**", " */"],
		["<#", "#>"]
	],

Remove that last decl for <# #> and voila, help-completion works again. So we need to figure out who on the VSCode we need to get this changed for 1.22.

Also, I'd really, really like to change the trigger char sequence from <# to <## . In C#, there is fortunately a distinction between a line comment \and a doc comment\` that avoids unwanted doc comment auto-expansion.

In VSCode I used to get the help completion when I just wanted to block comment out a function. I can easily change the trigger char seq to fix this but we will need to document it very visibly to get folks to change their behavior. To make this work I might have to change the ## trigger to ###.

If we decide not to change the trigger seqs then I'll definitely add a setting to turn off this feature to keep my sanity. :-)

Thoughts?

@rkeithhill
Copy link
Contributor

OK so using <## is a bust because it leaves the file in a state where the PSES code that generates the help can't find the FunctionDefintionAst on the subsequent/previous line. That is, the last character that triggers the help completion is not saved to file so it's not part of the AST. Apparently (verified) a single < on a line before a function does cause a problem but <# does. Go figure.

So one option would be to remove the <# trigger which would allow VSCode to keep the autoClosingPair for <##>. This would leave ## as the only help completion trigger. We could then add a setting to control the form of the help. I'd probably default the style to Block style but have setting for Line and None.

@csandfeld
Copy link
Author

Good find @rkeithhill.

So the issue logged here comes from fixing issue microsoft/vscode#9799 in commit microsoft/vscode@2504d71.

Personally I see limited benefit of auto-closing block comments. Typically I use block comments to comment out parts of the code I am working on, during development. When doing that, auto-closing the block comment is just annoying, as I will have to remove the auto inserted #>, to place it where I want it.

@TylerLeonhardt
Copy link
Member

@csandfeld raises a very good point. Very rarely, if ever, do I ever want the block comment to be automatically closed.

@rkeithhill if we kept in the autoClosingPair for <##> can we still use <# or rather <##> as a help trigger? (assuming the proper setting is set to Block)

@rkeithhill
Copy link
Contributor

rkeithhill commented Mar 25, 2018

The code as designed only works with two character triggers.

BTW why does the PowerShell lang config file contain this auto-closing pair:

		["/**", " */"],

WTF? And C# has a bit better auto-closing def that at leasst puts a space in there:

{ "open": "/*", "close": " */", "notIn": ["string"] }

So we should probably use this for PowerShell:

{ "open": "<#", "close": " #>", "notIn": ["string", "comment"] }

The bulk of the help comments I've seen and created use Block comment style. I'm really leaning towards a setting and a single trigger seq - ##.

@brettmillerb
Copy link
Contributor

I agree that ## with a setting to control behaviour is the best way to satisfy more people.

Alternatively as it's technically a snippet can we not use help-block along the lines of comment-help which already exists or is there a need to distinguish between the two different types of snippet?

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Mar 27, 2018

@brettmillerb makes a good point, if we already have the advanced snippet ## we might as well remove the vscode snippet and use help-block, comment-help and whatever other triggers to trigger the advanced snippet.

I haven't explored that part of the code much, but that makes sense to me.

Plus if the work is being done in PSES (which it should if it isn't), all the editors will get it for cheap.

@rkeithhill
Copy link
Contributor

rkeithhill commented Mar 27, 2018

Can we not use help-block along the lines of comment-help which already exists or is there a need to distinguish between the two different types of snippet?

There is a difference. The help completion provider understands the AST of the function and creates boiler-plate text for each parameter. That is one (maybe the only) feature that folks really like.

I think I'm going to create a PR with this change - ## is only trigger with a setting to disable this feature or select the comment style (block or line). And then we can see what folks think in the weekly drops.

BTW the help generation could be smarter and take into account [OutputType([type])] to put in the .OUTPUT section. Ditto for reading [Parameter()] attr values for populating .INPUT.

@csandfeld
Copy link
Author

The only drawback I see from not having <# trigger help completion, is limited discoverability of the feature (I would not have noticed the feature, had it not been for the <# trigger). With that said, I think the ## trigger with an option to configure the style would work well.

Besides the trigger, maybe a "PowerShell: Insert comment based help" option could be added to the command palette? And if possible, also an option to update already generated help (usefull when parameters have changed).

The suggested .INPUT/.OUTPUT additions to the help generation would also be usefull.

@Jaykul
Copy link

Jaykul commented Mar 27, 2018

Personally, I wish the help generation would only kick in when I really ask for it. I dislike using .PARAMETER in comment-based help, so there's not much I want generated in help anyway (a little snippet is enough).

I also never want help outside the function...

Maybe it could trigger off a period... but only when it's as the first non-whitespace, non-comment character, within the first comment block in a script / script block / function... and there's no param() above it. 🙄

P.S. I definitely think they should have a space on the closing #> as suggested. I don't mind the auto-closing, but normally I want to type: <# and have insert the #> but then 9 times out of 10 I'm going to hit {Enter} and I want it to indent and insert an extra line break before the closing #> ... something like:

<#
    {CursorHere}
#>

Then, if I type . ... that's when help-injection would be useful 😉

@rkeithhill
Copy link
Contributor

I wish the help generation would only kick in when I really ask for it.
... dislike using .PARAMETER in comment-based help

Agreed on both points.

I also never want help outside the function...

I know how you feel. I never want help inside the function... Years of doing C# doc comments has just burned in that mental pathway - permanently. :-)

We should be able to get the VSCode folks to tweak that auto-closing declaration.

Then, if I type . ... that's when help-injection would be useful

That's an interesting approach but would require a good bit of change to the PSES help completion code.

@TylerLeonhardt
Copy link
Member

Also wanted to chime in and say that having this setting set to null or empty string should turn off this setting completely.

rkeithhill added a commit that referenced this issue Apr 22, 2018
Changing this setting does require a reload window.

Trigger sequence changed to just "##".

Fix #1228
rkeithhill added a commit that referenced this issue Apr 22, 2018
* Add poweshell.helpCompletion setting to control/disable feature

Changing this setting does require a reload window.

Trigger sequence changed to just "##".

Fix #1228

* Rename triggerFinderLineComment field per PR review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants