Skip to content

Allow CompleteInput to return results from ArgumentCompleter when AST or Script has matching function definition #10574

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 10 commits into from
Dec 2, 2019
Merged

Allow CompleteInput to return results from ArgumentCompleter when AST or Script has matching function definition #10574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7b5dffe
Fallback to commandAst for commandName to locate CustomArgumentCompleter
M1kep Sep 18, 2019
34fb67e
Add test for CompleteInput when an ArgumentCompleter exists
M1kep Sep 19, 2019
baae993
Fix spacing issue
M1kep Sep 19, 2019
22f3e31
Address Feedback.
M1kep Sep 22, 2019
ae8439f
Fix parenthesis placement.
M1kep Sep 22, 2019
c93c879
Address feedback.
M1kep Oct 9, 2019
daaca3e
Fix spacing.
M1kep Oct 9, 2019
1b98abf
Apply suggestions from code review
M1kep Oct 31, 2019
ffebee6
Address feedback around logic flow
M1kep Oct 31, 2019
10de9e5
Update src/System.Management.Automation/engine/CommandCompletion/Comp…
M1kep Nov 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1993,16 +1993,27 @@ private static void NativeCommandArgumentCompletion(
CompletionContext context,
Dictionary<string, AstParameterArgumentPair> boundArguments = null)
{
if (string.IsNullOrEmpty(commandName))
string parameterName = parameter.Name;

// Fall back to the commandAst command name if a command name is not found. This can be caused by a script block or AST with the matching function definition being passed to CompleteInput
// This allows for editors and other tools using CompleteInput with Script/AST definations to get values from RegisteredArgumentCompleters to better match the console experience.
// See issue https://github.com/PowerShell/PowerShell/issues/10567
string actualCommandName = string.IsNullOrEmpty(commandName)
? commandAst.GetCommandName()
: commandName;

if (string.IsNullOrEmpty(actualCommandName))
{
return;
}

var parameterName = parameter.Name;
var customCompleter = GetCustomArgumentCompleter(
"CustomArgumentCompleters",
new[] { commandName + ":" + parameterName, parameterName },
context);
string parameterFullName = $"{actualCommandName}:{parameterName}";

ScriptBlock customCompleter = GetCustomArgumentCompleter(
"CustomArgumentCompleters",
new[] { parameterFullName, parameterName },
context);

if (customCompleter != null)
{
if (InvokeScriptArgumentCompleter(
Expand Down
26 changes: 26 additions & 0 deletions test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,32 @@ Describe "TabCompletion" -Tags CI {
$res.CompletionMatches[1].CompletionText | Should -BeExactly 'dog'
}

It "Tab completion for ArgumentCompleter when AST is passed to CompleteInput" {
$scriptBl = {
function Test-Completion {
param (
[String]$TestVal
)
}
[scriptblock]$completer = {
param($commandName, $parameterName, $wordToComplete, $commandAst, $fakeBoundParameters)

@('Val1', 'Val2')
}
Register-ArgumentCompleter -CommandName Test-Completion -ParameterName TestVal -ScriptBlock $completer
}
$pwsh = [PowerShell]::Create()
$pwsh.AddScript($scriptBl)
$pwsh.Invoke()
Copy link
Member

@daxian-dbw daxian-dbw Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem right. After invoking, Test-Completion would already been defined, and it's not the scenario you are aiming at -- AST or Script has matching function definition.

Copy link
Contributor Author

@M1kep M1kep Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of this is as follows:

The script block is executed to define the Test-Completion function and argument completer in the runspace.(As the runspace should provide the registered argument completers when in an editor).

From there, we are using the $script variable for completion input(Similar to how VSCode utilizes CompleteInput) as the issue arises when the function is defined in the $script block being passed to CompleteInput


$completeInput_Input = $scriptBl.ToString()
$completeInput_Input += "`nTest-Completion -TestVal "
$res = [System.Management.Automation.CommandCompletion]::CompleteInput($completeInput_Input, $completeInput_Input.Length, $null, $pwsh)
$res.CompletionMatches | Should -HaveCount 2
$res.CompletionMatches[0].CompletionText | Should -BeExactly 'Val1'
$res.CompletionMatches[1].CompletionText | Should -BeExactly 'Val2'
}

It "Tab completion for enum type parameter of a custom function" {
function baz ([consolecolor]$name, [ValidateSet('cat','dog')]$p){}
$inputStr = "baz -name "
Expand Down