Skip to content

Commit

Permalink
quick access - fix regression in symbol search
Browse files Browse the repository at this point in the history
  • Loading branch information
@bpasero
bpasero committed Apr 2, 2020
1 parent 9f48b00 commit d8e3cca
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 48 deletions.
44 changes: 23 additions & 21 deletions src/vs/base/common/fuzzyScorer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function scoreFuzzy(target: string, query: string, queryLower: string, fu
}
}

const res = doScore(query, queryLower, queryLength, target, targetLower, targetLength);
const res = doScoreFuzzy(query, queryLower, queryLength, target, targetLower, targetLength);

// if (DEBUG) {
// console.log(`%cFinal Score: ${res[0]}`, 'font-weight: bold');
Expand All @@ -62,7 +62,7 @@ export function scoreFuzzy(target: string, query: string, queryLower: string, fu
return res;
}

function doScore(query: string, queryLower: string, queryLength: number, target: string, targetLower: string, targetLength: number): FuzzyScore {
function doScoreFuzzy(query: string, queryLower: string, queryLength: number, target: string, targetLower: string, targetLength: number): FuzzyScore {
const scores: number[] = [];
const matches: number[] = [];

Expand Down Expand Up @@ -264,44 +264,46 @@ function scoreSeparatorAtPos(charCode: number): number {

//#region Alternate fuzzy scorer implementation that is e.g. used for symbols

export type FuzzyScore2 = [number /* Score*/, IMatch[]];
export type FuzzyScore2 = [number /* score*/, IMatch[]];

export function scoreFuzzy2(target: string, query: IPreparedQuery, matchOffset = 0): FuzzyScore2 | undefined {
const NO_SCORE2: FuzzyScore2 = [NO_MATCH, []];

export function scoreFuzzy2(target: string, query: IPreparedQuery, patternStart = 0, matchOffset = 0): FuzzyScore2 {

// Score: multiple inputs
if (query.values && query.values.length > 1) {
return doScoreFuzzy2Multiple(target, query.values, matchOffset);
return doScoreFuzzy2Multiple(target, query.values, patternStart, matchOffset);
}

// Score: single input
return doScoreFuzzy2Single(target, query, matchOffset);
return doScoreFuzzy2Single(target, query, patternStart, matchOffset);
}

function doScoreFuzzy2Multiple(target: string, query: IPreparedQueryPiece[], matchOffset: number): FuzzyScore2 | undefined {
function doScoreFuzzy2Multiple(target: string, query: IPreparedQueryPiece[], patternStart: number, matchOffset: number): FuzzyScore2 {
let totalScore = 0;
const totalMatches: IMatch[] = [];

for (const queryPiece of query) {
const score = doScoreFuzzy2Single(target, queryPiece, matchOffset);
const [score, matches] = doScoreFuzzy2Single(target, queryPiece, patternStart, matchOffset);
if (!score) {
// if a single query value does not match, return with
// no score entirely, we require all queries to match
return undefined;
return NO_SCORE2;
}

totalScore += score[0];
totalMatches.push(...score[1]);
totalScore += score;
totalMatches.push(...matches);
}

// if we have a score, ensure that the positions are
// sorted in ascending order and distinct
return [totalScore, normalizeMatches(totalMatches)];
}

function doScoreFuzzy2Single(target: string, query: IPreparedQueryPiece, matchOffset: number): FuzzyScore2 | undefined {
const score = fuzzyScore(query.original, query.originalLowercase, 0, target, target.toLowerCase(), 0, true);
function doScoreFuzzy2Single(target: string, query: IPreparedQueryPiece, patternStart: number, matchOffset: number): FuzzyScore2 {
const score = fuzzyScore(query.original, query.originalLowercase, patternStart, target, target.toLowerCase(), 0, true);
if (!score) {
return undefined;
return NO_SCORE2;
}

return [score[0], createFuzzyMatches(score, matchOffset)];
Expand Down Expand Up @@ -382,13 +384,13 @@ export function scoreItemFuzzy<T>(item: T, query: IPreparedQuery, fuzzy: boolean
return cached;
}

const itemScore = doScoreItem(label, description, accessor.getItemPath(item), query, fuzzy);
const itemScore = doScoreItemFuzzy(label, description, accessor.getItemPath(item), query, fuzzy);
cache[cacheHash] = itemScore;

return itemScore;
}

function doScoreItem(label: string, description: string | undefined, path: string | undefined, query: IPreparedQuery, fuzzy: boolean): IItemScore {
function doScoreItemFuzzy(label: string, description: string | undefined, path: string | undefined, query: IPreparedQuery, fuzzy: boolean): IItemScore {
const preferLabelMatches = !path || !query.containsPathSeparator;

// Treat identity matches on full path highest
Expand All @@ -398,20 +400,20 @@ function doScoreItem(label: string, description: string | undefined, path: strin

// Score: multiple inputs
if (query.values && query.values.length > 1) {
return doScoreItemMultiple(label, description, path, query.values, preferLabelMatches, fuzzy);
return doScoreItemFuzzyMultiple(label, description, path, query.values, preferLabelMatches, fuzzy);
}

// Score: single input
return doScoreItemSingle(label, description, path, query, preferLabelMatches, fuzzy);
return doScoreItemFuzzySingle(label, description, path, query, preferLabelMatches, fuzzy);
}

function doScoreItemMultiple(label: string, description: string | undefined, path: string | undefined, query: IPreparedQueryPiece[], preferLabelMatches: boolean, fuzzy: boolean): IItemScore {
function doScoreItemFuzzyMultiple(label: string, description: string | undefined, path: string | undefined, query: IPreparedQueryPiece[], preferLabelMatches: boolean, fuzzy: boolean): IItemScore {
let totalScore = 0;
const totalLabelMatches: IMatch[] = [];
const totalDescriptionMatches: IMatch[] = [];

for (const queryPiece of query) {
const { score, labelMatch, descriptionMatch } = doScoreItemSingle(label, description, path, queryPiece, preferLabelMatches, fuzzy);
const { score, labelMatch, descriptionMatch } = doScoreItemFuzzySingle(label, description, path, queryPiece, preferLabelMatches, fuzzy);
if (score === NO_MATCH) {
// if a single query value does not match, return with
// no score entirely, we require all queries to match
Expand All @@ -437,7 +439,7 @@ function doScoreItemMultiple(label: string, description: string | undefined, pat
};
}

function doScoreItemSingle(label: string, description: string | undefined, path: string | undefined, query: IPreparedQueryPiece, preferLabelMatches: boolean, fuzzy: boolean): IItemScore {
function doScoreItemFuzzySingle(label: string, description: string | undefined, path: string | undefined, query: IPreparedQueryPiece, preferLabelMatches: boolean, fuzzy: boolean): IItemScore {

// Prefer label matches if told so
if (preferLabelMatches) {
Expand Down
18 changes: 9 additions & 9 deletions src/vs/base/test/common/fuzzyScorer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,10 @@ function _doScore(target: string, query: string, fuzzy: boolean): scorer.FuzzySc
return scorer.scoreFuzzy(target, preparedQuery.normalized, preparedQuery.normalizedLowercase, fuzzy);
}

function _doScore2(target: string, query: string): scorer.FuzzyScore2 | [undefined, undefined] {
function _doScore2(target: string, query: string): scorer.FuzzyScore2 {
const preparedQuery = scorer.prepareQuery(query);

const res = scorer.scoreFuzzy2(target, preparedQuery);
if (res) {
return res;
}

return [undefined, undefined];
return scorer.scoreFuzzy2(target, preparedQuery);
}

function scoreItem<T>(item: T, query: string, fuzzy: boolean, accessor: scorer.IItemAccessor<T>, cache: scorer.FuzzyScorerCache): scorer.IItemScore {
Expand Down Expand Up @@ -898,6 +893,11 @@ suite('Fuzzy Scorer', () => {
assert.equal(query.values?.[1].normalized, 'World');
assert.equal(query.values?.[1].normalizedLowercase, 'World'.toLowerCase());

let restoredQuery = scorer.pieceToQuery(query.values!);
assert.equal(restoredQuery.original, query.original);
assert.equal(restoredQuery.values?.length, query.values?.length);
assert.equal(restoredQuery.containsPathSeparator, query.containsPathSeparator);

// with spaces that are empty
query = scorer.prepareQuery(' Hello World ');
assert.equal(query.original, ' Hello World ');
Expand Down Expand Up @@ -957,8 +957,8 @@ suite('Fuzzy Scorer', () => {
}

function assertNoScore() {
assert.equal(multiScore, undefined);
assert.equal(multiMatches, undefined);
assert.equal(multiScore, 0);
assert.equal(multiMatches.length, 0);
}

assertScore();
Expand Down
22 changes: 13 additions & 9 deletions src/vs/editor/contrib/quickAccess/gotoSymbolQuickAccess.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { DocumentSymbol, SymbolKinds, SymbolTag, DocumentSymbolProviderRegistry,
import { OutlineModel, OutlineElement } from 'vs/editor/contrib/documentSymbols/outlineModel';
import { values } from 'vs/base/common/collections';
import { trim, format } from 'vs/base/common/strings';
import { prepareQuery, IPreparedQuery, FuzzyScore2, pieceToQuery, scoreFuzzy2 } from 'vs/base/common/fuzzyScorer';
import { prepareQuery, IPreparedQuery, pieceToQuery, scoreFuzzy2 } from 'vs/base/common/fuzzyScorer';
import { IMatch } from 'vs/base/common/filters';

export interface IGotoSymbolQuickPickItem extends IQuickPickItem {
kind: SymbolKind,
Expand Down Expand Up @@ -229,29 +230,32 @@ export abstract class AbstractGotoSymbolQuickAccessProvider extends AbstractEdit
}
}

let symbolScore: FuzzyScore2 | undefined = undefined;
let containerScore: FuzzyScore2 | undefined = undefined;
let symbolScore: number | undefined = undefined;
let symbolMatches: IMatch[] | undefined = undefined;

let containerScore: number | undefined = undefined;
let containerMatches: IMatch[] | undefined = undefined;

if (query.original.length > filterPos) {

// Score by symbol
symbolScore = scoreFuzzy2(symbolLabel, symbolQuery, symbolLabelWithIcon.length - symbolLabel.length /* Readjust matches to account for codicons in label */);
[symbolScore, symbolMatches] = scoreFuzzy2(symbolLabel, symbolQuery, filterPos, symbolLabelWithIcon.length - symbolLabel.length /* Readjust matches to account for codicons in label */);
if (!symbolScore) {
continue;
}

// Score by container if specified
if (containerQuery) {
if (containerLabel && containerQuery.original.length > 0) {
containerScore = scoreFuzzy2(containerLabel, containerQuery);
[containerScore, containerMatches] = scoreFuzzy2(containerLabel, containerQuery);
}

if (!containerScore) {
continue;
}

if (symbolScore) {
symbolScore[0] += containerScore[0]; // boost symbolScore by containerScore
symbolScore += containerScore; // boost symbolScore by containerScore
}
}
}
Expand All @@ -261,13 +265,13 @@ export abstract class AbstractGotoSymbolQuickAccessProvider extends AbstractEdit
filteredSymbolPicks.push({
index,
kind: symbol.kind,
score: symbolScore?.[0],
score: symbolScore,
label: symbolLabelWithIcon,
ariaLabel: symbolLabel,
description: containerLabel,
highlights: deprecated ? undefined : {
label: symbolScore?.[1],
description: containerScore?.[1]
label: symbolMatches,
description: containerMatches
},
range: {
selection: Range.collapseToStart(symbol.selectionRange),
Expand Down
22 changes: 13 additions & 9 deletions src/vs/workbench/contrib/search/browser/symbolsQuickAccess.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import { URI } from 'vs/base/common/uri';
import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
import { getSelectionSearchString } from 'vs/editor/contrib/find/findController';
import { withNullAsUndefined } from 'vs/base/common/types';
import { prepareQuery, IPreparedQuery, scoreFuzzy2, FuzzyScore2, pieceToQuery } from 'vs/base/common/fuzzyScorer';
import { prepareQuery, IPreparedQuery, scoreFuzzy2, pieceToQuery } from 'vs/base/common/fuzzyScorer';
import { IMatch } from 'vs/base/common/filters';

interface ISymbolQuickPickItem extends IPickerQuickAccessItem {
resource: URI | undefined;
Expand Down Expand Up @@ -127,10 +128,12 @@ export class SymbolsQuickAccessProvider extends PickerQuickAccessProvider<ISymbo
const symbolLabel = symbol.name;
const symbolLabelWithIcon = `$(symbol-${SymbolKinds.toString(symbol.kind) || 'property'}) ${symbolLabel}`;


// Score by symbol label if searching
let symbolScore: FuzzyScore2 | undefined;
let symbolScore: number | undefined = undefined;
let symbolMatches: IMatch[] | undefined = undefined;
if (symbolQuery.original.length > 0) {
symbolScore = scoreFuzzy2(symbolLabel, symbolQuery, symbolLabelWithIcon.length - symbolLabel.length /* Readjust matches to account for codicons in label */);
[symbolScore, symbolMatches] = scoreFuzzy2(symbolLabel, symbolQuery, 0, symbolLabelWithIcon.length - symbolLabel.length /* Readjust matches to account for codicons in label */);
if (!symbolScore) {
continue;
}
Expand All @@ -148,18 +151,19 @@ export class SymbolsQuickAccessProvider extends PickerQuickAccessProvider<ISymbo
}

// Score by container if specified and searching
let containerScore: FuzzyScore2 | undefined = undefined;
let containerScore: number | undefined = undefined;
let containerMatches: IMatch[] | undefined = undefined;
if (containerQuery && containerQuery.original.length > 0) {
if (containerLabel) {
containerScore = scoreFuzzy2(containerLabel, containerQuery);
[containerScore, containerMatches] = scoreFuzzy2(containerLabel, containerQuery);
}

if (!containerScore) {
continue;
}

if (symbolScore) {
symbolScore[0] += containerScore[0]; // boost symbolScore by containerScore
symbolScore += containerScore; // boost symbolScore by containerScore
}
}

Expand All @@ -168,12 +172,12 @@ export class SymbolsQuickAccessProvider extends PickerQuickAccessProvider<ISymbo
symbolPicks.push({
symbol,
resource: symbolUri,
score: symbolScore?.[0],
score: symbolScore,
label: symbolLabelWithIcon,
ariaLabel: symbolLabel,
highlights: deprecated ? undefined : {
label: symbolScore?.[1],
description: containerScore?.[1]
label: symbolMatches,
description: containerMatches
},
description: containerLabel,
strikethrough: deprecated,
Expand Down

0 comments on commit d8e3cca

Please sign in to comment.