Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#28617 - implemented according to comments in PR #28952

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
import { IWorkbenchActionRegistry, Extensions as ActionExtensions } from 'vs/workbench/common/actionRegistry';
import { KeybindingsRegistry } from 'vs/platform/keybinding/common/keybindingsRegistry';
import { RemoveFromEditorHistoryAction } from 'vs/workbench/browser/parts/quickopen/quickOpenController';
import { GlobalQuickOpenAction, QuickOpenSelectNextAction, QuickOpenSelectPreviousAction, inQuickOpenContext, getQuickNavigateHandler, QuickOpenNavigateNextAction, QuickOpenNavigatePreviousAction, defaultQuickOpenContext } from 'vs/workbench/browser/parts/quickopen/quickopen';
import { QuickOpenSelectNextAction, QuickOpenSelectPreviousAction, inQuickOpenContext, getQuickNavigateHandler, QuickOpenNavigateNextAction, QuickOpenNavigatePreviousAction, defaultQuickOpenContext } from "vs/workbench/browser/parts/quickopen/quickopen";

KeybindingsRegistry.registerCommandAndKeybindingRule({
id: 'workbench.action.closeQuickOpen',
Expand Down Expand Up @@ -50,7 +50,6 @@ const registry = <IWorkbenchActionRegistry>Registry.as(ActionExtensions.Workbenc

const globalQuickOpenKeybinding = { primary: KeyMod.CtrlCmd | KeyCode.KEY_P, secondary: [KeyMod.CtrlCmd | KeyCode.KEY_E], mac: { primary: KeyMod.CtrlCmd | KeyCode.KEY_P, secondary: null } };

registry.registerWorkbenchAction(new SyncActionDescriptor(GlobalQuickOpenAction, GlobalQuickOpenAction.ID, GlobalQuickOpenAction.LABEL, globalQuickOpenKeybinding), 'Go to File...');
registry.registerWorkbenchAction(new SyncActionDescriptor(QuickOpenSelectNextAction, QuickOpenSelectNextAction.ID, QuickOpenSelectNextAction.LABEL, { primary: null, mac: { primary: KeyMod.WinCtrl | KeyCode.KEY_N } }, inQuickOpenContext, KeybindingsRegistry.WEIGHT.workbenchContrib(50)), 'Select Next in Quick Open');
registry.registerWorkbenchAction(new SyncActionDescriptor(QuickOpenSelectPreviousAction, QuickOpenSelectPreviousAction.ID, QuickOpenSelectPreviousAction.LABEL, { primary: null, mac: { primary: KeyMod.WinCtrl | KeyCode.KEY_P } }, inQuickOpenContext, KeybindingsRegistry.WEIGHT.workbenchContrib(50)), 'Select Previous in Quick Open');
registry.registerWorkbenchAction(new SyncActionDescriptor(QuickOpenNavigateNextAction, QuickOpenNavigateNextAction.ID, QuickOpenNavigateNextAction.LABEL), 'Navigate Next in Quick Open');
Expand Down
19 changes: 0 additions & 19 deletions src/vs/workbench/browser/parts/quickopen/quickopen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,6 @@ export const inQuickOpenContext = ContextKeyExpr.has('inQuickOpen');
export const defaultQuickOpenContextKey = 'inFilesPicker';
export const defaultQuickOpenContext = ContextKeyExpr.and(inQuickOpenContext, ContextKeyExpr.has(defaultQuickOpenContextKey));

export class GlobalQuickOpenAction extends Action {

public static ID = 'workbench.action.quickOpen';
public static LABEL = nls.localize('quickOpen', "Go to File...");

constructor(id: string, label: string, @IQuickOpenService private quickOpenService: IQuickOpenService) {
super(id, label);

this.order = 100; // Allow other actions to position before or after
this.class = 'quickopen';
}

public run(): TPromise<any> {
this.quickOpenService.show(null);

return TPromise.as(true);
}
}

export class BaseQuickOpenNavigateAction extends Action {

constructor(
Expand Down
27 changes: 27 additions & 0 deletions src/vs/workbench/electron-browser/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { CommandsRegistry } from 'vs/platform/commands/common/commands';
import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
import URI from 'vs/base/common/uri';
import { IEditorOptions, Position as EditorPosition } from 'vs/platform/editor/common/editor';
import { IQuickOpenService } from 'vs/platform/quickOpen/common/quickOpen';
import { MenuRegistry, MenuId } from 'vs/platform/actions/common/actions';

// --- List Commands

Expand Down Expand Up @@ -421,3 +423,28 @@ export function registerCommands(): void {
});
});
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove the namespace GlobalQuickOpenCommand here, not sure why we would want it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrieken please validate this with @bpasero

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpasero that makes this change smaller because GlobalQuickOpenCommand.ID is used here and there

namespace GlobalQuickOpenCommand {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristianhosu while this adds the command, it no longer shows up in the F1 command palette. @jrieken didn't we add support to contribute commands to the F1 list recently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is support for this, i'll add it to F1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristianhosu looks like currently we are not using this explicitly but this test shows how to add a command to the F1 list: https://github.com/Microsoft/vscode/blob/master/src/vs/platform/actions/test/common/menuService.test.ts#L192

Can you add this?

export const ID = 'workbench.action.quickOpen';
export const LABEL = nls.localize('quickOpen', "Go to File...");

CommandsRegistry.registerCommand(ID, function (accessor: ServicesAccessor, args: [string]) {
const quickOpenService = accessor.get(IQuickOpenService);
const [prefix] = args;

return quickOpenService.show(prefix).then(() => {
return void 0;
});
});

KeybindingsRegistry.registerKeybindingRule({
id: 'workbench.action.quickOpen',
weight: KeybindingsRegistry.WEIGHT.workbenchContrib(),
when: undefined,
primary: KeyMod.CtrlCmd | KeyCode.KEY_P
});

MenuRegistry.appendMenuItem(MenuId.CommandPalette, {
command: { id: ID, title: LABEL }
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import * as nls from 'vs/nls';
import * as panel from 'vs/workbench/browser/panel';
import * as platform from 'vs/base/common/platform';
import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry';
import { GlobalQuickOpenAction } from 'vs/workbench/browser/parts/quickopen/quickopen';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It had a reference to the Action. As I removed the action, this had to be removed as well

import { ITerminalService, KEYBINDING_CONTEXT_TERMINAL_FOCUS, KEYBINDING_CONTEXT_TERMINAL_TEXT_SELECTED, TERMINAL_PANEL_ID, TERMINAL_DEFAULT_RIGHT_CLICK_COPY_PASTE, KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_VISIBLE, TerminalCursorStyle } from 'vs/workbench/parts/terminal/common/terminal';
import { TERMINAL_DEFAULT_SHELL_LINUX, TERMINAL_DEFAULT_SHELL_OSX, TERMINAL_DEFAULT_SHELL_WINDOWS } from 'vs/workbench/parts/terminal/electron-browser/terminal';
import { IWorkbenchActionRegistry, Extensions as ActionExtensions } from 'vs/workbench/common/actionRegistry';
Expand Down Expand Up @@ -148,7 +147,6 @@ configurationRegistry.registerConfiguration({
'default': [
ToggleTabFocusModeAction.ID,
FocusActiveGroupAction.ID,
GlobalQuickOpenAction.ID,
ShowAllCommandsAction.ID,
CreateNewTerminalAction.ID,
CopyTerminalSelectionAction.ID,
Expand Down
13 changes: 6 additions & 7 deletions src/vs/workbench/parts/watermark/electron-browser/watermark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace
import { IWorkbenchContribution, IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from 'vs/workbench/common/contributions';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not change watermark.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It had a reference to the Action. As I removed the action, this had to be removed as well

import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { GlobalQuickOpenAction } from 'vs/workbench/browser/parts/quickopen/quickopen';
import { OpenRecentAction } from 'vs/workbench/electron-browser/actions';
import { GlobalNewUntitledFileAction, OpenFileAction } from 'vs/workbench/parts/files/browser/fileActions';
import { OpenFolderAction, OpenFileFolderAction } from 'vs/workbench/browser/actions/fileActions';
Expand All @@ -39,10 +38,6 @@ const showCommands: WatermarkEntry = {
text: nls.localize('watermark.showCommands', "Show All Commands"),
ids: [ShowAllCommandsAction.ID]
};
const quickOpen: WatermarkEntry = {
text: nls.localize('watermark.quickOpen', "Go to File"),
ids: [GlobalQuickOpenAction.ID]
};
const openFileNonMacOnly: WatermarkEntry = {
text: nls.localize('watermark.openFile', "Open File"),
ids: [OpenFileAction.ID],
Expand Down Expand Up @@ -80,6 +75,10 @@ const startDebugging: WatermarkEntry = {
text: nls.localize('watermark.startDebugging', "Start Debugging"),
ids: [StartAction.ID]
};
const quickOpen: WatermarkEntry = {
text: nls.localize('workbench.action.quickOpen', "Quick Open"),
ids: ['workbench.action.quickOpen']
};

const noFolderEntries = [
showCommands,
Expand All @@ -88,12 +87,12 @@ const noFolderEntries = [
openFileOrFolderMacOnly,
openRecent,
newUntitledFileMacOnly,
toggleTerminal
toggleTerminal,
quickOpen
];

const folderEntries = [
showCommands,
quickOpen,
findInFiles,
startDebugging,
toggleTerminal
Expand Down