completionresolve support

[TylerLeonhardt]

Comes after #1007

This adds support for completionItem/resolve

[SeeminglyScience]

Awesome job 🙂

Got some comments/requests, none blocking though.

[SeeminglyScience]

Suggested change

	static public async Task<CommandCompletion> GetCompletionsAsync(
	public static async Task<CommandCompletion> GetCompletionsAsync(

[SeeminglyScience]

Since this delegate doesn't need a closure can we save it statically?

[SeeminglyScience]

Can you have this implement IEquatable<CompletionDetails> to better telegraph that it has custom comparisons? Also == and != operators.

[SeeminglyScience]

What about case sensitivity? Also ideally this wouldn't create an extra string. Here's the template I usually go with:

unchecked
{
    int hash = 17;
    hash = hash * 23 + CompletionText.GetHashCode();
    hash = hash * 23 + CompletionType.GetHashCode();
    hash = hash * 23 + ListItemText.GetHashCode();
    hash = hash * 23 + ToolTipText.GetHashCode();
    // If one can be null:
    hash = hash * 23 + SymbolTypeName?.GetHashCode() ?? 0;
    return hash;
}

(Taken from https://stackoverflow.com/a/263416)

[SeeminglyScience]

How do we feel about making LangVersion preview?

return completionResultType switch
{
    CompletionResultType.Command => CompletionType.Command,
    CompletionResultType.Method => CompletionType.Method,
    CompletionResultType.ParameterName => CompletionType.ParameterName,
    CompletionResultType.ParameterValue => CompletionType.ParameterValue,
    CompletionResultType.Property => CompletionType.Property,
    CompletionResultType.Variable => CompletionType.Variable,
    CompletionResultType.Namespace => CompletionType.Namespace,
    CompletionResultType.Type => CompletionType.Type,
    CompletionResultType.Keyword => CompletionType.Keyword,
    CompletionResultType.DynamicKeyword => CompletionType.Keyword,
    CompletionResultType.ProviderContainer => CompletionType.Folder,
    CompletionResultType.ProviderItem => CompletionType.File,
    // TODO: Trace the unsupported CompletionResultType
    _ => CompletionType.Unknown,
};

Alternatively since it's mostly one to one you could have a map array, e.g.

private static readonly s_completionTypeMap =
{
    CompletionType.Command,
    // etc
};

void Access(CompletionResultType resultType) => s_completionTypeMap[(int)resultType];

[SeeminglyScience]

Does this effect untitled files?

[TylerLeonhardt]

Good catch, I can change this to apply to only powershell files

[SeeminglyScience]

When is this called? Just on select? Or for every completion item?

[TylerLeonhardt]

It's called when a completion item is highlighted in the completion list

[SeeminglyScience]

re: the CompletionDetails.ExtractSymbolTypeNameFromToolTip comment

[SeeminglyScience]

Not sure if EndsWith optimizes this, but maybe better to have a static method like:

bool EndsWithQuote(string text)
{
    if (string.IsNullOrEmpty(text))
    {
        return false;
    }

    char lastChar = text[text.Length - 1];
    return lastChar == '"' || lastChar == '\'';
}

[TylerLeonhardt]

added

[SeeminglyScience]

re: expression switch/array map

[rjmholt]

I suspect this isn't threadsafe and should be a ConcurrentDictionary<string, bool> or something, where the value is never used

[rjmholt]

We should also change blacklist to exclusionlist

[TylerLeonhardt]

done

[TylerLeonhardt]

@SeeminglyScience a few of your comments were really towards #1007 and that's merged in so I'm gonna skip them for now.

[TylerLeonhardt]

I'm planning on factoring this out into an abstract class or interface... just haven't gotten around to it.