From dc1c2c7e538ec30a19319be83e2da50529b7a06c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 20 May 2019 15:19:40 -0700 Subject: [PATCH] Remove shell notification in favor of dropdown entry Fixes #74019 --- .../externalTerminal.contribution.ts | 2 +- .../terminal/browser/terminalActions.ts | 30 ++++++-- .../terminal/browser/terminalService.ts | 76 +------------------ .../contrib/terminal/common/terminal.ts | 11 ++- .../terminal/common/terminalService.ts | 3 +- .../electron-browser/terminalService.ts | 9 ++- 6 files changed, 44 insertions(+), 87 deletions(-) diff --git a/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts b/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts index f380af8cd5a..4facc9d8566 100644 --- a/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts +++ b/src/vs/workbench/contrib/externalTerminal/electron-browser/externalTerminal.contribution.ts @@ -94,7 +94,7 @@ CommandsRegistry.registerCommand({ const directoriesToOpen = distinct(stats.filter(data => data.success).map(({ stat }) => stat!.isDirectory ? stat!.resource.fsPath : paths.dirname(stat!.resource.fsPath))); return directoriesToOpen.map(dir => { if (configurationService.getValue().terminal.explorerKind === 'integrated') { - const instance = integratedTerminalService.createTerminal({ cwd: dir }, true); + const instance = integratedTerminalService.createTerminal({ cwd: dir }); if (instance && (resources.length === 1 || !resource || dir === resource.fsPath || dir === paths.dirname(resource.fsPath))) { integratedTerminalService.setActiveInstance(instance); integratedTerminalService.showPanel(true); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalActions.ts b/src/vs/workbench/contrib/terminal/browser/terminalActions.ts index 8f33f667a3f..3fe9c77fdc6 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalActions.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalActions.ts @@ -35,6 +35,7 @@ import { Schemas } from 'vs/base/common/network'; import { URI } from 'vs/base/common/uri'; import { isWindows } from 'vs/base/common/platform'; import { withNullAsUndefined } from 'vs/base/common/types'; +import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; export const TERMINAL_PICKER_PREFIX = 'term '; @@ -85,7 +86,7 @@ export class ToggleTerminalAction extends TogglePanelAction { if (this.terminalService.terminalInstances.length === 0) { // If there is not yet an instance attempt to create it here so that we can suggest a // new shell on Windows (and not do so when the panel is restored on reload). - const newTerminalInstance = this.terminalService.createTerminal(undefined, true); + const newTerminalInstance = this.terminalService.createTerminal(undefined); const toDispose = newTerminalInstance.onProcessIdReady(() => { newTerminalInstance.focus(); toDispose.dispose(); @@ -329,7 +330,7 @@ export class CreateNewTerminalAction extends Action { if (folders.length <= 1) { // Allow terminal service to handle the path when there is only a // single root - instancePromise = Promise.resolve(this.terminalService.createTerminal(undefined, true)); + instancePromise = Promise.resolve(this.terminalService.createTerminal(undefined)); } else { const options: IPickOptions = { placeHolder: nls.localize('workbench.action.terminal.newWorkspacePlaceholder', "Select current working directory for new terminal") @@ -339,7 +340,7 @@ export class CreateNewTerminalAction extends Action { // Don't create the instance if the workspace picker was canceled return null; } - return this.terminalService.createTerminal({ cwd: workspace.uri }, true); + return this.terminalService.createTerminal({ cwd: workspace.uri }); }); } @@ -366,7 +367,7 @@ export class CreateNewInActiveWorkspaceTerminalAction extends Action { } public run(event?: any): Promise { - const instance = this.terminalService.createTerminal(undefined, true); + const instance = this.terminalService.createTerminal(undefined); if (!instance) { return Promise.resolve(undefined); } @@ -720,6 +721,14 @@ export class SwitchTerminalAction extends Action { if (!item || !item.split) { return Promise.resolve(null); } + if (item === SwitchTerminalActionViewItem.SEPARATOR) { + this.terminalService.refreshActiveTab(); + return Promise.resolve(null); + } + if (item === SelectDefaultShellWindowsTerminalAction.LABEL) { + this.terminalService.refreshActiveTab(); + return this.terminalService.selectDefaultWindowsShell(); + } const selectedTabIndex = parseInt(item.split(':')[0], 10) - 1; this.terminalService.setActiveTabByIndex(selectedTabIndex); return this.terminalService.showPanel(true); @@ -728,11 +737,14 @@ export class SwitchTerminalAction extends Action { export class SwitchTerminalActionViewItem extends SelectActionViewItem { + public static readonly SEPARATOR = '─────────'; + constructor( action: IAction, @ITerminalService private readonly terminalService: ITerminalService, @IThemeService themeService: IThemeService, - @IContextViewService contextViewService: IContextViewService + @IContextViewService contextViewService: IContextViewService, + @IWorkbenchEnvironmentService private workbenchEnvironmentService: IWorkbenchEnvironmentService ) { super(null, action, terminalService.getTabLabels().map(label => { text: label }), terminalService.activeTabIndex, contextViewService, { ariaLabel: nls.localize('terminals', 'Open Terminals.') }); @@ -743,7 +755,13 @@ export class SwitchTerminalActionViewItem extends SelectActionViewItem { } private _updateItems(): void { - this.setOptions(this.terminalService.getTabLabels().map(label => { text: label }), this.terminalService.activeTabIndex); + const items = this.terminalService.getTabLabels().map(label => { text: label }); + let enableSelectDefaultShell = this.workbenchEnvironmentService.configuration.remoteAuthority ? false : isWindows; + if (enableSelectDefaultShell) { + items.push({ text: SwitchTerminalActionViewItem.SEPARATOR }); + items.push({ text: SelectDefaultShellWindowsTerminalAction.LABEL }); + } + this.setOptions(items, this.terminalService.activeTabIndex); } } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index 094dcdb7e45..9a8b38a0fb7 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -3,21 +3,19 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as nls from 'vs/nls'; import * as platform from 'vs/base/common/platform'; -import { ITerminalService, TERMINAL_PANEL_ID, ITerminalInstance, IShellLaunchConfig, NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY, ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalService, TERMINAL_PANEL_ID, ITerminalInstance, IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; import { TerminalService as CommonTerminalService } from 'vs/workbench/contrib/terminal/common/terminalService'; import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle'; -import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; +import { IStorageService } from 'vs/platform/storage/common/storage'; import { TerminalPanel } from 'vs/workbench/contrib/terminal/browser/terminalPanel'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; -import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; +import { INotificationService } from 'vs/platform/notification/common/notification'; import { TerminalTab } from 'vs/workbench/contrib/terminal/browser/terminalTab'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IFileService } from 'vs/platform/files/common/files'; import { TerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminalInstance'; @@ -36,7 +34,6 @@ export abstract class TerminalService extends CommonTerminalService implements I @INotificationService notificationService: INotificationService, @IDialogService dialogService: IDialogService, @IInstantiationService protected readonly _instantiationService: IInstantiationService, - @IWorkbenchEnvironmentService private _environmentService: IWorkbenchEnvironmentService, @IExtensionService extensionService: IExtensionService, @IFileService fileService: IFileService, @IRemoteAgentService remoteAgentService: IRemoteAgentService @@ -52,7 +49,7 @@ export abstract class TerminalService extends CommonTerminalService implements I return instance; } - public createTerminal(shell: IShellLaunchConfig = {}, wasNewTerminalAction?: boolean): ITerminalInstance { + public createTerminal(shell: IShellLaunchConfig = {}): ITerminalInstance { const terminalTab = this._instantiationService.createInstance(TerminalTab, this._terminalFocusContextKey, this.configHelper, @@ -68,74 +65,9 @@ export abstract class TerminalService extends CommonTerminalService implements I this.setActiveInstanceByIndex(0); } this._onInstancesChanged.fire(); - this._suggestShellChange(wasNewTerminalAction); return instance; } - private _suggestShellChange(wasNewTerminalAction?: boolean): void { - // Only suggest on Windows since $SHELL works great for macOS/Linux - if (!platform.isWindows) { - return; - } - - if (this._environmentService.configuration.remoteAuthority) { - // Don't suggest if the opened workspace is remote - return; - } - - // Only suggest when the terminal instance is being created by an explicit user action to - // launch a terminal, as opposed to something like tasks, debug, panel restore, etc. - if (!wasNewTerminalAction) { - return; - } - - if (this._environmentService.configuration.remoteAuthority) { - // Don't suggest if the opened workspace is remote - return; - } - - // Don't suggest if the user has explicitly opted out - const neverSuggest = this._storageService.getBoolean(NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY, StorageScope.GLOBAL, false); - if (neverSuggest) { - return; - } - - // Never suggest if the setting is non-default already (ie. they set the setting manually) - if (this.configHelper.config.shell.windows !== this.getDefaultShell(platform.Platform.Windows)) { - this._storageService.store(NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY, true, StorageScope.GLOBAL); - return; - } - - this._notificationService.prompt( - Severity.Info, - nls.localize('terminal.integrated.chooseWindowsShellInfo', "You can change the default terminal shell by selecting the customize button."), - [{ - label: nls.localize('customize', "Customize"), - run: () => { - this.selectDefaultWindowsShell().then(shell => { - if (!shell) { - return Promise.resolve(null); - } - // Launch a new instance with the newly selected shell - const instance = this.createTerminal({ - executable: shell, - args: this.configHelper.config.shellArgs.windows - }); - if (instance) { - this.setActiveInstance(instance); - } - return Promise.resolve(null); - }); - } - }, - { - label: nls.localize('never again', "Don't Show Again"), - isSecondary: true, - run: () => this._storageService.store(NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY, true, StorageScope.GLOBAL) - }] - ); - } - public focusFindWidget(): Promise { return this.showPanel(false).then(() => { const panel = this._panelService.getActivePanel() as TerminalPanel; diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 6c20b2c49e5..719f1add08b 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -37,7 +37,6 @@ export const KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_FOCUSED = new RawContextKey export const KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_INPUT_NOT_FOCUSED: ContextKeyExpr = KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_INPUT_FOCUSED.toNegated(); export const IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY = 'terminal.integrated.isWorkspaceShellAllowed'; -export const NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY = 'terminal.integrated.neverSuggestSelectWindowsShell'; export const NEVER_MEASURE_RENDER_TIME_STORAGE_KEY = 'terminal.integrated.neverMeasureRenderTime'; // The creation of extension host terminals is delayed by this value (milliseconds). The purpose of @@ -216,10 +215,8 @@ export interface ITerminalService { /** * Creates a terminal. * @param shell The shell launch configuration to use. - * @param wasNewTerminalAction Whether this was triggered by a new terminal action, if so a - * default shell selection dialog may display. */ - createTerminal(shell?: IShellLaunchConfig, wasNewTerminalAction?: boolean): ITerminalInstance; + createTerminal(shell?: IShellLaunchConfig): ITerminalInstance; /** * Creates a terminal renderer. @@ -245,6 +242,12 @@ export interface ITerminalService { setActiveTabToPrevious(): void; setActiveTabByIndex(tabIndex: number): void; + /** + * Fire the onActiveTabChanged event, this will trigger the terminal dropdown to be updated, + * among other things. + */ + refreshActiveTab(): void; + showPanel(focus?: boolean): Promise; hidePanel(): void; focusFindWidget(): Promise; diff --git a/src/vs/workbench/contrib/terminal/common/terminalService.ts b/src/vs/workbench/contrib/terminal/common/terminalService.ts index 01b3254479f..74d076206c9 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalService.ts @@ -41,7 +41,7 @@ export abstract class TerminalService implements ITerminalService { public get terminalInstances(): ITerminalInstance[] { return this._terminalInstances; } public get terminalTabs(): ITerminalTab[] { return this._terminalTabs; } - private readonly _onActiveTabChanged = new Emitter(); + protected readonly _onActiveTabChanged = new Emitter(); public get onActiveTabChanged(): Event { return this._onActiveTabChanged.event; } protected readonly _onInstanceCreated = new Emitter(); public get onInstanceCreated(): Event { return this._onInstanceCreated.event; } @@ -104,6 +104,7 @@ export abstract class TerminalService implements ITerminalService { protected abstract _getWslPath(path: string): Promise; protected abstract _getWindowsBuildNumber(): number; + public abstract refreshActiveTab(): void; public abstract createTerminal(shell?: IShellLaunchConfig, wasNewTerminalAction?: boolean): ITerminalInstance; public abstract createInstance(terminalFocusContextKey: IContextKey, configHelper: ITerminalConfigHelper, container: HTMLElement, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance; public abstract getDefaultShell(platform: Platform): string; diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalService.ts index 6ead83f5b67..929f9626d11 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalService.ts @@ -20,7 +20,6 @@ import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { ipcRenderer as ipc } from 'electron'; import { IOpenFileRequest } from 'vs/platform/windows/common/windows'; -import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IQuickInputService, IQuickPickItem, IPickOptions } from 'vs/platform/quickinput/common/quickInput'; import { coalesce } from 'vs/base/common/arrays'; @@ -45,11 +44,10 @@ export class TerminalService extends BrowserTerminalService implements ITerminal @INotificationService notificationService: INotificationService, @IDialogService dialogService: IDialogService, @IExtensionService extensionService: IExtensionService, - @IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService, @IFileService fileService: IFileService, @IRemoteAgentService remoteAgentService: IRemoteAgentService ) { - super(contextKeyService, panelService, layoutService, lifecycleService, storageService, notificationService, dialogService, instantiationService, environmentService, extensionService, fileService, remoteAgentService); + super(contextKeyService, panelService, layoutService, lifecycleService, storageService, notificationService, dialogService, instantiationService, extensionService, fileService, remoteAgentService); this._configHelper = this._instantiationService.createInstance(TerminalConfigHelper, linuxDistro); ipc.on('vscode:openFiles', (_event: any, request: IOpenFileRequest) => { @@ -102,6 +100,11 @@ export class TerminalService extends BrowserTerminalService implements ITerminal return getDefaultShell(p); } + public refreshActiveTab(): void { + // Fire active instances changed + this._onActiveTabChanged.fire(); + } + public selectDefaultWindowsShell(): Promise { return this._detectWindowsShells().then(shells => { const options: IPickOptions = {