Address PR comments regarding default handling upon startup

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>

Author: tsmaeder

packages/terminal/src/browser/terminal-frontend-contribution.ts

@@ -30,9 +30,9 @@ import {
     isWindows
 } from '@theia/core/lib/common';
 import {
-    ApplicationShell, KeybindingContribution, KeyCode, Key, WidgetManager,
+    ApplicationShell, KeybindingContribution, KeyCode, Key, WidgetManager, PreferenceService,
     KeybindingRegistry, Widget, LabelProvider, WidgetOpenerOptions, StorageService,
-    QuickInputService, codicon, CommonCommands, FrontendApplicationContribution, OnWillStopAction, Dialog, ConfirmDialog, FrontendApplication
+    QuickInputService, codicon, CommonCommands, FrontendApplicationContribution, OnWillStopAction, Dialog, ConfirmDialog, FrontendApplication, PreferenceScope
 } from '@theia/core/lib/browser';
 import { TabBarToolbarContribution, TabBarToolbarRegistry } from '@theia/core/lib/browser/shell/tab-bar-toolbar';
 import { TERMINAL_WIDGET_FACTORY_ID, TerminalWidgetFactoryOptions, TerminalWidgetImpl } from './terminal-widget-impl';
@@ -198,6 +198,9 @@ export class TerminalFrontendContribution implements FrontendApplicationContribu
     @inject(StorageService)
     protected readonly storageService: StorageService;
 
+    @inject(PreferenceService)
+    protected readonly preferenceService: PreferenceService;
+
     @inject(TerminalPreferences)
     protected terminalPreferences: TerminalPreferences;
 
@@ -245,9 +248,31 @@ export class TerminalFrontendContribution implements FrontendApplicationContribu
         await this.contributeDefaultProfiles();
 
         this.terminalPreferences.onPreferenceChanged(e => {
-            this.mergePreferencesPromise = this.mergePreferencesPromise.finally(() => this.mergePreferences());
+            if (e.preferenceName.startsWith('terminal.integrated.')) {
+                this.mergePreferencesPromise = this.mergePreferencesPromise.finally(() => this.mergePreferences());
+            }
         });
         this.mergePreferencesPromise = this.mergePreferencesPromise.finally(() => this.mergePreferences());
+
+        this.profileService.onAdded(id => {
+            // extension contributions get read after this point: need to set the default profile if necessary
+            let defaultProfileId;
+            switch (OS.type()) {
+                case OS.Type.Windows: {
+                    defaultProfileId = this.terminalPreferences['terminal.integrated.defaultProfile.windows'];
+                    break;
+                }
+                case OS.Type.Linux: {
+                    defaultProfileId = this.terminalPreferences['terminal.integrated.defaultProfile.linux'];
+                    break;
+                }
+                case OS.Type.OSX: {
+                    defaultProfileId = this.terminalPreferences['terminal.integrated.defaultProfile.osx'];
+                    break;
+                }
+            }
+            this.profileService.setDefaultProfile(defaultProfileId);
+        });
     }
 
     async contributeDefaultProfiles(): Promise<void> {
@@ -259,7 +284,7 @@ export class TerminalFrontendContribution implements FrontendApplicationContribu
                 ])!
             }));
         } else {
-            this.contributedProfileStore.registerTerminalProfile('cmd', new ShellTerminalProfile(this, {
+            this.contributedProfileStore.registerTerminalProfile('SHELL', new ShellTerminalProfile(this, {
                 shellPath: await this.resolveShellPath('${SHELL}')!,
                 shellArgs: ['-l']
             }));
@@ -916,7 +941,7 @@ export class TerminalFrontendContribution implements FrontendApplicationContribu
             return;
         }
 
-        this.profileService.setDefaultProfile(result[0]);
+        this.preferenceService.set(`terminal.integrated.defaultProfile.${OS.type().toLowerCase()}`, result[0], PreferenceScope.User);
     }
 
     protected async openActiveWorkspaceTerminal(options?: ApplicationShell.WidgetOptions): Promise<void> {

packages/terminal/src/browser/terminal-profile-service.ts

@@ -32,6 +32,8 @@ export const NULL_PROFILE: TerminalProfile = {
 };
 
 export interface TerminalProfileService {
+    onAdded: Event<string>;
+    onRemoved: Event<string>;
     getProfile(id: string): TerminalProfile | undefined
     readonly all: [string, TerminalProfile][];
     setDefaultProfile(id: string): void;
@@ -84,6 +86,12 @@ export class DefaultTerminalProfileService implements TerminalProfileService {
     protected order: string[] = [];
     protected readonly stores: TerminalProfileStore[];
 
+    protected readonly onAddedEmitter: Emitter<string> = new Emitter();
+    protected readonly onRemovedEmitter: Emitter<string> = new Emitter();
+
+    onAdded: Event<string> = this.onAddedEmitter.event;
+    onRemoved: Event<string> = this.onRemovedEmitter.event;
+
     constructor(...stores: TerminalProfileStore[]) {
         this.stores = stores;
         for (const store of this.stores) {
@@ -111,13 +119,15 @@ export class DefaultTerminalProfileService implements TerminalProfileService {
             // the profile was removed, but it's still in the `order` array
             this.order.splice(index, 1);
             this.defaultProfileIndex = Math.max(0, Math.min(this.order.length - 1, index));
+            this.onRemovedEmitter.fire(id);
         }
     }
 
     handleAdded(id: string): void {
         const index = this.order.indexOf(id);
         if (index < 0) {
             this.order.push(id);
+            this.onAddedEmitter.fire(id);
         }
     }