Improve completion logic

[andyleejordan]

This makes the deduced CompletionItemKind enum more accurate, which
results in better icons for the user when viewing completions in an
editor. Also cleaned up the file as there were remnants of dead code.

Fixes PowerShell/vscode-powershell#3364

[andyleejordan]

@MartinGC94 I think you'll like this!

From your demo (ignore ProviderItem, Code does some magic to get filetype specific icons later):

Files:

Operators:

Namespace:

And even classes:

[MartinGC94]

You are right, this looks great and great choice of icons as well. The only icon that looks a little out of place is the ParameterName. I know it shares its icon with the operators but parameters are used much more frequently so maybe the chosen icon should look more like a parameter?

[SeeminglyScience]

Looks great! Just a couple of nits

[andyleejordan]

You are right, this looks great and great choice of icons as well. The only icon that looks a little out of place is the ParameterName. I know it shares its icon with the operators but parameters are used much more frequently so maybe the chosen icon should look more like a parameter?

@MartinGC94 That is an artifact of the way that test displays all of them at once and the logic that figures out if it's a parameter. They're actually currently mapped to CompletionItemKind.Variable (which was their previous mapping) and look like this:

However, I'm trying to decide if instead they should map to Field or Property (which are maybe slightly more accurate? The parameters declared on a PowerShell function are kind of like fields/properties).

Field (which is so far unused):

Property (which already has the PowerShell CompletionResultType.Property mapping to it):

[MartinGC94]

I see. All 3 of those are fine but if I had to pick I would go with Field because I like the icon and the fact that it's unique.

[JustinGrote]

Using other languages as prior art, parameters are generally considered variables (Typescript as example):

C# as another:

So I think it should stay as variable for a consistent experience.

[andyleejordan]

Thanks for pointing that out @JustinGrote, I agree. What tool are you using to inspect like that?

[JustinGrote]

@andschwa it's built into vscode, very useful for troubleshooting syntax highlighting

[andyleejordan]

@andschwa it's built into vscode, very useful for troubleshooting syntax highlighting

I'm so sad I've learned about this a year later...what else should I know?!

[JustinGrote]

@andschwa uuuh, you can access devtools for the running vscode instance maybe?

Basically check out everything prefixed with Developer:

[andyleejordan]

@JustinGrote @SeeminglyScience @MartinGC94 Give this new commit a whirl would you? I removed the middle abstraction layer as it seemed superfluous.

[andyleejordan]

Oops, probably tests expecting old code. Fixing up.

[SeeminglyScience]

LGTM, couple of little things