From 231c6e776289c122968de35480cbbf862687e9a9 Mon Sep 17 00:00:00 2001
From: Eric Amodio <eamodio@microsoft.com>
Date: Mon, 16 Dec 2019 18:45:52 -0500
Subject: [PATCH] Updates #84695 - api review

`appendMarkdown` is now the only way to use ThemeIcons
---
 src/vs/base/common/htmlContent.ts             |  9 +---
 .../test/browser/markdownRenderer.test.ts     | 32 +++++--------
 .../base/test/common/markdownString.test.ts   | 48 ++++++++-----------
 src/vs/vscode.d.ts                            | 12 ++---
 src/vs/workbench/api/common/extHostTypes.ts   | 13 ++---
 5 files changed, 42 insertions(+), 72 deletions(-)

diff --git a/src/vs/base/common/htmlContent.ts b/src/vs/base/common/htmlContent.ts
index 59e2359e090..be074865f2b 100644
--- a/src/vs/base/common/htmlContent.ts
+++ b/src/vs/base/common/htmlContent.ts
@@ -5,7 +5,7 @@
 
 import { equals } from 'vs/base/common/arrays';
 import { UriComponents } from 'vs/base/common/uri';
-import { escapeCodicons, markdownUnescapeCodicons } from 'vs/base/common/codicons';
+import { escapeCodicons } from 'vs/base/common/codicons';
 
 export interface IMarkdownString {
 	readonly value: string;
@@ -39,10 +39,9 @@ export class MarkdownString implements IMarkdownString {
 
 	appendText(value: string): MarkdownString {
 		// escape markdown syntax tokens: http://daringfireball.net/projects/markdown/syntax#backslash
-		value = value
+		this._value += (this._supportThemeIcons ? escapeCodicons(value) : value)
 			.replace(/[\\`*_{}[\]()#+\-.!]/g, '\\$&')
 			.replace('\n', '\n\n');
-		this._value += this.supportThemeIcons ? markdownUnescapeCodicons(value) : value;
 
 		return this;
 	}
@@ -61,10 +60,6 @@ export class MarkdownString implements IMarkdownString {
 		this._value += '\n```\n';
 		return this;
 	}
-
-	static escapeThemeIcons(value: string): string {
-		return escapeCodicons(value);
-	}
 }
 
 export function isEmptyMarkdownString(oneOrMany: IMarkdownString | IMarkdownString[] | null | undefined): boolean {
diff --git a/src/vs/base/test/browser/markdownRenderer.test.ts b/src/vs/base/test/browser/markdownRenderer.test.ts
index 128c483aa12..499565a641c 100644
--- a/src/vs/base/test/browser/markdownRenderer.test.ts
+++ b/src/vs/base/test/browser/markdownRenderer.test.ts
@@ -54,34 +54,26 @@ suite('MarkdownRenderer', () => {
 
 		test('render appendText', () => {
 			const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-			mds.appendText('$(zap) $(dont match me)');
+			mds.appendText('$(zap) $(not a theme icon) $(add)');
 
 			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p><span class="codicon codicon-zap"></span> $(dont match me)</p>`);
-		});
-
-		test('render appendText escaped', () => {
-			const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-			mds.appendText(MarkdownString.escapeThemeIcons('$(zap) $(dont match me)'));
-
-			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p>$(zap) $(dont match me)</p>`);
+			assert.strictEqual(result.innerHTML, `<p>$(zap) $(not a theme icon) $(add)</p>`);
 		});
 
 		test('render appendMarkdown', () => {
 			const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-			mds.appendMarkdown('$(zap) $(dont match me)');
+			mds.appendMarkdown('$(zap) $(not a theme icon) $(add)');
 
 			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p><span class="codicon codicon-zap"></span> $(dont match me)</p>`);
+			assert.strictEqual(result.innerHTML, `<p><span class="codicon codicon-zap"></span> $(not a theme icon) <span class="codicon codicon-add"></span></p>`);
 		});
 
-		test('render appendMarkdown escaped', () => {
+		test('render appendMarkdown with escaped icon', () => {
 			const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-			mds.appendMarkdown(MarkdownString.escapeThemeIcons('$(zap) $(dont match me)'));
+			mds.appendMarkdown('\\$(zap) $(not a theme icon) $(add)');
 
 			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p>$(zap) $(dont match me)</p>`);
+			assert.strictEqual(result.innerHTML, `<p>$(zap) $(not a theme icon) <span class="codicon codicon-add"></span></p>`);
 		});
 
 	});
@@ -90,18 +82,18 @@ suite('MarkdownRenderer', () => {
 
 		test('render appendText', () => {
 			const mds = new MarkdownString(undefined, { supportThemeIcons: false });
-			mds.appendText('$(zap) $(dont match me)');
+			mds.appendText('$(zap) $(not a theme icon) $(add)');
 
 			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p>$(zap) $(dont match me)</p>`);
+			assert.strictEqual(result.innerHTML, `<p>$(zap) $(not a theme icon) $(add)</p>`);
 		});
 
-		test('render appendMarkdown', () => {
+		test('render appendMarkdown with escaped icon', () => {
 			const mds = new MarkdownString(undefined, { supportThemeIcons: false });
-			mds.appendMarkdown('$(zap) $(dont match me)');
+			mds.appendMarkdown('\\$(zap) $(not a theme icon) $(add)');
 
 			let result: HTMLElement = renderMarkdown(mds);
-			assert.strictEqual(result.innerHTML, `<p>$(zap) $(dont match me)</p>`);
+			assert.strictEqual(result.innerHTML, `<p>$(zap) $(not a theme icon) $(add)</p>`);
 		});
 
 	});
diff --git a/src/vs/base/test/common/markdownString.test.ts b/src/vs/base/test/common/markdownString.test.ts
index ee165144e32..f33f741f624 100644
--- a/src/vs/base/test/common/markdownString.test.ts
+++ b/src/vs/base/test/common/markdownString.test.ts
@@ -8,10 +8,9 @@ import { MarkdownString } from 'vs/base/common/htmlContent';
 
 suite('MarkdownString', () => {
 
-	test('escape', () => {
+	test('appendText', () => {
 
 		const mds = new MarkdownString();
-
 		mds.appendText('# foo\n*bar*');
 
 		assert.equal(mds.value, '\\# foo\n\n\\*bar\\*');
@@ -19,59 +18,54 @@ suite('MarkdownString', () => {
 
 	suite('ThemeIcons', () => {
 
-		test('escapeThemeIcons', () => {
-			assert.equal(
-				MarkdownString.escapeThemeIcons('$(zap) $(not an icon) foo$(bar)'),
-				'\\$(zap) $(not an icon) foo\\$(bar)'
-			);
-		});
-
 		suite('Support On', () => {
 
 			test('appendText', () => {
 				const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-				mds.appendText('$(zap)');
-
-				assert.equal(mds.value, '$(zap)');
-			});
-
-			test('appendText escaped', () => {
-				const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-				mds.appendText(MarkdownString.escapeThemeIcons('$(zap)'));
+				mds.appendText('$(zap) $(not a theme icon) $(add)');
 
-				assert.equal(mds.value, '\\\\$\\(zap\\)');
+				assert.equal(mds.value, '\\\\$\\(zap\\) $\\(not a theme icon\\) \\\\$\\(add\\)');
 			});
 
 			test('appendMarkdown', () => {
 				const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-				mds.appendMarkdown('$(zap)');
+				mds.appendMarkdown('$(zap) $(not a theme icon) $(add)');
 
-				assert.equal(mds.value, '$(zap)');
+				assert.equal(mds.value, '$(zap) $(not a theme icon) $(add)');
 			});
 
-			test('appendMarkdown escaped', () => {
+			test('appendMarkdown with escaped icon', () => {
 				const mds = new MarkdownString(undefined, { supportThemeIcons: true });
-				mds.appendMarkdown(MarkdownString.escapeThemeIcons('$(zap)'));
+				mds.appendMarkdown('\\$(zap) $(not a theme icon) $(add)');
 
-				assert.equal(mds.value, '\\$(zap)');
+				assert.equal(mds.value, '\\$(zap) $(not a theme icon) $(add)');
 			});
+
 		});
 
 		suite('Support Off', () => {
 
 			test('appendText', () => {
 				const mds = new MarkdownString(undefined, { supportThemeIcons: false });
-				mds.appendText('$(zap)');
+				mds.appendText('$(zap) $(not a theme icon) $(add)');
 
-				assert.equal(mds.value, '$\\(zap\\)');
+				assert.equal(mds.value, '$\\(zap\\) $\\(not a theme icon\\) $\\(add\\)');
 			});
 
 			test('appendMarkdown', () => {
 				const mds = new MarkdownString(undefined, { supportThemeIcons: false });
-				mds.appendMarkdown('$(zap)');
+				mds.appendMarkdown('$(zap) $(not a theme icon) $(add)');
+
+				assert.equal(mds.value, '$(zap) $(not a theme icon) $(add)');
+			});
 
-				assert.equal(mds.value, '$(zap)');
+			test('appendMarkdown with escaped icon', () => {
+				const mds = new MarkdownString(undefined, { supportThemeIcons: true });
+				mds.appendMarkdown('\\$(zap) $(not a theme icon) $(add)');
+
+				assert.equal(mds.value, '\\$(zap) $(not a theme icon) $(add)');
 			});
+
 		});
 
 	});
diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts
index 90b446a8075..c3a6c7ff176 100644
--- a/src/vs/vscode.d.ts
+++ b/src/vs/vscode.d.ts
@@ -2341,12 +2341,6 @@ declare module 'vscode' {
 	 */
 	export class MarkdownString {
 
-		/**
-		 * Escapes any [ThemeIcons](#ThemeIcon), e.g. `$(zap)`, in the string.
-		 * @param value A string.
-		 */
-		static escapeThemeIcons(value: string): string;
-
 		/**
 		 * The markdown string.
 		 */
@@ -2362,9 +2356,9 @@ declare module 'vscode' {
 		 * Creates a new markdown string with the given value.
 		 *
 		 * @param value Optional, initial value.
-		 * @param options Optional, options to specify whether [ThemeIcons](#ThemeIcon) are supported within the [`MarkdownString`](#MarkdownString).
+		 * @param supportThemeIcons Optional, Specifies whether [ThemeIcons](#ThemeIcon) are supported within the [`MarkdownString`](#MarkdownString).
 		 */
-		constructor(value?: string, options?: { supportThemeIcons?: boolean });
+		constructor(value?: string, supportThemeIcons?: boolean);
 
 		/**
 		 * Appends and escapes the given string to this markdown string.
@@ -2373,7 +2367,7 @@ declare module 'vscode' {
 		appendText(value: string): MarkdownString;
 
 		/**
-		 * Appends the given string 'as is' to this markdown string.
+		 * Appends the given string 'as is' to this markdown string. When [`supportThemeIcons`](#MarkdownString.supportThemeIcons) is `true`, [ThemeIcons](#ThemeIcon) in the `value` will be iconified.
 		 * @param value Markdown string.
 		 */
 		appendMarkdown(value: string): MarkdownString;
diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts
index d8afcf43654..51c2eff6d41 100644
--- a/src/vs/workbench/api/common/extHostTypes.ts
+++ b/src/vs/workbench/api/common/extHostTypes.ts
@@ -14,7 +14,7 @@ import { generateUuid } from 'vs/base/common/uuid';
 import * as vscode from 'vscode';
 import { FileSystemProviderErrorCode, markAsFileSystemProviderError } from 'vs/platform/files/common/files';
 import { RemoteAuthorityResolverErrorCode } from 'vs/platform/remote/common/remoteAuthorityResolver';
-import { markdownUnescapeCodicons, escapeCodicons } from 'vs/base/common/codicons';
+import { escapeCodicons } from 'vs/base/common/codicons';
 
 function es5ClassCompat(target: Function): any {
 	///@ts-ignore
@@ -1234,17 +1234,16 @@ export class MarkdownString {
 	isTrusted?: boolean;
 	readonly supportThemeIcons?: boolean;
 
-	constructor(value?: string, { supportThemeIcons }: { supportThemeIcons?: boolean } = {}) {
+	constructor(value?: string, supportThemeIcons: boolean = false) {
 		this.value = value ?? '';
-		this.supportThemeIcons = supportThemeIcons ?? false;
+		this.supportThemeIcons = supportThemeIcons;
 	}
 
 	appendText(value: string): MarkdownString {
 		// escape markdown syntax tokens: http://daringfireball.net/projects/markdown/syntax#backslash
-		value = value
+		this.value += (this.supportThemeIcons ? escapeCodicons(value) : value)
 			.replace(/[\\`*_{}[\]()#+\-.!]/g, '\\$&')
 			.replace('\n', '\n\n');
-		this.value += this.supportThemeIcons ? markdownUnescapeCodicons(value) : value;
 
 		return this;
 	}
@@ -1263,10 +1262,6 @@ export class MarkdownString {
 		this.value += '\n```\n';
 		return this;
 	}
-
-	static escapeThemeIcons(value: string): string {
-		return escapeCodicons(value);
-	}
 }
 
 @es5ClassCompat
-- 
GitLab