clean up icon path confusion

src/vs/code/electron-main/window.ts

@@ -953,7 +953,7 @@ export class CodeWindow implements ICodeWindow {
 		const segments: ITouchBarSegment[] = items.map(item => {
 			let icon: Electron.NativeImage;
 			if (item.iconPath) {
-				icon = nativeImage.createFromPath(item.iconPath);
+				icon = nativeImage.createFromPath(typeof item.iconPath === 'string' ? item.iconPath : item.iconPath.dark);
 				if (icon.isEmpty()) {
 					icon = void 0;
 				}

src/vs/platform/actions/browser/menuItemActionItem.ts

@@ -7,7 +7,7 @@
 
 import { localize } from 'vs/nls';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
-import { IMenu, MenuItemAction, IMenuActionOptions } from 'vs/platform/actions/common/actions';
+import { IMenu, MenuItemAction, IMenuActionOptions, ICommandAction } from 'vs/platform/actions/common/actions';
 import { IMessageService } from 'vs/platform/message/common/message';
 import Severity from 'vs/base/common/severity';
 import { IAction } from 'vs/base/common/actions';
@@ -17,6 +17,9 @@ import { domEvent } from 'vs/base/browser/event';
 import { Emitter } from 'vs/base/common/event';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
 import { memoize } from 'vs/base/common/decorators';
+import { IdGenerator } from 'vs/base/common/idGenerator';
+import { createCSSRule } from 'vs/base/browser/dom';
+import URI from 'vs/base/common/uri';
 
 class AltKeyEmitter extends Emitter<boolean> {
 
@@ -114,17 +117,22 @@ export function createActionItem(action: IAction, keybindingService: IKeybinding
 	return undefined;
 }
 
+const ids = new IdGenerator('menu-item-action-item-icon-');
+
 export class MenuItemActionItem extends ActionItem {
 
+	static readonly ICON_PATH_TO_CSS_RULES: Map<string /* path*/, string /* CSS rule */> = new Map<string, string>();
+
 	private _wantsAltCommand: boolean = false;
+	private _itemClassDispose: IDisposable;
 
 	constructor(
-		action: MenuItemAction,
+		private action: MenuItemAction,
 		@IKeybindingService private _keybindingService: IKeybindingService,
 		@IMessageService protected _messageService: IMessageService,
 		@IContextMenuService private _contextMenuService: IContextMenuService
 	) {
-		super(undefined, action, { icon: !!action.class, label: !action.class });
+		super(undefined, action, { icon: !!(action.class || action.item.iconPath), label: !action.class && !action.item.iconPath });
 	}
 
 	protected get _commandAction(): IAction {
@@ -142,6 +150,8 @@ export class MenuItemActionItem extends ActionItem {
 	render(container: HTMLElement): void {
 		super.render(container);
 
+		this._updateItemClass(this.action.item);
+
 		let mouseOver = false;
 		let altDown = false;
 
@@ -189,13 +199,41 @@ export class MenuItemActionItem extends ActionItem {
 
 	_updateClass(): void {
 		if (this.options.icon) {
-			const element = this.$e.getHTMLElement();
 			if (this._commandAction !== this._action) {
-				element.classList.remove(this._action.class);
+				this._updateItemClass(this.action.alt.item);
 			} else if ((<MenuItemAction>this._action).alt) {
-				element.classList.remove((<MenuItemAction>this._action).alt.class);
+				this._updateItemClass(this.action.item);
+			}
+		}
+	}
+
+	_updateItemClass(item: ICommandAction): void {
+		dispose(this._itemClassDispose);
+		this._itemClassDispose = undefined;
+
+		if (item.iconPath) {
+			let iconClass: string;
+			if (typeof item.iconPath === 'string') {
+				if (MenuItemActionItem.ICON_PATH_TO_CSS_RULES.has(item.iconPath)) {
+					iconClass = MenuItemActionItem.ICON_PATH_TO_CSS_RULES.get(item.iconPath);
+				} else {
+					iconClass = ids.nextId();
+					createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath).toString()}")`);
+					MenuItemActionItem.ICON_PATH_TO_CSS_RULES.set(item.iconPath, iconClass);
+				}
+			} else {
+				if (MenuItemActionItem.ICON_PATH_TO_CSS_RULES.has(item.iconPath.dark)) {
+					iconClass = MenuItemActionItem.ICON_PATH_TO_CSS_RULES.get(item.iconPath.dark);
+				} else {
+					iconClass = ids.nextId();
+					createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.light).toString()}")`);
+					createCSSRule(`.vs-dark .icon.${iconClass}, .hc-black .icon.${iconClass}`, `background-image: url("${URI.file(item.iconPath.dark).toString()}")`);
+					MenuItemActionItem.ICON_PATH_TO_CSS_RULES.set(item.iconPath.dark, iconClass);
+				}
 			}
-			element.classList.add('icon', this._commandAction.class);
+
+			this.$e.getHTMLElement().classList.add('icon', iconClass);
+			this._itemClassDispose = { dispose: () => this.$e.getHTMLElement().classList.remove('icon', iconClass) };
 		}
 	}
 }

src/vs/platform/actions/common/actions.ts

@@ -23,8 +23,7 @@ export interface ICommandAction {
 	id: string;
 	title: string | ILocalizedString;
 	category?: string | ILocalizedString;
-	iconClass?: string;
-	iconPath?: string;
+	iconPath?: string | { light: string; dark: string; };
 	precondition?: ContextKeyExpr;
 }
 
@@ -180,7 +179,7 @@ export class MenuItemAction extends ExecuteCommandAction {
 		@ICommandService commandService: ICommandService
 	) {
 		typeof item.title === 'string' ? super(item.id, item.title, commandService) : super(item.id, item.title.value, commandService);
-		this._cssClass = item.iconClass;
+		this._cssClass = undefined;
 		this._enabled = !item.precondition || contextKeyService.contextMatchesRules(item.precondition);
 		this._options = options || {};
 

src/vs/platform/actions/electron-browser/menusExtensionPoint.ts

@@ -4,12 +4,9 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import URI from 'vs/base/common/uri';
-import { createCSSRule } from 'vs/base/browser/dom';
 import { localize } from 'vs/nls';
 import { isFalsyOrWhitespace } from 'vs/base/common/strings';
 import { join } from 'path';
-import { IdGenerator } from 'vs/base/common/idGenerator';
 import { IJSONSchema } from 'vs/base/common/jsonSchema';
 import { forEach } from 'vs/base/common/collections';
 import { IExtensionPointUser, ExtensionMessageCollector, ExtensionsRegistry } from 'vs/platform/extensions/common/extensionsRegistry';
@@ -281,33 +278,27 @@ namespace schema {
 
 ExtensionsRegistry.registerExtensionPoint<schema.IUserFriendlyCommand | schema.IUserFriendlyCommand[]>('commands', [], schema.commandsContribution).setHandler(extensions => {
 
-	const ids = new IdGenerator('contrib-cmd-icon-');
-
 	function handleCommand(userFriendlyCommand: schema.IUserFriendlyCommand, extension: IExtensionPointUser<any>) {
 
 		if (!schema.isValidCommand(userFriendlyCommand, extension.collector)) {
 			return;
 		}
 
-		let { icon, category, title, command } = userFriendlyCommand;
-		let iconClass: string;
-		let iconPath: string;
+		const { icon, category, title, command } = userFriendlyCommand;
+
+		let absoluteIcon: string | { light: string; dark: string; };
 		if (icon) {
-			iconClass = ids.nextId();
 			if (typeof icon === 'string') {
-				iconPath = join(extension.description.extensionFolderPath, icon);
-				createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(iconPath).toString()}")`);
+				absoluteIcon = join(extension.description.extensionFolderPath, icon);
 			} else {
-				const light = join(extension.description.extensionFolderPath, icon.light);
-				const dark = join(extension.description.extensionFolderPath, icon.dark);
-				createCSSRule(`.icon.${iconClass}`, `background-image: url("${URI.file(light).toString()}")`);
-				createCSSRule(`.vs-dark .icon.${iconClass}, .hc-black .icon.${iconClass}`, `background-image: url("${URI.file(dark).toString()}")`);
-
-				iconPath = join(extension.description.extensionFolderPath, icon.dark);
+				absoluteIcon = {
+					dark: join(extension.description.extensionFolderPath, icon.dark),
+					light: join(extension.description.extensionFolderPath, icon.light)
+				};
 			}
 		}
 
-		if (MenuRegistry.addCommand({ id: command, title, category, iconClass, iconPath })) {
+		if (MenuRegistry.addCommand({ id: command, title, category, iconPath: absoluteIcon })) {
 			extension.collector.info(localize('dup', "Command `{0}` appears multiple times in the `commands` section.", userFriendlyCommand.command));
 		}
 	}

src/vs/workbench/parts/codeEditor/electron-browser/media/codeEditor.css

-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-.toggle-word-wrap-action {
-	background: url('WordWrap_16x.svg') center center no-repeat;
-}

src/vs/workbench/parts/codeEditor/electron-browser/toggleWordWrap.ts

@@ -4,7 +4,6 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import 'vs/css!./media/codeEditor';
 import * as nls from 'vs/nls';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
 import { IEditorContribution } from 'vs/editor/common/editorCommon';
@@ -259,7 +258,7 @@ MenuRegistry.appendMenuItem(MenuId.EditorTitle, {
 	command: {
 		id: 'editor.action.toggleWordWrap',
 		title: nls.localize('unwrapMinified', "Disable wrapping for this file"),
-		iconClass: 'toggle-word-wrap-action'
+		iconPath: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath
 	},
 	group: 'navigation',
 	order: 1,
@@ -273,7 +272,7 @@ MenuRegistry.appendMenuItem(MenuId.EditorTitle, {
 	command: {
 		id: 'editor.action.toggleWordWrap',
 		title: nls.localize('wrapMinified', "Enable wrapping for this file"),
-		iconClass: 'toggle-word-wrap-action'
+		iconPath: URI.parse(require.toUrl('vs/workbench/parts/codeEditor/electron-browser/WordWrap_16x.svg')).fsPath
 	},
 	group: 'navigation',
 	order: 1,

src/vs/workbench/parts/files/electron-browser/fileActions.contribution.ts

@@ -23,7 +23,7 @@ import { OPEN_FOLDER_SETTINGS_COMMAND, OPEN_FOLDER_SETTINGS_LABEL } from 'vs/wor
 import { AutoSaveContext } from 'vs/workbench/services/textfile/common/textfiles';
 import { ResourceContextKey } from 'vs/workbench/common/resources';
 import { WorkbenchListDoubleSelection } from 'vs/platform/list/browser/listService';
-
+import URI from 'vs/base/common/uri';
 
 // Contribute Global Actions
 const category = nls.localize('filesCategory', "File");
@@ -116,17 +116,23 @@ function appendEditorTitleContextMenuItem(id: string, title: string, when: Conte
 }
 
 // Editor Title Menu for Conflict Resolution
-appendSaveConflictEditorTitleAction('workbench.files.action.acceptLocalChanges', nls.localize('acceptLocalChanges', "Use your changes and overwrite disk contents"), 'save-conflict-action-accept-changes', -10, acceptLocalChangesCommand);
-appendSaveConflictEditorTitleAction('workbench.files.action.revertLocalChanges', nls.localize('revertLocalChanges', "Discard your changes and revert to content on disk"), 'save-conflict-action-revert-changes', -9, revertLocalChangesCommand);
+appendSaveConflictEditorTitleAction('workbench.files.action.acceptLocalChanges', nls.localize('acceptLocalChanges', "Use your changes and overwrite disk contents"), {
+	light: URI.parse(require.toUrl(`vs/workbench/parts/files/electron-browser/media/check.svg`)).fsPath,
+	dark: URI.parse(require.toUrl(`vs/workbench/parts/files/electron-browser/media/check-inverse.svg`)).fsPath
+}, -10, acceptLocalChangesCommand);
+appendSaveConflictEditorTitleAction('workbench.files.action.revertLocalChanges', nls.localize('revertLocalChanges', "Discard your changes and revert to content on disk"), {
+	light: URI.parse(require.toUrl(`vs/workbench/parts/files/electron-browser/media/undo.svg`)).fsPath,
+	dark: URI.parse(require.toUrl(`vs/workbench/parts/files/electron-browser/media/undo-inverse.svg`)).fsPath
+}, -9, revertLocalChangesCommand);
 
-function appendSaveConflictEditorTitleAction(id: string, title: string, iconClass: string, order: number, command: ICommandHandler): void {
+function appendSaveConflictEditorTitleAction(id: string, title: string, iconPath: { dark: string; light: string; }, order: number, command: ICommandHandler): void {
 
 	// Command
 	CommandsRegistry.registerCommand(id, command);
 
 	// Action
 	MenuRegistry.appendMenuItem(MenuId.EditorTitle, {
-		command: { id, title, iconClass },
+		command: { id, title, iconPath },
 		when: ContextKeyExpr.equals(CONFLICT_RESOLUTION_CONTEXT, true),
 		group: 'navigation',
 		order

src/vs/workbench/parts/files/electron-browser/media/fileactions.css

@@ -75,22 +75,6 @@
 	background-image: url('split-editor-horizontal-inverse.svg');
 }
 
-.monaco-workbench .save-conflict-action-accept-changes {
-	background: url('check.svg') center center no-repeat;
-}
-
-.vs-dark .monaco-workbench .save-conflict-action-accept-changes {
-	background: url('check-inverse.svg') center center no-repeat;
-}
-
-.monaco-workbench .save-conflict-action-revert-changes {
-	background: url('undo.svg') center center no-repeat;
-}
-
-.vs-dark .monaco-workbench .save-conflict-action-revert-changes {
-	background: url('undo-inverse.svg') center center no-repeat;
-}
-
 .monaco-workbench .file-editor-action.action-open-preview {
 	background: url('Preview.svg') center center no-repeat;
 }