[Bug] When completion Item description is empty, Powershell completion is not working

[SilkyFowl]

cobra/powershell_completions_test.go

Lines 50 to 85 in 7f8e83d

	"[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, '')",
	"[CompletionResult]::new('sub2', 'sub2', [CompletionResultType]::ParameterValue, '')",
	"'tree;sub1'",
	"[CompletionResult]::new('sub11', 'sub11', [CompletionResultType]::ParameterValue, '')",
	"[CompletionResult]::new('sub12', 'sub12', [CompletionResultType]::ParameterValue, '')",
	"'tree;sub1;sub11'",
	"'tree;sub1;sub12'",
	"'tree;sub2'",
	"[CompletionResult]::new('sub21', 'sub21', [CompletionResultType]::ParameterValue, '')",
	"[CompletionResult]::new('sub22', 'sub22', [CompletionResultType]::ParameterValue, '')",
	"'tree;sub2;sub21'",
	"'tree;sub2;sub22'",
	},
	},
	{
	name: "flags",
	root: func() *Command {
	r := &Command{Use: "flags"}
	r.Flags().StringP("flag1", "a", "", "")
	r.Flags().String("flag2", "", "")
	sub1 := &Command{Use: "sub1"}
	sub1.Flags().StringP("flag3", "c", "", "")
	r.AddCommand(sub1)
	return r
	}(),
	expectedExpressions: []string{
	"'flags'",
	"[CompletionResult]::new('-a', 'a', [CompletionResultType]::ParameterName, '')",
	"[CompletionResult]::new('--flag1', 'flag1', [CompletionResultType]::ParameterName, '')",
	"[CompletionResult]::new('--flag2', 'flag2', [CompletionResultType]::ParameterName, '')",
	"[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, '')",
	"'flags;sub1'",
	"[CompletionResult]::new('-c', 'c', [CompletionResultType]::ParameterName, '')",
	"[CompletionResult]::new('--flag3', 'flag3', [CompletionResultType]::ParameterName, '')",

From this issue: cli/cli/issues/1775

I found that 4th argument of CompletionResult's constructor can't use empty string.

Steps to reproduce

1. Register completions.

 (gh completion -s powershell) -join "`n" | Invoke-Expression

2. Completion

gh <Ctrl+Space>

Expected behavior

Offer completions.

❯ gh credits
alias        auth         config       environment  help         pr           repo
api          completion   credits      gist         issue        release      version

View credits for this tool

Actual behavior

NOT offer completions.

However, <command> <tab>offer completions.

After investigating this bug, I found that if the 4th argument of CompletionResult's constructor is emptystring, it is interpreted as null and an MethodInvocationException is thrown.
In order to avoid this, it seems to be necessary to set some string, even if it is a whitespace.

using namespace System.Management.Automation
using namespace System.Management.Automation.Language

[CompletionResult]::new('version', 'version', [CompletionResultType]::ParameterValue, '')
[CompletionResult]::new('version', 'version', [CompletionResultType]::ParameterValue, ' ')

MethodInvocationException:
Line |
   4 |  [CompletionResult]::new('version', 'version', [CompletionResultType]: …
     |  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling ".ctor" with "4" argument(s): "Cannot process argument because the value of argument "toolTip" is null. Change the value of argument "toolTip" to a non-null value."

CompletionText ListItemText     ResultType ToolTip
-------------- ------------     ---------- -------
version        version      ParameterValue

So what should the 4th argument be set to?

when the 1st and 2nd arguments are the same

MsDocs said this.

$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

So I tried setting the same value as the 1st and 2nd arguments.

$testParams = @{
    Native      = $true
    CommandName = 'gh'
    ScriptBlock = {
        [CompletionResult]::new('repo', 'repo', [CompletionResultType]::ParameterValue, 'Create, clone, fork, and view repositories')
        [CompletionResult]::new('version', 'version', [CompletionResultType]::ParameterValue, 'version') # FIXED '' -> 'version'
    }
}
Register-ArgumentCompleter @testParams

From this result, it seems that if no details for the completions is shown, it's OK to set the same string as the 1st and 2nd arguments instead of an empty string.

when the 1st and 2nd arguments are different

This is also the case assumed in powershell_completions_test.go. I also checked how it behaves when the 1st argument, which is used as the auto completion result, and the 2nd argument, which is displayed in a list, are different.

$testParams = @{
    Native      = $true
    CommandName = 'flags'
    ScriptBlock = {
        [CompletionResult]::new('-a', 'a', [CompletionResultType]::ParameterName, 'a')
        [CompletionResult]::new('--flag1', 'flag1', [CompletionResultType]::ParameterName, 'flag1')
        [CompletionResult]::new('--flag2', 'flag2', [CompletionResultType]::ParameterName, 'flag2')
        [CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, 'sub1')
    }
}
Register-ArgumentCompleter @testParams

$testParams = @{
    Native      = $true
    CommandName = 'flags'
    ScriptBlock = {
        [CompletionResult]::new('-a', 'a', [CompletionResultType]::ParameterName, '-a')
        [CompletionResult]::new('--flag1', 'flag1', [CompletionResultType]::ParameterName, '--flag1')
        [CompletionResult]::new('--flag2', 'flag2', [CompletionResultType]::ParameterName, '--flag2')
        [CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, 'sub1')
    }
}
Register-ArgumentCompleter @testParams

It seems that at the least Console only needs to set one of the arguments and there is no clear difference.

In VScode

I was somewhat curious about the last one, so I looked into it and found some interesting results.

Whitespace and When the 1st and 2nd arguments are the same

In the same case as the 1st argument

In the same case as the 2nd argument

Considering the difference, I wondered if it should be the same string as the 1st argument.
Because in some settings it is possible to see the 1st argument which is not displayed until selected.

FIX

So my suggestion is to modify the test case as follows.
I think fixing it to pass this modified test will fix the bug.

diff --git a/powershell_completions_test.go b/powershell_completions_test.go
index 29b609d..4b46fb6 100644
--- a/powershell_completions_test.go
+++ b/powershell_completions_test.go
@@ -47,16 +47,16 @@ func TestPowerShellCompletion(t *testing.T) {
                        }(),
                        expectedExpressions: []string{
                                "'tree'",
-                               "[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, '')",
-                               "[CompletionResult]::new('sub2', 'sub2', [CompletionResultType]::ParameterValue, '')",
+                               "[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, 'sub1')",
+                               "[CompletionResult]::new('sub2', 'sub2', [CompletionResultType]::ParameterValue, 'sub2')",
                                "'tree;sub1'",
-                               "[CompletionResult]::new('sub11', 'sub11', [CompletionResultType]::ParameterValue, '')",
-                               "[CompletionResult]::new('sub12', 'sub12', [CompletionResultType]::ParameterValue, '')",
+                               "[CompletionResult]::new('sub11', 'sub11', [CompletionResultType]::ParameterValue, 'sub11')",
+                               "[CompletionResult]::new('sub12', 'sub12', [CompletionResultType]::ParameterValue, 'sub12')",
                                "'tree;sub1;sub11'",
                                "'tree;sub1;sub12'",
                                "'tree;sub2'",
-                               "[CompletionResult]::new('sub21', 'sub21', [CompletionResultType]::ParameterValue, '')",
-                               "[CompletionResult]::new('sub22', 'sub22', [CompletionResultType]::ParameterValue, '')",
+                               "[CompletionResult]::new('sub21', 'sub21', [CompletionResultType]::ParameterValue, 'sub21')",
+                               "[CompletionResult]::new('sub22', 'sub22', [CompletionResultType]::ParameterValue, 'sub22')",
                                "'tree;sub2;sub21'",
                                "'tree;sub2;sub22'",
                        },
@@ -76,13 +76,13 @@ func TestPowerShellCompletion(t *testing.T) {
                        }(),
                        expectedExpressions: []string{
                                "'flags'",
-                               "[CompletionResult]::new('-a', 'a', [CompletionResultType]::ParameterName, '')",
-                               "[CompletionResult]::new('--flag1', 'flag1', [CompletionResultType]::ParameterName, '')",
-                               "[CompletionResult]::new('--flag2', 'flag2', [CompletionResultType]::ParameterName, '')",
-                               "[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, '')",
+                               "[CompletionResult]::new('-a', 'a', [CompletionResultType]::ParameterName, '-a')",
+                               "[CompletionResult]::new('--flag1', 'flag1', [CompletionResultType]::ParameterName, '--flag1')",
+                               "[CompletionResult]::new('--flag2', 'flag2', [CompletionResultType]::ParameterName, '--flag2')",
+                               "[CompletionResult]::new('sub1', 'sub1', [CompletionResultType]::ParameterValue, 'sub1')",
                                "'flags;sub1'",
-                               "[CompletionResult]::new('-c', 'c', [CompletionResultType]::ParameterName, '')",
-                               "[CompletionResult]::new('--flag3', 'flag3', [CompletionResultType]::ParameterName, '')",
+                               "[CompletionResult]::new('-c', 'c', [CompletionResultType]::ParameterName, '-c')",
+                               "[CompletionResult]::new('--flag3', 'flag3', [CompletionResultType]::ParameterName, '--flag3')",
                        },
                },
                {

Environment data

gh version 1.0.0 (2020-09-16)

VScode 1.49.1 x64
ms-vscode.powershell-preview@2020.9.0

Name                           Value
----                           -----
PSVersion                      7.1.0-preview.7
PSEdition                      Core
GitCommitId                    7.1.0-preview.7
OS                             Microsoft Windows 10.0.19041
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[Luap99]

Hey, I have a PR #1208 which brings a bunch of improvements for powershell completions including the support for custom args completion. This should fix your issue. I would appreciate if you would test this out. Thanks.

[SilkyFowl]

@Luap99 Thanks for letting me know. I'll test it out.

[Luap99]

@SilkyFowl Did you had time to test my PR?

[SilkyFowl]

@Luap99
Sorry for the delay.
We have confirmed that Issue bug in the issue has been fixed. Thank a lot.

However, I commented on one point that I was concerned about.
https://github.com/spf13/cobra/pull/1208/files#r500873024

I really wanted to do some detailed testing, but it was a foolhardy challenge for me to build and actually run it when I don't know the Go lang ......
I'm reviewing the code from a Powershell perspective only, so I may be missing the point.

[github-actions]

This issue is being marked as stale due to a long period of inactivity