Skip to content

Commit

Permalink
more predictable recent files sorting (#10690, #20546)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bpasero
bpasero committed Oct 4, 2017
1 parent d869040 commit 7a2b6fd
Show file tree
Hide file tree
Showing 10 changed files with 215 additions and 229 deletions.
2 changes: 1 addition & 1 deletion src/vs/base/common/OSSREADME.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"version": "0.1.20",
"license": "MIT",
"repositoryURL": "https://github.com/joshaven/string_score",
"description": "The file scorer.ts was inspired by the string_score algorithm from Joshaven Potter.",
"description": "The file quickOpenScorer.ts was inspired by the string_score algorithm from Joshaven Potter.",
"licenseDetail": [
"This software is released under the MIT license:",
"",
Expand Down
4 changes: 2 additions & 2 deletions src/vs/base/common/filters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function isLower(code: number): boolean {
return CharCode.a <= code && code <= CharCode.z;
}

function isUpper(code: number): boolean {
export function isUpper(code: number): boolean {
return CharCode.A <= code && code <= CharCode.Z;
}

Expand Down Expand Up @@ -421,7 +421,7 @@ function printTable(table: number[][], pattern: string, patternLen: number, word
return ret;
}

function isSeparatorAtPos(value: string, index: number): boolean {
export function isSeparatorAtPos(value: string, index: number): boolean {
if (index < 0 || index >= value.length) {
return false;
}
Expand Down
145 changes: 35 additions & 110 deletions src/vs/base/parts/quickopen/browser/quickOpenModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ import { TPromise } from 'vs/base/common/winjs.base';
import types = require('vs/base/common/types');
import URI from 'vs/base/common/uri';
import { ITree, IActionProvider } from 'vs/base/parts/tree/browser/tree';
import filters = require('vs/base/common/filters');
import strings = require('vs/base/common/strings');
import paths = require('vs/base/common/paths');
import { IconLabel, IIconLabelOptions } from 'vs/base/browser/ui/iconLabel/iconLabel';
import { IQuickNavigateConfiguration, IModel, IDataSource, IFilter, IAccessiblityProvider, IRenderer, IRunner, Mode } from 'vs/base/parts/quickopen/common/quickOpen';
import { Action, IAction, IActionRunner } from 'vs/base/common/actions';
Expand All @@ -24,7 +21,7 @@ import { IQuickOpenStyles } from 'vs/base/parts/quickopen/browser/quickOpenWidge
import { KeybindingLabel } from 'vs/base/browser/ui/keybindingLabel/keybindingLabel';
import { OS } from 'vs/base/common/platform';
import { ResolvedKeybinding } from 'vs/base/common/keyCodes';
import { IItemAccessor } from 'vs/base/common/scorer';
import { IItemAccessor } from 'vs/base/parts/quickopen/common/quickOpenScorer';

export interface IContext {
event: any;
Expand Down Expand Up @@ -174,112 +171,6 @@ export class QuickOpenEntry {
return false;
}

/**
* A good default sort implementation for quick open entries respecting highlight information
* as well as associated resources.
*/
public static compare(elementA: QuickOpenEntry, elementB: QuickOpenEntry, lookFor: string): number {

// Give matches with label highlights higher priority over
// those with only description highlights
const labelHighlightsA = elementA.getHighlights()[0] || [];
const labelHighlightsB = elementB.getHighlights()[0] || [];
if (labelHighlightsA.length && !labelHighlightsB.length) {
return -1;
} else if (!labelHighlightsA.length && labelHighlightsB.length) {
return 1;
}

// Fallback to the full path if labels are identical and we have associated resources
let nameA = elementA.getLabel();
let nameB = elementB.getLabel();
if (nameA === nameB) {
const resourceA = elementA.getResource();
const resourceB = elementB.getResource();

if (resourceA && resourceB) {
nameA = resourceA.fsPath;
nameB = resourceB.fsPath;
}
}

return compareAnything(nameA, nameB, lookFor);
}

/**
* A good default highlight implementation for an entry with label and description.
*/
public static highlight(entry: QuickOpenEntry, lookFor: string, fuzzyHighlight = false): { labelHighlights: IHighlight[], descriptionHighlights: IHighlight[] } {
let labelHighlights: IHighlight[] = [];
const descriptionHighlights: IHighlight[] = [];

const normalizedLookFor = strings.stripWildcards(lookFor);
const label = entry.getLabel();
const description = entry.getDescription();

// Highlight file aware
if (entry.getResource()) {

// Highlight entire label and description if searching for full absolute path
const fsPath = entry.getResource().fsPath;
if (lookFor.length === fsPath.length && lookFor.toLowerCase() === fsPath.toLowerCase()) {
labelHighlights.push({ start: 0, end: label.length });
descriptionHighlights.push({ start: 0, end: description.length });
}

// Fuzzy/Full-Path: Highlight is special
else if (fuzzyHighlight || lookFor.indexOf(paths.nativeSep) >= 0) {
const candidateLabelHighlights = filters.matchesFuzzy(lookFor, label, fuzzyHighlight);
if (!candidateLabelHighlights) {
const pathPrefix = description ? (description + paths.nativeSep) : '';
const pathPrefixLength = pathPrefix.length;

// If there are no highlights in the label, build a path out of description and highlight and match on both,
// then extract the individual label and description highlights back to the original positions
let pathHighlights = filters.matchesFuzzy(lookFor, pathPrefix + label, fuzzyHighlight);
if (!pathHighlights && lookFor !== normalizedLookFor) {
pathHighlights = filters.matchesFuzzy(normalizedLookFor, pathPrefix + label, fuzzyHighlight);
}

if (pathHighlights) {
pathHighlights.forEach(h => {

// Match overlaps label and description part, we need to split it up
if (h.start < pathPrefixLength && h.end > pathPrefixLength) {
labelHighlights.push({ start: 0, end: h.end - pathPrefixLength });
descriptionHighlights.push({ start: h.start, end: pathPrefixLength });
}

// Match on label part
else if (h.start >= pathPrefixLength) {
labelHighlights.push({ start: h.start - pathPrefixLength, end: h.end - pathPrefixLength });
}

// Match on description part
else {
descriptionHighlights.push(h);
}
});
}
} else {
labelHighlights = candidateLabelHighlights;
}
}

// Highlight only inside label
else {
labelHighlights = filters.matchesFuzzy(lookFor, label);
}
}

// Highlight by label otherwise
else {
labelHighlights = filters.matchesFuzzy(lookFor, label);
}

return { labelHighlights, descriptionHighlights };
}

public isFile(): boolean {
return false; // TODO@Ben debt with editor history merging
}
Expand Down Expand Up @@ -690,3 +581,37 @@ export class QuickOpenModel implements
return entry.run(mode, context);
}
}

/**
* A good default sort implementation for quick open entries respecting highlight information
* as well as associated resources.
*/
export function compareEntries(elementA: QuickOpenEntry, elementB: QuickOpenEntry, lookFor: string): number {

// Give matches with label highlights higher priority over
// those with only description highlights
const labelHighlightsA = elementA.getHighlights()[0] || [];
const labelHighlightsB = elementB.getHighlights()[0] || [];
if (labelHighlightsA.length && !labelHighlightsB.length) {
return -1;
}

if (!labelHighlightsA.length && labelHighlightsB.length) {
return 1;
}

// Fallback to the full path if labels are identical and we have associated resources
let nameA = elementA.getLabel();
let nameB = elementB.getLabel();
if (nameA === nameB) {
const resourceA = elementA.getResource();
const resourceB = elementB.getResource();

if (resourceA && resourceB) {
nameA = resourceA.fsPath;
nameB = resourceB.fsPath;
}
}

return compareAnything(nameA, nameB, lookFor);
}
64 changes: 43 additions & 21 deletions src/vs/base/common/scorer.ts → ...parts/quickopen/common/quickOpenScorer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@
'use strict';

import { compareAnything } from 'vs/base/common/comparers';
import { matchesPrefix, IMatch, createMatches, matchesCamelCase } from 'vs/base/common/filters';
import { matchesPrefix, IMatch, createMatches, matchesCamelCase, isSeparatorAtPos, isUpper } from 'vs/base/common/filters';
import { isEqual, nativeSep } from 'vs/base/common/paths';

export type Score = [number /* score */, number[] /* match positions */];
export type ScorerCache = { [key: string]: IItemScore };

const NO_SCORE: Score = [0, []];

const wordPathBoundary = ['-', '_', ' ', '/', '\\', '.'];

// Based on material from:
/*!
BEGIN THIRD PARTY
Expand Down Expand Up @@ -45,7 +43,7 @@ BEGIN THIRD PARTY
* Start of word/path bonus: 7
* Start of string bonus: 8
*/
export function _doScore(target: string, query: string, inverse?: boolean): Score {
export function _doScore(target: string, query: string, fuzzy: boolean, inverse?: boolean): Score {
if (!target || !query) {
return NO_SCORE; // return early if target or query are undefined
}
Expand All @@ -72,8 +70,34 @@ export function _doScore(target: string, query: string, inverse?: boolean): Scor
startAt = target.length - 1; // inverse: from end of target to beginning
}

// When not searching fuzzy, we require the query to be contained fully
// in the target string.
if (!fuzzy) {
let indexOfQueryInTarget: number;
if (!inverse) {
indexOfQueryInTarget = targetLower.indexOf(queryLower);
} else {
indexOfQueryInTarget = targetLower.lastIndexOf(queryLower);
}

if (indexOfQueryInTarget === -1) {
// console.log(`Characters not matching consecutively ${queryLower} within ${targetLower}`);

return NO_SCORE;
}

// Adjust the start position with the offset of the query
if (!inverse) {
startAt = indexOfQueryInTarget;
} else {
startAt = indexOfQueryInTarget + query.length;
}
}

let score = 0;
while (inverse ? index >= 0 : index < queryLen) {

// Check for query character being contained in target
let indexOf: number;
if (!inverse) {
indexOf = targetLower.indexOf(queryLower[index], startAt);
Expand All @@ -82,10 +106,9 @@ export function _doScore(target: string, query: string, inverse?: boolean): Scor
}

if (indexOf < 0) {

// console.log(`Character not part of target ${query[index]}`);

score = 0; // This makes sure that the query is contained in the target
score = 0;
break;
}

Expand Down Expand Up @@ -119,7 +142,7 @@ export function _doScore(target: string, query: string, inverse?: boolean): Scor
}

// After separator bonus
else if (wordPathBoundary.some(w => w === target[indexOf - 1])) {
else if (isSeparatorAtPos(target, indexOf - 1)) {
score += 7;

// console.log('After separtor bonus: +7');
Expand Down Expand Up @@ -156,9 +179,6 @@ export function _doScore(target: string, query: string, inverse?: boolean): Scor
return res;
}

function isUpper(code: number): boolean {
return 65 <= code && code <= 90;
}
/*!
END THIRD PARTY
*/
Expand Down Expand Up @@ -209,7 +229,7 @@ const LABEL_PREFIX_SCORE = 1 << 17;
const LABEL_CAMELCASE_SCORE = 1 << 16;
const LABEL_SCORE_THRESHOLD = 1 << 15;

export function scoreItem<T>(item: T, query: string, accessor: IItemAccessor<T>, cache: ScorerCache): IItemScore {
export function scoreItem<T>(item: T, query: string, fuzzy: boolean, accessor: IItemAccessor<T>, cache: ScorerCache): IItemScore {
if (!item || !query) {
return NO_ITEM_SCORE; // we need an item and query to score on at least
}
Expand All @@ -221,23 +241,25 @@ export function scoreItem<T>(item: T, query: string, accessor: IItemAccessor<T>,

const description = accessor.getItemDescription(item);

let cacheHash = label + query;
let cacheHash: string;
if (description) {
cacheHash += description;
cacheHash = `${label}${description}${query}${fuzzy}`;
} else {
cacheHash = `${label}${query}${fuzzy}`;
}

const cached = cache[cacheHash];
if (cached) {
return cached;
}

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

return itemScore;
}

function doScoreItem<T>(label: string, description: string, path: string, query: string, accessor: IItemAccessor<T>): IItemScore {
function doScoreItem<T>(label: string, description: string, path: string, query: string, fuzzy: boolean, accessor: IItemAccessor<T>): IItemScore {

// 1.) treat identity matches on full path highest
if (path && isEqual(query, path, true)) {
Expand All @@ -257,7 +279,7 @@ function doScoreItem<T>(label: string, description: string, path: string, query:
}

// 4.) prefer scores on the label if any
const [labelScore, labelPositions] = _doScore(label, query);
const [labelScore, labelPositions] = _doScore(label, query, fuzzy);
if (labelScore) {
return { score: labelScore + LABEL_SCORE_THRESHOLD, labelMatch: createMatches(labelPositions) };
}
Expand All @@ -272,12 +294,12 @@ function doScoreItem<T>(label: string, description: string, path: string, query:
const descriptionPrefixLength = descriptionPrefix.length;
const descriptionAndLabel = `${descriptionPrefix}${label}`;

let [labelDescriptionScore, labelDescriptionPositions] = _doScore(descriptionAndLabel, query);
let [labelDescriptionScore, labelDescriptionPositions] = _doScore(descriptionAndLabel, query, fuzzy);

// Optimize for file paths: score from the back to the beginning to catch more specific folder
// names that match on the end of the file. This yields better results in most cases.
if (!!path) {
const [labelDescriptionScoreInverse, labelDescriptionPositionsInverse] = _doScore(descriptionAndLabel, query, true /* inverse */);
const [labelDescriptionScoreInverse, labelDescriptionPositionsInverse] = _doScore(descriptionAndLabel, query, fuzzy, true /* inverse */);
if (labelDescriptionScoreInverse && labelDescriptionScoreInverse > labelDescriptionScore) {
labelDescriptionScore = labelDescriptionScoreInverse;
labelDescriptionPositions = labelDescriptionPositionsInverse;
Expand Down Expand Up @@ -316,9 +338,9 @@ function doScoreItem<T>(label: string, description: string, path: string, query:
return NO_ITEM_SCORE;
}

export function compareItemsByScore<T>(itemA: T, itemB: T, query: string, accessor: IItemAccessor<T>, cache: ScorerCache): number {
const scoreA = scoreItem(itemA, query, accessor, cache).score;
const scoreB = scoreItem(itemB, query, accessor, cache).score;
export function compareItemsByScore<T>(itemA: T, itemB: T, query: string, fuzzy: boolean, accessor: IItemAccessor<T>, cache: ScorerCache): number {
const scoreA = scoreItem(itemA, query, fuzzy, accessor, cache).score;
const scoreB = scoreItem(itemB, query, fuzzy, accessor, cache).score;

// 1.) check for identity matches
if (scoreA === PATH_IDENTITY_SCORE || scoreB === PATH_IDENTITY_SCORE) {
Expand Down
Loading

0 comments on commit 7a2b6fd

Please sign in to comment.