Skip to content
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

Example for Register-ArgumentCompleter -Native has wrong parameter names #5393

Closed
Jawz84 opened this issue Feb 1, 2020 · 4 comments · Fixed by #5485
Closed

Example for Register-ArgumentCompleter -Native has wrong parameter names #5393

Jawz84 opened this issue Feb 1, 2020 · 4 comments · Fixed by #5485
Labels
area-core Area - Microsoft.PowerShell.Core module

Comments

@Jawz84
Copy link
Contributor

Jawz84 commented Feb 1, 2020

The naming of the scriptblock parameters in the example for the native argument completer are misleading/incorrect. This is the example:

$scriptblock = {
     param($commandName, $wordToComplete, $cursorPosition)
         dotnet complete --position $cursorPosition "$wordToComplete" | ForEach-Object {
            [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterValue', $_)
         }
 }
Register-ArgumentCompleter -Native -CommandName dotnet -ScriptBlock $scriptblock

When I set a breakpoint inside the scriptblock, I find that $commandName is of type String, and $wordToComplete is of type CommandAst. The $commandName variable holds the last 'partial word' on the command line, the word to complete.

This matches with what I found after digging in the code, at this line, the exact same pattern is used: CompletionCompleter.cs:1307

It seems to me the parameters in the example scriptblock should be named:
$wordToComplete, $commandAst, $cursorPosition

The example does work, because $commandAst.ToString() returns the full command line up to that point (eg. "dotnet --h"), and dotnet uses the --position x information to filter out where to start completing.

Proposed improved example:

$scriptblock = {
     param($wordToComplete, $commandAst, $cursorPosition)
         dotnet complete --position $cursorPosition $commandAst.ToString() | ForEach-Object {
            [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterValue', $_)
         }
 }
Register-ArgumentCompleter -Native -CommandName dotnet -ScriptBlock $scriptblock

Edit:

I realized later that there is more. The description for the -ScriptBlock parameter combined with the -Native parameter needs to be corrected too.

Current text:

$commandName (Position 0) - This parameter is set to the name of the command for which the script block is providing tab completion.
$wordToComplete (Position 1) - This parameter is set to value the user has provided before they pressed Tab. Your script block should use this value to determine tab completion values.
$cursorPosition (Position 2) - This parameter is set to the position of the cursor when the user pressed Tab.

Suggested improved version, adapted from the parameter description for non-Native:

$wordToComplete (Position 0) - This parameter is set to value the user has provided before they pressed Tab. Your script block should use this value to determine tab completion values.
$commandAst (Position 1) - This parameter is set to the Abstract Syntax Tree (AST) for the current input line. For more information, see Ast Class.
$cursorPosition (Position 2) - This parameter is set to the position of the cursor when the user pressed Tab.

Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@Jawz84 Jawz84 changed the title Example for -Native has wrong parameter names Example for Register-ArgumentCompleter -Native has wrong parameter names Feb 3, 2020
@sdwheeler sdwheeler added the area-core Area - Microsoft.PowerShell.Core module label Feb 3, 2020
@Jawz84
Copy link
Contributor Author

Jawz84 commented Feb 25, 2020

I'm willing to take this one on and write a PR for it.

@Jawz84
Copy link
Contributor Author

Jawz84 commented Feb 27, 2020

@sdwheeler can I ask what the usual time frame is for merged changes to appear live on the website? Just curious what I can expect. Thanks!

@sdwheeler
Copy link
Contributor

@Jawz84 It is a manual process. I try to publish to the live site once a week.

@Jawz84
Copy link
Contributor Author

Jawz84 commented Feb 27, 2020

@sdwheeler thank you! I think you guys are doing an amazing job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core Area - Microsoft.PowerShell.Core module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants