refactor: adjust attachByLaunchConfig params (#811)

* refactor: adjust attachByLaunchConfig params

* chore: update comments

* test: fix test case

* chore: make params optional

* chore: add missing typing

* test: fix test case

Author: bytemain

packages/terminal-next/__tests__/browser/inject.ts

@@ -188,9 +188,6 @@ export const injector = new Injector([
       $resolveUnixShellPath(p) {
         return p;
       },
-      $resolvePotentialUnixShellPath() {
-        return 'detectedBash';
-      },
     },
   },
 ]);

packages/terminal-next/__tests__/browser/terminal.service.test.ts

@@ -93,9 +93,6 @@ describe('terminal service test cases', () => {
         $resolveUnixShellPath(p) {
           return p;
         },
-        $resolvePotentialUnixShellPath() {
-          return 'detectedBash';
-        },
       },
     });
 
@@ -123,38 +120,24 @@ describe('terminal service test cases', () => {
     expect(windowClientId).toMatch(/^test-window-client-id.*/);
   });
 
-  it('[attach] should be valid launchConfig with a valid shell path and ignore type', async () => {
-    await terminalService.attach(
+  it('[attachByLaunchConfig] should be valid launchConfig with a valid shell path and ignore type', async () => {
+    await terminalService.attachByLaunchConfig(
       sessionId,
-      {} as any,
       200,
       200,
       {
-        shellPath,
+        executable: shellPath,
       },
-      'asdasd',
+      {} as any,
     );
     expect(launchConfig?.executable).toEqual(shellPath);
   });
-  it('[attach] should be valid launchConfig with empty type or default', async () => {
-    await terminalService.attach(sessionId, {} as any, 200, 200, {}, '');
-    expect(launchConfig?.executable).toEqual('detectedBash');
-    await terminalService.attach(sessionId, {} as any, 200, 200, {}, 'default');
-    expect(launchConfig?.executable).toEqual('detectedBash');
-  });
-
-  it('[attach] should be valid launchConfig with specific type', async () => {
-    await terminalService.attach(sessionId, {} as any, 200, 200, {}, 'bash');
-    expect(launchConfig?.executable).toEqual('bash');
-    await terminalService.attach(sessionId, {} as any, 200, 200, {}, 'asdasdasdasd');
-    expect(launchConfig?.executable).toEqual('asdasdasdasd');
-  });
 
   it('[attachByLaunchConfig] can launch valid config', async () => {
     const launchConfig: IShellLaunchConfig = {
       executable: shellPath,
     };
-    await terminalService.attachByLaunchConfig(sessionId, 200, 200, launchConfig);
+    await terminalService.attachByLaunchConfig(sessionId, 200, 200, launchConfig, {} as any);
     expect(launchConfig?.executable).toEqual(shellPath);
   });
 
@@ -163,7 +146,7 @@ describe('terminal service test cases', () => {
       const launchConfig: IShellLaunchConfig = {
         executable: shellPath,
       };
-      terminalService.attachByLaunchConfig(sessionId, 200, 200, launchConfig);
+      terminalService.attachByLaunchConfig(sessionId, 200, 200, launchConfig, {} as any);
       terminalService.onProcessChange((e) => {
         expect(e.sessionId).toBe(sessionId);
         expect(e.processName).toBe('zsh');

packages/terminal-next/src/browser/terminal.client.ts

@@ -586,7 +586,13 @@ export class TerminalClient extends Disposable implements ITerminalClient {
     this.logger.log('attach terminal by launchConfig: ', this._launchConfig);
 
     try {
-      connection = await this.internalService.attachByLaunchConfig(sessionId, cols, rows, this._launchConfig);
+      connection = await this.internalService.attachByLaunchConfig(
+        sessionId,
+        cols,
+        rows,
+        this._launchConfig,
+        this.xterm,
+      );
     } catch (e) {
       this.logger.error(`attach ${sessionId} terminal failed, _launchConfig`, JSON.stringify(this._launchConfig), e);
     }

packages/terminal-next/src/browser/terminal.internal.service.ts

@@ -10,12 +10,12 @@ import {
   ITerminalError,
   IPtyExitEvent,
   ITerminalController,
-  TerminalOptions,
   ITerminalProfile,
   IShellLaunchConfig,
   ITerminalConnection,
   IPtyProcessChangeEvent,
 } from '../common';
+import { IXTerm } from '../common/xterm';
 
 import { TerminalProcessExtHostProxy } from './terminal.ext.host.proxy';
 
@@ -45,32 +45,6 @@ export class TerminalInternalService implements ITerminalInternalService {
     return this._processExtHostProxies.get(id);
   }
 
-  async attach(
-    sessionId: string,
-    xterm: XTermTerminal,
-    rows: number,
-    cols: number,
-    options: TerminalOptions = {},
-    type: string,
-  ) {
-    if (options.isExtensionTerminal) {
-      const proxy = new TerminalProcessExtHostProxy(sessionId, cols, rows, this.controller);
-      proxy.start();
-      proxy.onProcessExit(() => {
-        this._processExtHostProxies.delete(sessionId);
-      });
-      this._processExtHostProxies.set(sessionId, proxy);
-      return {
-        name: options.name || 'ExtensionTerminal-' + sessionId,
-        readonly: false,
-        onData: proxy.onProcessData.bind(proxy),
-        sendData: proxy.input.bind(proxy),
-        onExit: proxy.onProcessExit.bind(proxy),
-      };
-    }
-    return this.service.attach(sessionId, xterm, rows, cols, options, type);
-  }
-
   async sendText(sessionId: string, message: string) {
     const proxy = this._getExtHostProxy(sessionId);
     if (proxy) {
@@ -137,6 +111,7 @@ export class TerminalInternalService implements ITerminalInternalService {
     cols: number,
     rows: number,
     launchConfig: IShellLaunchConfig,
+    xterm: IXTerm,
   ): Promise<ITerminalConnection | undefined> {
     if (launchConfig.customPtyImplementation) {
       const proxy = launchConfig.customPtyImplementation(sessionId, cols, rows) as TerminalProcessExtHostProxy;
@@ -153,6 +128,6 @@ export class TerminalInternalService implements ITerminalInternalService {
         onExit: proxy.onProcessExit.bind(proxy),
       };
     }
-    return await this.service.attachByLaunchConfig(sessionId, cols, rows, launchConfig);
+    return await this.service.attachByLaunchConfig(sessionId, cols, rows, launchConfig, xterm);
   }
 }

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

@@ -26,6 +26,8 @@ import {
 import { CodeTerminalSettingPrefix } from '../common/preference';
 import { ShellType, WindowsShellType } from '../common/shell';
 
+import { XTerm } from './xterm';
+
 export interface EventMessage {
   data: string;
 }
@@ -100,93 +102,13 @@ export class NodePtyTerminalService implements ITerminalService {
     ptyInstance: pty,
   });
 
-  /**
-   *
-   * @param sessionId
-   * @param _
-   * @param rows
-   * @param cols
-   * @param options
-   * @param shellType
-   * @returns
-   */
-  async attach(
+  async attachByLaunchConfig(
     sessionId: string,
-    _: Terminal,
-    rows: number,
     cols: number,
-    options: TerminalOptions = {},
-    shellType?: ShellType,
+    rows: number,
+    launchConfig: IShellLaunchConfig,
+    _xterm: XTerm,
   ) {
-    let shellPath = options.shellPath;
-    const shellArgs = typeof options.shellArgs === 'string' ? [options.shellArgs] : options.shellArgs || [];
-    const platformKey = await this.getCodePlatformKey();
-    const terminalOs = await this.getOs();
-    if (!shellPath) {
-      // if terminal options.shellPath is not set, we should resolve the shell path from preference: `terminal.type`
-      if (shellType && shellType !== 'default') {
-        if (terminalOs === OperatingSystem.Windows) {
-          shellPath = await this.serviceClientRPC.$resolveWindowsShellPath(shellType as WindowsShellType);
-        } else {
-          shellPath = await this.serviceClientRPC.$resolveUnixShellPath(shellType);
-        }
-
-        if (!shellPath) {
-          // TODO: we can show error message here
-          // "the shell you want to launch is not exists"
-        }
-      }
-
-      // and now, we have the following two situations:
-      if (!shellPath) {
-        if (!shellType || shellType === 'default') {
-          // 1. `terminal.type` is set to a falsy value, or set to `default`
-          if (terminalOs === OperatingSystem.Windows) {
-            // in windows, at least we can launch the cmd.exe
-            const { type: _type, path } = await this.serviceClientRPC.$resolvePotentialWindowsShellPath();
-            shellType = _type;
-            shellPath = path;
-          } else {
-            // in unix, at least we can launch the sh
-            shellPath = await this.serviceClientRPC.$resolvePotentialUnixShellPath();
-          }
-        } else {
-          // 2. `terminal.type` is set to a truthy value, but the shell path is not resolved, for example cannot resolve 'git-bash'
-          //     but in this situation, we preserve the user settings, launch the type as shell path
-          //     on PtyService we also have a fallback to check the shellPath is valid
-          shellPath = shellType;
-        }
-      }
-
-      // if we still can not find the shell path, we use shellType as the target shell path
-      if (!shellPath && shellType !== 'default') {
-        shellPath = shellType;
-      }
-
-      const platformSpecificArgs = this.preferenceService.get<string[]>(
-        `${CodeTerminalSettingPrefix.ShellArgs}${platformKey}`,
-        [],
-      );
-
-      shellArgs.push(...platformSpecificArgs);
-
-      if (shellType === WindowsShellType['git-bash']) {
-        shellArgs.push('--login');
-      }
-    }
-
-    const launchConfig: IShellLaunchConfig = {
-      executable: shellPath,
-      cwd: options.cwd,
-      args: shellArgs,
-      env: options.env,
-      name: options.name,
-      strictEnv: options.strictEnv,
-    };
-    return this.attachByLaunchConfig(sessionId, cols, rows, launchConfig);
-  }
-
-  async attachByLaunchConfig(sessionId: string, cols: number, rows: number, launchConfig: IShellLaunchConfig) {
     // If code runs to here, it means that we want to create a real terminal.
     // So if `launchConfig.executable` is not set, we should use the default shell.
     if (!launchConfig.executable) {

packages/terminal-next/src/browser/xterm.ts

@@ -10,6 +10,7 @@ import { WorkbenchThemeService } from '@opensumi/ide-theme/lib/browser/workbench
 import { IThemeService } from '@opensumi/ide-theme/lib/common/theme.service';
 
 import { SupportedOptions } from '../common/preference';
+import { IXTerm } from '../common/xterm';
 
 import styles from './component/terminal.module.less';
 import {
@@ -30,7 +31,7 @@ export interface XTermOptions {
 }
 
 @Injectable({ multiple: true })
-export class XTerm extends Disposable {
+export class XTerm extends Disposable implements IXTerm {
   @Autowired(INJECTOR_TOKEN)
   protected readonly injector: Injector;
 

packages/terminal-next/src/common/pty.ts

@@ -228,8 +228,6 @@ export interface ITerminalServiceClient {
   ensureTerminal(terminalIdArr: string[]): boolean;
   $resolveWindowsShellPath(type: WindowsShellType): Promise<string | undefined>;
   $resolveUnixShellPath(type: string): Promise<string | undefined>;
-  $resolvePotentialUnixShellPath(): Promise<string | undefined>;
-  $resolvePotentialWindowsShellPath(): Promise<{ path: string; type: WindowsShellType }>;
   $resolveShellPath(paths: string[]): Promise<string | undefined>;
   detectAvailableProfiles(options: IDetectProfileOptions): Promise<ITerminalProfile[]>;
   getDefaultSystemShell(os: OperatingSystem): Promise<string>;

packages/terminal-next/src/common/service.ts

@@ -7,6 +7,7 @@ import { ITerminalConnection } from './client';
 import { ITerminalError } from './error';
 import { ITerminalProfile } from './profile';
 import { IShellLaunchConfig, TerminalOptions } from './pty';
+import { IXTerm } from './xterm';
 
 export interface IPtyExitEvent {
   sessionId: string;
@@ -37,28 +38,20 @@ export interface ITerminalService {
    * @param sessionIds
    */
   check?(sessionIds: string[]): Promise<boolean>;
+
   /**
-   * @deprecated will remove on 2.17.0, please use `attachByLaunchConfig` instead
    * @param sessionId 会话唯一标识
-   * @param xterm 返回的 Xterm 终端实例
    * @param rows 终端初始化使用的列数
    * @param cols 终端初始化使用的行数
-   * @param options 创建一个新终端的进程选项
-   * @param shellType 终端类型
+   * @param launchConfig 创建一个新终端的进程选项
+   * @param xterm 创建的 xterm 实例
    */
-  attach(
-    sessionId: string,
-    xterm: Terminal,
-    cols: number,
-    rows: number,
-    options?: TerminalOptions,
-    shellType?: string,
-  ): Promise<ITerminalConnection | undefined>;
   attachByLaunchConfig(
     sessionId: string,
     cols: number,
     rows: number,
     launchConfig: IShellLaunchConfig,
+    xterm?: IXTerm,
   ): Promise<ITerminalConnection | undefined>;
   /**
    *

packages/terminal-next/src/common/xterm.ts

@@ -0,0 +1,18 @@
+import { ITerminalOptions, ITheme, Terminal } from 'xterm';
+
+import { SupportedOptions } from './preference';
+
+export interface IXTerm {
+  raw: Terminal;
+  container: HTMLDivElement;
+  xtermOptions: ITerminalOptions & SupportedOptions;
+
+  copySelection(): Promise<void>;
+  onSelectionChange(): Promise<void>;
+  open(): void;
+  fit(): void;
+  findNext(text: string): boolean;
+  closeSearch(): void;
+  updatePreferences(options: SupportedOptions): void;
+  updateTheme(theme: ITheme | undefined): void;
+}

packages/terminal-next/src/node/terminal.service.client.ts

@@ -129,42 +129,6 @@ export class TerminalServiceClientImpl extends RPCService<IRPCTerminalService> i
     return await findShellExecutableAsync(paths);
   }
 
-  async $resolvePotentialUnixShellPath(): Promise<string | undefined> {
-    if (process.env.SHELL) {
-      return process.env.SHELL;
-    }
-
-    const candidates = ['zsh', 'bash', 'sh'];
-    for (const candidate of candidates) {
-      const path = await this.$resolveUnixShellPath(candidate);
-      if (path) {
-        return path;
-      }
-    }
-  }
-
-  async $resolvePotentialWindowsShellPath(): Promise<{ path: string; type: WindowsShellType }> {
-    let path = await findShellExecutableAsync(WINDOWS_GIT_BASH_PATHS);
-    if (path) {
-      return {
-        path,
-        type: WindowsShellType['git-bash'],
-      };
-    }
-    path = await findExecutable(WINDOWS_DEFAULT_SHELL_PATH_MAPS.powershell);
-    if (path) {
-      return {
-        path,
-        type: WindowsShellType.powershell,
-      };
-    }
-
-    return {
-      path: WINDOWS_DEFAULT_SHELL_PATH_MAPS.cmd,
-      type: WindowsShellType.cmd,
-    };
-  }
-
   async detectAvailableProfiles(options: IDetectProfileOptions): Promise<ITerminalProfile[]> {
     return await this.terminalProfileService.detectAvailableProfiles(options);
   }