Skip to content

Commit

Permalink
Fix sorting problem in QuickFileOpenService (#10694)
Browse files Browse the repository at this point in the history
  • Loading branch information
colin-grant-work authored Feb 3, 2022
1 parent 5ba2d9a commit 2faa18b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 31 deletions.
26 changes: 21 additions & 5 deletions examples/api-tests/src/file-search.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ describe('file-search', function () {

describe('#compareItems', () => {

/** @type import ('@theia/file-search/lib/browser/quick-file-open').QuickFileOpenService['compareItems']*/
const sortByCompareItems = (a, b) => quickFileOpenService['compareItems'](a, b);

it('should compare two quick-open-items by `label`', () => {

/** @type import ('@theia/file-search/lib/browser/quick-file-open').FileQuickPickItem*/
const a = { label: 'a', uri: new Uri.default('b') };
/** @type import ('@theia/file-search/lib/browser/quick-file-open').FileQuickPickItem*/
const b = { label: 'b', uri: new Uri.default('a') };

assert.equal(quickFileOpenService['compareItems'](a, b), 1, 'a should be before b');
assert.equal(quickFileOpenService['compareItems'](b, a), -1, 'a should be before b');
assert.deepEqual([a, b].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.deepEqual([b, a].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.equal(quickFileOpenService['compareItems'](a, a), 0, 'items should be equal');
});

Expand All @@ -49,10 +52,23 @@ describe('file-search', function () {
const a = { label: 'a', uri: new Uri.default('a') };
/** @type import ('@theia/file-search/lib/browser/quick-file-open').FileQuickPickItem*/
const b = { label: 'a', uri: new Uri.default('b') };
assert.deepEqual([a, b].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.deepEqual([b, a].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.equal(sortByCompareItems(a, a), 0, 'items should be equal');
});

assert.equal(quickFileOpenService['compareItems'](a, b), 1, 'a should be before b');
assert.equal(quickFileOpenService['compareItems'](b, a), -1, 'a should be before b');
assert.equal(quickFileOpenService['compareItems'](a, a), 0, 'items should be equal');
it('should not place very good matches above exact matches', () => {
const exactMatch = 'aa_bb_cc_dd.file';
const veryGoodMatch = 'aa_bb_cc_ed_fd.file';
quickFileOpenService['filterAndRange'] = { filter: exactMatch };
/** @type import ('@theia/file-search/lib/browser/quick-file-open').FileQuickPickItem*/
const a = { label: exactMatch, uri: new Uri.default(exactMatch) };
/** @type import ('@theia/file-search/lib/browser/quick-file-open').FileQuickPickItem*/
const b = { label: veryGoodMatch, uri: new Uri.default(veryGoodMatch) };
assert.deepEqual([a, b].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.deepEqual([b, a].sort(sortByCompareItems), [a, b], 'a should be before b');
assert.equal(sortByCompareItems(a, a), 0, 'items should be equal');
quickFileOpenService['filterAndRange'] = quickFileOpenService['filterAndRangeDefault'];
});

});
Expand Down
41 changes: 15 additions & 26 deletions packages/file-search/src/browser/quick-file-open.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { OpenerService, KeybindingRegistry, QuickAccessRegistry, QuickAccessProv
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import URI from '@theia/core/lib/common/uri';
import { FileSearchService, WHITESPACE_QUERY_SEPARATOR } from '../common/file-search-service';
import { CancellationToken, Command } from '@theia/core/lib/common';
import { CancellationToken, Command, MAX_SAFE_INTEGER } from '@theia/core/lib/common';
import { LabelProvider } from '@theia/core/lib/browser/label-provider';
import { NavigationLocationService } from '@theia/editor/lib/browser/navigation/navigation-location-service';
import * as fuzzy from '@theia/core/shared/fuzzy';
Expand Down Expand Up @@ -198,7 +198,7 @@ export class QuickFileOpenService implements QuickAccessProvider {
sortedResults.sort((a, b) => this.compareItems(a, b));

if (sortedResults.length > 0) {
result.unshift({ type: 'separator', label: 'file results' });
result.push({ type: 'separator', label: 'file results' });
result.push(...sortedResults);
}

Expand All @@ -219,6 +219,7 @@ export class QuickFileOpenService implements QuickAccessProvider {
return roots.length !== 0 ? recentlyUsedItems : [];
}
}

protected compareItems(
left: FileQuickPickItem,
right: FileQuickPickItem): number {
Expand All @@ -233,47 +234,35 @@ export class QuickFileOpenService implements QuickAccessProvider {
if (!str) {
return 0;
}
// Adjust for whitespaces in the query.
const querySplit = query.split(WHITESPACE_QUERY_SEPARATOR);
const queryJoin = querySplit.join('');

// Check exact and partial exact matches.
let exactMatch = true;
let partialMatch = false;
querySplit.forEach(part => {
const partialMatches = querySplit.reduce((matched, part) => {
const partMatches = str.includes(part);
exactMatch = exactMatch && partMatches;
partialMatch = partialMatch || partMatches;
});
return partMatches ? matched + QuickFileOpenService.Scores.partial : matched;
}, 0);

// Check fuzzy matches.
const fuzzyMatch = fuzzy.match(queryJoin, str);
let matchScore = 0;
// eslint-disable-next-line no-null/no-null
if (!!fuzzyMatch && matchScore !== null) {
matchScore = (fuzzyMatch.score === Infinity) ? QuickFileOpenService.Scores.max : fuzzyMatch.score;
const fuzzyMatch = fuzzy.match(queryJoin, str) ?? { score: 0 };
if (fuzzyMatch.score === Infinity && exactMatch) {
return MAX_SAFE_INTEGER;
}

// Prioritize exact matches, then partial exact matches, then fuzzy matches.
if (exactMatch) {
return matchScore + QuickFileOpenService.Scores.exact;
} else if (partialMatch) {
return matchScore + QuickFileOpenService.Scores.partial;
} else {
// eslint-disable-next-line no-null/no-null
return (fuzzyMatch === null) ? 0 : matchScore;
}
return fuzzyMatch.score + partialMatches + (exactMatch ? QuickFileOpenService.Scores.exact : 0);
}

const query: string = normalize(this.filterAndRange.filter);
// Adjust for whitespaces in the query.
const querySplit = query.split(WHITESPACE_QUERY_SEPARATOR);
const queryJoin = querySplit.join('');

const compareByLabelScore = (l: FileQuickPickItem, r: FileQuickPickItem) => score(r.label) - score(l.label);
const compareByLabelIndex = (l: FileQuickPickItem, r: FileQuickPickItem) => r.label.indexOf(query) - l.label.indexOf(query);
const compareByLabel = (l: FileQuickPickItem, r: FileQuickPickItem) => r.label.localeCompare(l.label);
const compareByLabel = (l: FileQuickPickItem, r: FileQuickPickItem) => l.label.localeCompare(r.label);

const compareByPathScore = (l: FileQuickPickItem, r: FileQuickPickItem) => score(r.uri.path.toString()) - score(l.uri.path.toString());
const compareByPathIndex = (l: FileQuickPickItem, r: FileQuickPickItem) => r.uri.path.toString().indexOf(query) - l.uri.path.toString().indexOf(query);
const compareByPathLabel = (l: FileQuickPickItem, r: FileQuickPickItem) => r.uri.path.toString().localeCompare(l.uri.path.toString());
const compareByPathLabel = (l: FileQuickPickItem, r: FileQuickPickItem) => l.uri.path.toString().localeCompare(r.uri.path.toString());

return compareWithDiscriminators(left, right, compareByLabelScore, compareByLabelIndex, compareByLabel, compareByPathScore, compareByPathIndex, compareByPathLabel);
}
Expand Down

0 comments on commit 2faa18b

Please sign in to comment.