💄 octicon handling in quick open

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);
-	if (firstOcticonIndex === -1) {
-		return word; // return early if the word does not include an octicon
-	}
-
-	return doParseOcticonAware(word, firstOcticonIndex).wordWithoutOcticons.trim();
+export interface IParsedOcticons {
+	text: string;
+	octiconOffsets?: number[];
 }
 
-function doParseOcticonAware(word: string, firstOcticonIndex: number): { wordWithoutOcticons: string, octiconOffsets: number[] } {
+export function parseOcticons(text: string): IParsedOcticons {
+	const firstOcticonIndex = text.indexOf(octiconStartMarker);
+	if (firstOcticonIndex === -1) {
+		return { text }; // return early if the word does not include an octicon
+	}
+
+	return doParseOcticons(text, firstOcticonIndex);
+}
+
+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) {

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 },
-		]);
-	});
 });

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 },
+		]);
+	});
+});

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;
 	}