Improve suggestion results

[cdce8p]

A first attempt to improve the suggestion list. See: microsoft/pylance-release#608

Instead of the Levenshtein Distance I though that Longest Common Subsequence might work better. The only drawback being that that user must only enter chars that appear in the symbol he/she is looking for and they must be in the correct order.

An example result would look like this:

Please let me know if that's something worth exploring further.

--
Algorithm used: Similar to isPatternInWord from VS Code

[ghost]

All CLA requirements met.

[erictraut]

We've received feedback from many other pyright/pylance users that they don't want to be restricted to only characters that appear within the symbol, so I think we'd receive a significant backlash if we went with this approach. The approach needs to accommodate small typos (different order, different capitalization, small deviations in characters).

[cdce8p]

Capitalization is already taken care of, since it compares all lower case strings.
Regarding the other points: Maybe a combined approach would work. I'll think about it.

[heejaechang]

returning result that doesn't contain given pattern will be filtered out by vscode so regardless whether LS returns them user will never be able to see those?

you can try that by enabling LSP logging and see what shows up in completion. (microsoft/pylance-release#362 (comment))

by the way, I think we don't need to do anything more than what vscode recommended (https://github.com/microsoft/vscode/blob/21dc66054203ab742d36be9f0ef6ecb774ae62f2/src/vs/base/common/filters.ts#L506-L514) for vscode since it does its own filter over data returned.

not sure about other editors that support LSP such as atom, vim-lsp and etc. for those, we might use different algorithm?

[cdce8p]

I tried a few things and basically observed what @heejaechang mentioned already. VSC does exact pattern matching for the typed string. Setting "editor.suggest.filterGraceful": true (the default) allows for probably one permutation but that's it. Other results are filtered although Pyright provided them (using Levenshtein Distance). Additionally if the user provided chars that don't appear in the symbol, it is also filtered.

Having said thar, I don't see much value in sticking with the current approach. Then again it's not my decision to make.
We could also use a combined method which I suggested previously. However this would probably come with at least a small performance hit, depending on the project size.

[erictraut]

Yeah, if that's the case, then I agree that sticking with the current approach doesn't provide much value.

[cdce8p]

I've change the algorithm to the one @heejaechang suggested and VS Code uses: isPatternInWord (Link). Before that I also did some tests with a recursive one to allow for permutations. I'll include it below, however I'm not sure if it's worth it.

export function computeCompletionSimilarity(typedValue: string, symbolName: string): number {
    if (recurseSimilarity(typedValue.toLocaleLowerCase(), symbolName.toLocaleLowerCase())) {
        return 1;
    }
    return 0;
}

function recurseSimilarity(typedLower: string, symbolLower: string): boolean {
    if (typedLower.length === 0) {
        return true;
    }
    const index = symbolLower.indexOf(typedLower[0]);
    if (index === -1) {
        return false;
    }
    if (typedLower.length === 1) {
        return true;
    } else if (typedLower.length === 2) {
        symbolLower = symbolLower.slice(index + 1);
        return recurseSimilarity(typedLower.slice(1), symbolLower);
    } else {
        symbolLower = symbolLower.slice(index + 1);
        return (
            recurseSimilarity(typedLower.slice(1), symbolLower) ||
            recurseSimilarity(typedLower[2] + typedLower[1] + typedLower.slice(3), symbolLower)
        );
    }
}

[heejaechang]

I agree that we don't do permutation for now. for me, code LGTM, but I will leave it to @erictraut to decide.

[cdce8p]

I've pushed a new commit with all requested changes, see commit description.
Should I remove all references to similarityLimit, now that it isn't used anymore?

[jakebailey]

similarityLimit appears to be a local in a bunch of modules, I think, so would be safe to remove if truly not used. It's the exported code that we wouldn't want to remove (given we do reference things from pylance).

[cdce8p]

@jakebailey Should it remain in function signatures of exported classes?

[jakebailey]

Oh, my mistake. I see, it's a parameter. I would just leave it and we can perform the cleanup later as it's a part of the API.

[heejaechang]

it looks good to me? if there is no objection, I can accept the PR.

[heejaechang]

@cdce8p we will take the PR soon. but probably for the next release (Pylance) due to some change we need to make for add import code actions.

[cdce8p]

@heejaechang Thanks for letting me know. Looking forward to using it when it's released!

[jakebailey]

Thanks for this; it's not likely to come out in Pylance next week (holiday week), but should make it afterward. 🙂