From 7b707406f4dfe7bd6bf800a18c98f32fd5516520 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 8 Feb 2018 10:41:38 +0100 Subject: [PATCH] :lipstick: octicon handling in quick open --- src/vs/base/common/octicon.ts | 46 +++++++------- src/vs/base/test/common/filters.test.ts | 52 +-------------- src/vs/base/test/common/octicon.test.ts | 63 +++++++++++++++++++ .../parts/quickopen/quickOpenController.ts | 30 +++++++-- 4 files changed, 113 insertions(+), 78 deletions(-) create mode 100644 src/vs/base/test/common/octicon.test.ts diff --git a/src/vs/base/common/octicon.ts b/src/vs/base/common/octicon.ts index 2b446e55594..42358caa50d 100644 --- a/src/vs/base/common/octicon.ts +++ b/src/vs/base/common/octicon.ts @@ -10,22 +10,27 @@ import { ltrim } from 'vs/base/common/strings'; const octiconStartMarker = '$('; -export function removeOcticons(word: string): string { - const firstOcticonIndex = word.indexOf(octiconStartMarker); +export interface IParsedOcticons { + text: string; + octiconOffsets?: number[]; +} + +export function parseOcticons(text: string): IParsedOcticons { + const firstOcticonIndex = text.indexOf(octiconStartMarker); if (firstOcticonIndex === -1) { - return word; // return early if the word does not include an octicon + return { text }; // return early if the word does not include an octicon } - return doParseOcticonAware(word, firstOcticonIndex).wordWithoutOcticons.trim(); + return doParseOcticons(text, firstOcticonIndex); } -function doParseOcticonAware(word: string, firstOcticonIndex: number): { wordWithoutOcticons: string, octiconOffsets: number[] } { +function doParseOcticons(text: string, firstOcticonIndex: number): IParsedOcticons { const octiconOffsets: number[] = []; - let wordWithoutOcticons: string = ''; + let textWithoutOcticons: string = ''; function appendChars(chars: string) { if (chars) { - wordWithoutOcticons += chars; + textWithoutOcticons += chars; for (let i = 0; i < chars.length; i++) { octiconOffsets.push(octiconsOffset); // make sure to fill in octicon offsets @@ -41,15 +46,15 @@ function doParseOcticonAware(word: string, firstOcticonIndex: number): { wordWit let nextChar: string; let offset = firstOcticonIndex; - const length = word.length; + const length = text.length; // Append all characters until the first octicon - appendChars(word.substr(0, firstOcticonIndex)); + appendChars(text.substr(0, firstOcticonIndex)); // example: $(file-symlink-file) my cool $(other-octicon) entry while (offset < length) { - char = word[offset]; - nextChar = word[offset + 1]; + char = text[offset]; + nextChar = text[offset + 1]; // beginning of octicon: some value $( <-- if (char === octiconStartMarker[0] && nextChar === octiconStartMarker[1]) { @@ -91,27 +96,24 @@ function doParseOcticonAware(word: string, firstOcticonIndex: number): { wordWit // so we have to add it to the actual value appendChars(currentOcticonValue); - return { wordWithoutOcticons, octiconOffsets }; + return { text: textWithoutOcticons, octiconOffsets }; } -export function matchesFuzzyOcticonAware(word: string, wordToMatchAgainst: string, enableSeparateSubstringMatching = false): IMatch[] { +export function matchesFuzzyOcticonAware(query: string, target: IParsedOcticons, enableSeparateSubstringMatching = false): IMatch[] { + const { text, octiconOffsets } = target; // Return early if there are no octicon markers in the word to match against - const firstOcticonIndex = wordToMatchAgainst.indexOf(octiconStartMarker); - if (firstOcticonIndex === -1) { - return matchesFuzzy(word, wordToMatchAgainst, enableSeparateSubstringMatching); + if (!octiconOffsets || octiconOffsets.length === 0) { + return matchesFuzzy(query, text, enableSeparateSubstringMatching); } - // Parse - const { wordWithoutOcticons, octiconOffsets } = doParseOcticonAware(wordToMatchAgainst, firstOcticonIndex); - // Trim the word to match against because it could have leading // whitespace now if the word started with an octicon - const wordToMatchAgainstWithoutOcticonsTrimmed = ltrim(wordWithoutOcticons, ' '); - const leadingWhitespaceOffset = wordWithoutOcticons.length - wordToMatchAgainstWithoutOcticonsTrimmed.length; + const wordToMatchAgainstWithoutOcticonsTrimmed = ltrim(text, ' '); + const leadingWhitespaceOffset = text.length - wordToMatchAgainstWithoutOcticonsTrimmed.length; // match on value without octicons - const matches = matchesFuzzy(word, wordToMatchAgainstWithoutOcticonsTrimmed, enableSeparateSubstringMatching); + const matches = matchesFuzzy(query, wordToMatchAgainstWithoutOcticonsTrimmed, enableSeparateSubstringMatching); // Map matches back to offsets with octicons and trimming if (matches) { diff --git a/src/vs/base/test/common/filters.test.ts b/src/vs/base/test/common/filters.test.ts index 39050254755..bc330457fa8 100644 --- a/src/vs/base/test/common/filters.test.ts +++ b/src/vs/base/test/common/filters.test.ts @@ -5,8 +5,7 @@ 'use strict'; import * as assert from 'assert'; -import { IFilter, or, matchesPrefix, matchesStrictPrefix, matchesCamelCase, matchesSubString, matchesContiguousSubString, matchesWords, fuzzyScore, IMatch, fuzzyScoreGraceful, fuzzyScoreGracefulAggressive, matchesFuzzy } from 'vs/base/common/filters'; -import { matchesFuzzyOcticonAware } from 'vs/base/common/octicon'; +import { IFilter, or, matchesPrefix, matchesStrictPrefix, matchesCamelCase, matchesSubString, matchesContiguousSubString, matchesWords, fuzzyScore, IMatch, fuzzyScoreGraceful, fuzzyScoreGracefulAggressive } from 'vs/base/common/filters'; function filterOk(filter: IFilter, word: string, wordToMatchAgainst: string, highlights?: { start: number; end: number; }[]) { let r = filter(word, wordToMatchAgainst); @@ -443,53 +442,4 @@ suite('Filters', () => { assertMatches('cno', 'co_new', '^c^o_^new', fuzzyScoreGraceful); assertMatches('cno', 'co_new', '^c^o_^new', fuzzyScoreGracefulAggressive); }); - - test('matchesFuzzzyOcticonAware', function () { - - // Camel Case - - filterOk(matchesFuzzy, 'ccr', 'CamelCaseRocks', [ - { start: 0, end: 1 }, - { start: 5, end: 6 }, - { start: 9, end: 10 } - ]); - - filterOk(matchesFuzzyOcticonAware, 'ccr', '$(octicon)CamelCaseRocks$(octicon)', [ - { start: 10, end: 11 }, - { start: 15, end: 16 }, - { start: 19, end: 20 } - ]); - - filterOk(matchesFuzzyOcticonAware, 'ccr', '$(octicon) CamelCaseRocks $(octicon)', [ - { start: 11, end: 12 }, - { start: 16, end: 17 }, - { start: 20, end: 21 } - ]); - - filterOk(matchesFuzzyOcticonAware, 'iut', '$(octicon) Indent $(octico) Using $(octic) Tpaces', [ - { start: 11, end: 12 }, - { start: 28, end: 29 }, - { start: 43, end: 44 }, - ]); - - // Prefix - - filterOk(matchesFuzzy, 'using', 'Indent Using Spaces', [ - { start: 7, end: 12 } - ]); - - filterOk(matchesFuzzyOcticonAware, 'using', '$(octicon) Indent Using Spaces', [ - { start: 18, end: 23 }, - ]); - - // Broken Octicon - - filterOk(matchesFuzzyOcticonAware, 'octicon', 'This $(octicon Indent Using Spaces', [ - { start: 7, end: 14 }, - ]); - - filterOk(matchesFuzzyOcticonAware, 'indent', 'This $octicon Indent Using Spaces', [ - { start: 14, end: 20 }, - ]); - }); }); diff --git a/src/vs/base/test/common/octicon.test.ts b/src/vs/base/test/common/octicon.test.ts new file mode 100644 index 00000000000..a8ab85f111a --- /dev/null +++ b/src/vs/base/test/common/octicon.test.ts @@ -0,0 +1,63 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +'use strict'; + +import * as assert from 'assert'; +import { IMatch } from 'vs/base/common/filters'; +import { matchesFuzzyOcticonAware, parseOcticons } from 'vs/base/common/octicon'; + +export interface IOcticonFilter { + // Returns null if word doesn't match. + (query: string, target: { text: string, octiconOffsets?: number[] }): IMatch[]; +} + +function filterOk(filter: IOcticonFilter, word: string, target: { text: string, octiconOffsets?: number[] }, highlights?: { start: number; end: number; }[]) { + let r = filter(word, target); + assert(r); + if (highlights) { + assert.deepEqual(r, highlights); + } +} + +suite('Octicon', () => { + test('matchesFuzzzyOcticonAware', function () { + + // Camel Case + + filterOk(matchesFuzzyOcticonAware, 'ccr', parseOcticons('$(octicon)CamelCaseRocks$(octicon)'), [ + { start: 10, end: 11 }, + { start: 15, end: 16 }, + { start: 19, end: 20 } + ]); + + filterOk(matchesFuzzyOcticonAware, 'ccr', parseOcticons('$(octicon) CamelCaseRocks $(octicon)'), [ + { start: 11, end: 12 }, + { start: 16, end: 17 }, + { start: 20, end: 21 } + ]); + + filterOk(matchesFuzzyOcticonAware, 'iut', parseOcticons('$(octicon) Indent $(octico) Using $(octic) Tpaces'), [ + { start: 11, end: 12 }, + { start: 28, end: 29 }, + { start: 43, end: 44 }, + ]); + + // Prefix + + filterOk(matchesFuzzyOcticonAware, 'using', parseOcticons('$(octicon) Indent Using Spaces'), [ + { start: 18, end: 23 }, + ]); + + // Broken Octicon + + filterOk(matchesFuzzyOcticonAware, 'octicon', parseOcticons('This $(octicon Indent Using Spaces'), [ + { start: 7, end: 14 }, + ]); + + filterOk(matchesFuzzyOcticonAware, 'indent', parseOcticons('This $octicon Indent Using Spaces'), [ + { start: 14, end: 20 }, + ]); + }); +}); diff --git a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts index 9d3f668fe67..0dbb76d454d 100644 --- a/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts +++ b/src/vs/workbench/browser/parts/quickopen/quickOpenController.ts @@ -54,7 +54,8 @@ import { FileKind, IFileService } from 'vs/platform/files/common/files'; import { scoreItem, ScorerCache, compareItemsByScore, prepareQuery } from 'vs/base/parts/quickopen/common/quickOpenScorer'; import { getBaseLabel } from 'vs/base/common/labels'; import { WorkbenchTree } from 'vs/platform/list/browser/listService'; -import { matchesFuzzyOcticonAware, removeOcticons } from 'vs/base/common/octicon'; +import { matchesFuzzyOcticonAware, parseOcticons, IParsedOcticons } from 'vs/base/common/octicon'; +import { IMatch } from 'vs/base/common/filters'; const HELP_PREFIX = '?'; @@ -434,9 +435,7 @@ export class QuickOpenController extends Component implements IQuickOpenService // Filter by value (since we support octicons, use octicon aware fuzzy matching) else { entries.forEach(entry => { - const labelHighlights = matchesFuzzyOcticonAware(value, entry.getLabel()); - const descriptionHighlights = options.matchOnDescription && matchesFuzzyOcticonAware(value, entry.getDescription()); - const detailHighlights = options.matchOnDetail && entry.getDetail() && matchesFuzzyOcticonAware(value, entry.getDetail()); + const { labelHighlights, descriptionHighlights, detailHighlights } = entry.matchesFuzzy(value, options); if (entry.shouldAlwaysShow() || labelHighlights || descriptionHighlights || detailHighlights) { entry.setHighlights(labelHighlights, descriptionHighlights, detailHighlights); @@ -1034,6 +1033,9 @@ class PickOpenEntry extends PlaceholderQuickOpenEntry implements IPickOpenItem { private _action: IAction; private removed: boolean; private payload: any; + private labelOcticons: IParsedOcticons; + private descriptionOcticons: IParsedOcticons; + private detailOcticons: IParsedOcticons; constructor( item: IPickOpenEntry, @@ -1048,7 +1050,8 @@ class PickOpenEntry extends PlaceholderQuickOpenEntry implements IPickOpenItem { this.description = item.description; this.detail = item.detail; this.tooltip = item.tooltip; - this.descriptionTooltip = item.description ? removeOcticons(item.description) : void 0; + this.descriptionOcticons = item.description ? parseOcticons(item.description) : void 0; + this.descriptionTooltip = this.descriptionOcticons ? this.descriptionOcticons.text : void 0; this.hasSeparator = item.separator && item.separator.border; this.separatorLabel = item.separator && item.separator.label; this.alwaysShow = item.alwaysShow; @@ -1060,6 +1063,23 @@ class PickOpenEntry extends PlaceholderQuickOpenEntry implements IPickOpenItem { this.fileKind = fileItem.fileKind; } + public matchesFuzzy(query: string, options: IInternalPickOptions): { labelHighlights: IMatch[], descriptionHighlights: IMatch[], detailHighlights: IMatch[] } { + if (!this.labelOcticons) { + this.labelOcticons = parseOcticons(this.getLabel()); // parse on demand + } + + const detail = this.getDetail(); + if (detail && options.matchOnDetail && !this.detailOcticons) { + this.detailOcticons = parseOcticons(detail); // parse on demand + } + + return { + labelHighlights: matchesFuzzyOcticonAware(query, this.labelOcticons), + descriptionHighlights: options.matchOnDescription && this.descriptionOcticons ? matchesFuzzyOcticonAware(query, this.descriptionOcticons) : void 0, + detailHighlights: options.matchOnDetail && this.detailOcticons ? matchesFuzzyOcticonAware(query, this.detailOcticons) : void 0 + }; + } + public getPayload(): any { return this.payload; } -- GitLab