Fix #164630 (#165096)

* The true fix * some code clean up
microsoft · Nov 1, 2022 · 9d55dde · 9d55dde
1 parent 2e37ceb
commit 9d55dde
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 20 deletions.
diff --git a/src/vs/platform/extensionManagement/common/extensionNls.ts b/src/vs/platform/extensionManagement/common/extensionNls.ts
@@ -9,13 +9,14 @@ import { IExtensionManifest } from 'vs/platform/extensions/common/extensions';
 import { localize } from 'vs/nls';
 
 export interface ITranslations {
-	[key: string]: string | { message: string; comment: string[] };
+	[key: string]: string | { message: string; comment: string[] } | undefined;
 }
 
 export function localizeManifest(extensionManifest: IExtensionManifest, translations: ITranslations, fallbackTranslations?: ITranslations): IExtensionManifest {
 	try {
 		replaceNLStrings(extensionManifest, translations, fallbackTranslations);
 	} catch (error) {
+		console.error(error?.message ?? error);
 		/*Ignore Error*/
 	}
 	return extensionManifest;
@@ -39,27 +40,32 @@ function replaceNLStrings(extensionManifest: IExtensionManifest, messages: ITran
 				if (translated === undefined && originalMessages) {
 					translated = originalMessages[messageKey];
 				}
-				const message: string | undefined = typeof translated === 'string' ? translated : translated.message;
-				if (message !== undefined) {
-					// This branch returns ILocalizedString's instead of Strings so that the Command Palette can contain both the localized and the original value.
-					const original = originalMessages?.[messageKey];
-					const originalMessage: string | undefined = typeof original === 'string' ? original : original?.message;
-					if (
-						// if we are translating the title or category of a command
-						command && (key === 'title' || key === 'category') &&
-						// and the original value is not the same as the translated value
-						originalMessage && originalMessage !== message
-					) {
-						const localizedString: ILocalizedString = {
-							value: message,
-							original: originalMessage
-						};
-						obj[key] = localizedString;
-					} else {
-						obj[key] = message;
+				const message: string | undefined = typeof translated === 'string' ? translated : translated?.message;
+
+				// This branch returns ILocalizedString's instead of Strings so that the Command Palette can contain both the localized and the original value.
+				const original = originalMessages?.[messageKey];
+				const originalMessage: string | undefined = typeof original === 'string' ? original : original?.message;
+
+				if (!message) {
+					if (!originalMessage) {
+						console.warn(`[${extensionManifest.name}]: ${localize('missingNLSKey', "Couldn't find message for key {0}.", messageKey)}`);
 					}
+					return;
+				}
+
+				if (
+					// if we are translating the title or category of a command
+					command && (key === 'title' || key === 'category') &&
+					// and the original value is not the same as the translated value
+					originalMessage && originalMessage !== message
+				) {
+					const localizedString: ILocalizedString = {
+						value: message,
+						original: originalMessage
+					};
+					obj[key] = localizedString;
 				} else {
-					console.warn(`[${extensionManifest.name}]: ${localize('missingNLSKey', "Couldn't find message for key {0}.", messageKey)}`);
+					obj[key] = message;
 				}
 			}
 		} else if (isObject(value)) {

diff --git a/src/vs/platform/extensionManagement/test/common/extensionNls.test.ts b/src/vs/platform/extensionManagement/test/common/extensionNls.test.ts
@@ -107,4 +107,42 @@ suite('Localize Manifest', () => {
 		assert.strictEqual(localizedManifest.contributes?.authentication?.[0].label, 'Testauthentifizierung');
 		assert.strictEqual((localizedManifest.contributes?.configuration as IConfiguration).title, 'Testkonfiguration');
 	});
+
+	test('replaces template strings - is best effort #164630', function () {
+		const manifestWithTypo: IExtensionManifest = {
+			name: 'test',
+			publisher: 'test',
+			version: '1.0.0',
+			engines: {
+				vscode: '*'
+			},
+			contributes: {
+				authentication: [
+					{
+						id: 'test.authentication',
+						// This not existing in the bundle shouldn't cause an error.
+						label: '%doesnotexist%',
+					}
+				],
+				commands: [
+					{
+						command: 'test.command',
+						title: '%test.command.title%',
+						category: '%test.command.category%'
+					},
+				],
+			}
+		};
+
+		const localizedManifest = localizeManifest(
+			deepClone(manifestWithTypo),
+			{
+				'test.command.title': 'Test Command',
+				'test.command.category': 'Test Category'
+			});
+
+		assert.strictEqual(localizedManifest.contributes?.commands?.[0].title, 'Test Command');
+		assert.strictEqual(localizedManifest.contributes?.commands?.[0].category, 'Test Category');
+		assert.strictEqual(localizedManifest.contributes?.authentication?.[0].label, '%doesnotexist%');
+	});
 });