From 0d5ce5e579935cb326ba3c7d35ea77a0ed07d04c Mon Sep 17 00:00:00 2001 From: David Feltell Date: Mon, 15 Oct 2018 23:46:08 +0100 Subject: [PATCH 1/2] De-deduplicate completion items + improve sorting * Remove completion items that are duplicates for all attributes other than `sortText` (which is always different but is just some machine-readable hash for sorting by). * Continue to sort abbreviations below full matches. * Sort matching `filterText`/`labels` so they appear next to each other. * Otherwise use `sortText` given by the language server. --- .../Completion/CompletionSelectors.ts | 65 ++++++++++++++++--- .../Completion/CompletionSelectorsTests.ts | 60 +++++++++++++++++ 2 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 browser/test/Services/Completion/CompletionSelectorsTests.ts diff --git a/browser/src/Services/Completion/CompletionSelectors.ts b/browser/src/Services/Completion/CompletionSelectors.ts index 127c46efcc..8a5ee2cc6f 100644 --- a/browser/src/Services/Completion/CompletionSelectors.ts +++ b/browser/src/Services/Completion/CompletionSelectors.ts @@ -3,6 +3,7 @@ * * Selectors are functions that take a state and derive a value from it. */ +import * as _ from "lodash" import { ICompletionState } from "./CompletionState" @@ -67,25 +68,69 @@ export const filterCompletionOptions = ( if (!searchText) { return items } - if (!items || !items.length) { return null } + // Must start with first letter in searchText, and then be at least abbreviated by searchText. const filterRegEx = new RegExp("^" + searchText.split("").join(".*") + ".*") - const filteredOptions = items.filter(f => { const textToFilterOn = f.filterText || f.label return textToFilterOn.match(filterRegEx) }) + const sortedOptions = filteredOptions.sort((itemA, itemB) => { + const itemASortText = itemA.filterText || itemA.label + const itemBSortText = itemB.filterText || itemB.label + + const indexOfA = itemASortText.indexOf(searchText) + const indexOfB = itemBSortText.indexOf(searchText) + + // Ensure abbreviated matches are sorted below exact matches. + if (indexOfA >= 0 && indexOfB === -1) { + return -1 + } else if (indexOfA === -1 && indexOfB >= 0) { + return 1 + // Else sort by label to keep related results together. + } else if (itemASortText < itemBSortText) { + return -1 + } else if (itemASortText > itemBSortText) { + return 1 + // Fallback to sort by language server specified sortText. + } else if (itemA.sortText < itemB.sortText) { + return -1 + } else if (itemA.sortText > itemB.sortText) { + return 1 + } + return 0 + }) + // Language servers can return duplicate entries (e.g. cquery). + const uniqueOptions: types.CompletionItem[] = _uniq(sortedOptions) - return filteredOptions.sort((itemA, itemB) => { - const itemAFilterText = itemA.filterText || itemA.label - const itemBFilterText = itemB.filterText || itemB.label - - const indexOfA = itemAFilterText.indexOf(searchText) - const indexOfB = itemBFilterText.indexOf(searchText) + return uniqueOptions +} - return indexOfB - indexOfA - }) +/** + * Get unique completion items, assuming they're sorted so duplicates are contiguous. + * + * Adapted from https://github.com/lodash/lodash/blob/master/.internal/baseSortedUniq.js, since + * lodash has no `sortedUniqWith` function. + */ +const _uniq = (array: types.CompletionItem[]) => { + let seenReduced: any + let index = -1 + let resIndex = 0 + + const { length } = array + const result = [] + + while (++index < length) { + const value = array[index] + const reduced = _.omit(value, "sortText") + // Omit the `sortText` which can be different even if all other attributes are the same. + if (!index || !_.isEqual(reduced, seenReduced)) { + seenReduced = reduced + result[resIndex++] = value + } + } + return result } diff --git a/browser/test/Services/Completion/CompletionSelectorsTests.ts b/browser/test/Services/Completion/CompletionSelectorsTests.ts new file mode 100644 index 0000000000..67d0a2f6bf --- /dev/null +++ b/browser/test/Services/Completion/CompletionSelectorsTests.ts @@ -0,0 +1,60 @@ +import * as assert from "assert" +import { CompletionItem } from "vscode-languageserver-types" + +import { filterCompletionOptions } from "../../../src/Services/Completion/CompletionSelectors" + +describe("filterCompletionOptions", () => { + it("strips duplicates and sorts in order of abbreviation=>label=>sortText", () => { + const item1: CompletionItem = { + label: "mock duplicate", + detail: "mock detail", + sortText: "c", + } + const item2: CompletionItem = { + label: "mock duplicate", + detail: "mock detail", + sortText: "b", + } + const item3: CompletionItem = { + label: "mock duplicate", + detail: "mock not duplicate detail", + sortText: "a", + } + const item4: CompletionItem = { + label: "maaaaoaaaacaaaak abbreviation", + } + const item5: CompletionItem = { + label: "mock cherry", + filterText: "mock cherry", + } + const item6: CompletionItem = { + label: "mock cherry", + filterText: "mock banana", + } + const item7: CompletionItem = { + label: "mock apple", + } + const item8: CompletionItem = { + label: "doesnt match", + } + const item9: CompletionItem = { + label: "mock apple", + filterText: "doesnt match either", + } + const items: CompletionItem[] = [ + item1, + item2, + item3, + item4, + item5, + item6, + item7, + item8, + item9, + ] + + const filteredItems = filterCompletionOptions(items, "mock") + + assert.deepStrictEqual(filteredItems, [item7, item6, item5, item3, item2, item4]) + }) +}) From 4ef58f932a3e2d1d270b5fec42626afc337bc6d9 Mon Sep 17 00:00:00 2001 From: David Feltell Date: Wed, 17 Oct 2018 23:23:45 +0100 Subject: [PATCH 2/2] Don't import all of lodash just for a couple of functions --- browser/src/Services/Completion/CompletionSelectors.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/browser/src/Services/Completion/CompletionSelectors.ts b/browser/src/Services/Completion/CompletionSelectors.ts index 8a5ee2cc6f..5b03bbc46d 100644 --- a/browser/src/Services/Completion/CompletionSelectors.ts +++ b/browser/src/Services/Completion/CompletionSelectors.ts @@ -3,7 +3,8 @@ * * Selectors are functions that take a state and derive a value from it. */ -import * as _ from "lodash" +import * as isEqual from "lodash/isEqual" +import * as omit from "lodash/omit" import { ICompletionState } from "./CompletionState" @@ -125,9 +126,9 @@ const _uniq = (array: types.CompletionItem[]) => { while (++index < length) { const value = array[index] - const reduced = _.omit(value, "sortText") // Omit the `sortText` which can be different even if all other attributes are the same. - if (!index || !_.isEqual(reduced, seenReduced)) { + const reduced = omit(value, "sortText") + if (!index || !isEqual(reduced, seenReduced)) { seenReduced = reduced result[resIndex++] = value }