support group property for menu item and treat 'navigation' special

extensions/markdown/package.json

@@ -74,17 +74,20 @@
 				{
 					"when": "resourceLangId == markdown",
 					"command": "markdown.showPreview",
-					"alt": "markdown.showPreviewToSide"
+					"alt": "markdown.showPreviewToSide",
+					"group": "navigation"
 				},
 				{
 					"when": "resourceScheme == markdown",
-					"command": "markdown.showSource"
+					"command": "markdown.showSource",
+					"group": "navigation"
 				}
 			],
 			"explorer/context": [
 				{
 					"when": "resourceLangId == markdown",
-					"command": "markdown.showPreview"
+					"command": "markdown.showPreview",
+					"group": "navigation"
 				}
 			]
 		},

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

@@ -7,11 +7,37 @@
 
 import {localize} from 'vs/nls';
 import {IKeybindingService} from 'vs/platform/keybinding/common/keybindingService';
-import {MenuItemAction} from 'vs/platform/actions/common/actions';
+import {IMenu, MenuItemAction} from 'vs/platform/actions/common/actions';
 import {IAction} from 'vs/base/common/actions';
-import {ActionItem} from 'vs/base/browser/ui/actionbar/actionbar';
+import {ActionItem, Separator} from 'vs/base/browser/ui/actionbar/actionbar';
 import {domEvent} from 'vs/base/browser/event';
 
+
+export function fillInActions(menu: IMenu, target: IAction[] | { primary: IAction[]; secondary: IAction[];}): void {
+	const actions = menu.getActions();
+	if (actions.length === 0) {
+		return;
+	}
+
+	for (let tuple of actions) {
+		let [group, actions] = tuple;
+		if (group === 'navigation') {
+			if (Array.isArray<IAction>(target)) {
+				target.unshift(...actions);
+			} else {
+				target.primary.unshift(...actions);
+			}
+		} else {
+			if (Array.isArray<IAction>(target)) {
+				target.push(new Separator(), ...actions);
+			} else {
+				target.secondary.push(new Separator(), ...actions);
+			}
+		}
+	}
+}
+
+
 export function createActionItem(action: IAction, keybindingService: IKeybindingService): ActionItem {
 	if (action instanceof MenuItemAction) {
 		return new MenuItemActionItem(action, keybindingService);

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

@@ -20,6 +20,7 @@ export interface IDeclaredMenuItem {
 	command: string;
 	alt?: string;
 	when?: string;
+	group?: string;
 }
 
 export interface IMenuRegistry {
@@ -48,15 +49,21 @@ const _registry = new class {
 		}
 	}
 	getMenuItems(loc: MenuId): IMenuItem[] {
+		const result: IMenuItem[] = [];
 		const menuItems = this.menuItems[loc];
 		if (menuItems) {
-			return menuItems.map(item => {
-				const when = KbExpr.deserialize(item.when);
+			for (let item of menuItems) {
 				const command = this.commands[item.command];
+				if (!command) {
+					// warn?
+					continue;
+				}
+				const when = KbExpr.deserialize(item.when);
 				const alt = this.commands[item.alt];
-				return { when, command, alt };
-			});
+				result.push({ when, command, alt, group: item.group });
+			}
 		}
+		return result;
 	}
 };
 
@@ -81,9 +88,11 @@ export class MenuService implements IMenuService {
 	}
 }
 
+type MenuItemGroup = [string, IMenuItem[]];
+
 class Menu implements IMenu {
 
-	private _menuItems: IMenuItem[] = [];
+	private _menuGroups: MenuItemGroup[] = [];
 	private _disposables: IDisposable[] = [];
 	private _onDidChange = new Emitter<IMenu>();
 
@@ -94,22 +103,26 @@ class Menu implements IMenu {
 	) {
 		this._extensionService.onReady().then(_ => {
 
-			let menuItems = _registry.getMenuItems(id);
-			if (!menuItems) {
-				return;
-			}
-			let keysFilter: { [key: string]: boolean } = Object.create(null);
+			const menuItems = _registry.getMenuItems(id);
+			const keysFilter: { [key: string]: boolean } = Object.create(null);
+
+			let group: MenuItemGroup;
+			menuItems.sort(Menu._compareMenuItems);
+
 			for (let item of menuItems) {
-				if (!item.command) {
-					//TODO@joh, warn? default command?
-					continue;
+				// group by groupId
+				const groupName = Menu._group(item.group);
+				if (!group || group[0] !== groupName) {
+					group = [groupName, []];
+					this._menuGroups.push(group);
 				}
+				group[1].push(item);
 
-				// keep menu item
-				this._menuItems.push(item);
+				// keep keys for eventing
 				Menu._fillInKbExprKeys(item.when, keysFilter);
 			}
 
+			// subscribe to context changes
 			this._disposables.push(this._keybindingService.onDidChangeContext(keys => {
 				for (let k of keys) {
 					if (keysFilter[k]) {
@@ -132,13 +145,20 @@ class Menu implements IMenu {
 		return this._onDidChange.event;
 	}
 
-	getActions(): IAction[] {
-		const result: IAction[] = [];
-		for (let item of this._menuItems) {
-			if (this._keybindingService.contextMatchesRules(item.when)) {
-				result.push(new MenuItemAction(item,
-					this._keybindingService.getContextValue<URI>(ResourceContextKey.Resource),
-					this._keybindingService));
+	getActions(): [string, IAction[]][] {
+		const result: [string, IAction[]][] = [];
+		for (let group of this._menuGroups) {
+			const [id, items] = group;
+			const actions: IAction[] = [];
+			for (let item of items) {
+				if (this._keybindingService.contextMatchesRules(item.when)) {
+					actions.push(new MenuItemAction(item,
+						this._keybindingService.getContextValue<URI>(ResourceContextKey.Resource),
+						this._keybindingService));
+				}
+			}
+			if (actions.length > 0) {
+				result.push([id, actions]);
 			}
 		}
 		return result;
@@ -155,4 +175,28 @@ class Menu implements IMenu {
 			}
 		}
 	}
+
+	private static _compareMenuItems(a: IMenuItem, b: IMenuItem): number {
+		let ret: number;
+		if (a.group && b.group) {
+			ret = Menu._compareGroupId(a.group, b.group);
+		}
+		if (!ret) {
+			ret = a.command.title.localeCompare(b.command.title);
+		}
+		return ret;
+	}
+
+	private static _compareGroupId(a: string, b: string): number {
+		const a_boost = Number(a.substr(a.lastIndexOf('@') + 1));
+		const b_boost = Number(b.substr(b.lastIndexOf('@') + 1));
+		if (a_boost !== b_boost && !isNaN(a_boost) && !isNaN(b_boost)) {
+			return a_boost < b_boost ? -1 : 1;
+		}
+		return a.localeCompare(b);
+	}
+
+	private static _group(a: string): string {
+		return a && (a.substr(0, a.lastIndexOf('@')) || a);
+	}
 }
\ No newline at end of file

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

@@ -45,6 +45,10 @@ namespace schema {
 				collector.error(localize('optstring', "property `{0}` can be omitted or must be of type `string`", 'when'));
 				return false;
 			}
+			if (item.group && typeof item.group !== 'string') {
+				collector.error(localize('optstring', "property `{0}` can be omitted or must be of type `string`", 'group'));
+				return false;
+			}
 		}
 
 		return true;
@@ -64,6 +68,10 @@ namespace schema {
 			when: {
 				description: localize('vscode.extension.contributes.menuItem.when', 'Condition which must be true to show this item'),
 				type: 'string'
+			},
+			group: {
+				description: localize('vscode.extension.contributes.menuItem.group', 'Group into which this command belongs'),
+				type: 'string'
 			}
 		}
 	};

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

@@ -24,13 +24,14 @@ export interface CommandAction {
 
 export interface IMenu extends IDisposable {
 	onDidChange: Event<IMenu>;
-	getActions(): Actions.IAction[];
+	getActions(): [string, Actions.IAction[]][];
 }
 
 export interface IMenuItem {
 	command: CommandAction;
 	alt?: CommandAction;
 	when?: KbExpr;
+	group?: string;
 }
 
 export enum MenuId {

src/vs/workbench/browser/parts/editor/titleControl.ts

@@ -31,7 +31,7 @@ import {IInstantiationService} from 'vs/platform/instantiation/common/instantiat
 import {IKeybindingService} from 'vs/platform/keybinding/common/keybindingService';
 import {CloseEditorsInGroupAction, MoveGroupLeftAction, MoveGroupRightAction, SplitEditorAction, CloseEditorAction, KeepEditorAction, CloseOtherEditorsInGroupAction, CloseRightEditorsInGroupAction, ShowEditorsInGroupAction} from 'vs/workbench/browser/parts/editor/editorActions';
 import {IDisposable, dispose} from 'vs/base/common/lifecycle';
-import {createActionItem} from 'vs/platform/actions/browser/menuItemActionItem';
+import {createActionItem, fillInActions} from 'vs/platform/actions/browser/menuItemActionItem';
 import {IMenuService, IMenu, MenuId} from 'vs/platform/actions/common/actions';
 import {ResourceContextKey} from 'vs/platform/actions/common/resourceContextKey';
 
@@ -304,7 +304,7 @@ export abstract class TitleControl implements ITitleAreaControl {
 			secondary.push(...editorInputActions.secondary);
 
 			// MenuItems
-			primary.push(...this.contributedTitleBarMenu.getActions());
+			fillInActions(this.contributedTitleBarMenu, { primary, secondary });
 		}
 
 		return { primary, secondary };

src/vs/workbench/parts/files/browser/views/explorerViewer.ts

@@ -48,6 +48,7 @@ import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
 import {Keybinding, CommonKeybindings} from 'vs/base/common/keyCodes';
 import {IKeyboardEvent} from 'vs/base/browser/keyboardEvent';
 import {IMenuService, IMenu, MenuId} from 'vs/platform/actions/common/actions';
+import {fillInActions} from 'vs/platform/actions/browser/menuItemActionItem';
 
 export class FileDataSource implements IDataSource {
 	private workspace: IWorkspace;
@@ -484,8 +485,8 @@ export class FileController extends DefaultController {
 			getAnchor: () => anchor,
 			getActions: () => {
 				return this.state.actionProvider.getSecondaryActions(tree, stat).then(actions => {
-					// TODO@joh sorting,grouping
-					return [...this.contributedContextMenu.getActions(), ...actions];
+					fillInActions(this.contributedContextMenu, actions);
+					return actions;
 				});
 			},
 			getActionItem: this.state.actionProvider.getActionItem.bind(this.state.actionProvider, tree, stat),