Refactor process execution options to handle encoding and improve logging compatibility

Author: eleanorjboyd

src/client/common/process/rawProcessApis.ts

@@ -21,13 +21,9 @@ function getDefaultOptions<T extends ShellOptions | SpawnOptions>(options: T, de
     const defaultOptions = { ...options };
     const execOptions = defaultOptions as SpawnOptions;
     if (execOptions) {
-        execOptions.encoding =
-            typeof execOptions.encoding === 'string' && execOptions.encoding.length > 0
-                ? execOptions.encoding
-                : DEFAULT_ENCODING;
-        const { encoding } = execOptions;
-        delete execOptions.encoding;
-        execOptions.encoding = encoding;
+        // Handle encoding which can be string, null, or undefined from ExecOptions
+        const encoding = execOptions.encoding;
+        execOptions.encoding = typeof encoding === 'string' && encoding.length > 0 ? encoding : DEFAULT_ENCODING;
     }
     if (!defaultOptions.env || Object.keys(defaultOptions.env).length === 0) {
         const env = defaultEnv || process.env;
@@ -58,7 +54,12 @@ export function shellExec(
     const shellOptions = getDefaultOptions(options, defaultEnv);
     if (!options.doNotLog) {
         const processLogger = new ProcessLogger(new WorkspaceService());
-        processLogger.logProcess(command, undefined, shellOptions);
+        // Create compatible options for logging by ensuring encoding is not null
+        const logOptions: SpawnOptions = {
+            ...shellOptions,
+            encoding: shellOptions.encoding || undefined,
+        };
+        processLogger.logProcess(command, undefined, logOptions);
     }
     return new Promise((resolve, reject) => {
         // eslint-disable-next-line @typescript-eslint/no-explicit-any

src/client/ioc/container.ts

@@ -4,7 +4,7 @@
 import { EventEmitter } from 'events';
 import { Container, decorate, injectable, interfaces } from 'inversify';
 import { traceWarn } from '../logging';
-import { Abstract, IServiceContainer, Newable } from './types';
+import { IServiceContainer } from './types';
 
 // This needs to be done once, hence placed in a common location.
 // Used by UnitTestSockerServer and also the extension unit tests.
@@ -25,7 +25,7 @@ export class ServiceContainer implements IServiceContainer {
     }
 
     public getAll<T>(
-        serviceIdentifier: string | symbol | Newable<T> | Abstract<T>,
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
         name?: string | number | symbol | undefined,
     ): T[] {
         return name

src/client/ioc/serviceManager.ts

@@ -2,16 +2,14 @@
 // Licensed under the MIT License.
 import { Container, injectable, interfaces } from 'inversify';
 
-import { Abstract, ClassType, IServiceManager, Newable } from './types';
-
-type identifier<T> = string | symbol | Newable<T> | Abstract<T>;
+import { ClassType, IServiceManager } from './types';
 
 @injectable()
 export class ServiceManager implements IServiceManager {
     constructor(private container: Container) {}
 
     public add<T>(
-        serviceIdentifier: identifier<T>,
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
 
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         constructor: new (...args: any[]) => T,
@@ -38,12 +36,12 @@ export class ServiceManager implements IServiceManager {
         this.container.bind<interfaces.Factory<T>>(factoryIdentifier).toFactory<T>(factoryMethod);
     }
 
-    public addBinding<T1, T2>(from: identifier<T1>, to: identifier<T2>): void {
+    public addBinding<T1, T2>(from: interfaces.ServiceIdentifier<T1>, to: interfaces.ServiceIdentifier<T2>): void {
         this.container.bind(to).toService(from);
     }
 
     public addSingleton<T>(
-        serviceIdentifier: identifier<T>,
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
 
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         constructor: new (...args: any[]) => T,
@@ -64,7 +62,7 @@ export class ServiceManager implements IServiceManager {
     }
 
     public addSingletonInstance<T>(
-        serviceIdentifier: identifier<T>,
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
         instance: T,
         name?: string | number | symbol | undefined,
     ): void {
@@ -75,11 +73,14 @@ export class ServiceManager implements IServiceManager {
         }
     }
 
-    public get<T>(serviceIdentifier: identifier<T>, name?: string | number | symbol | undefined): T {
+    public get<T>(serviceIdentifier: interfaces.ServiceIdentifier<T>, name?: string | number | symbol | undefined): T {
         return name ? this.container.getNamed<T>(serviceIdentifier, name) : this.container.get<T>(serviceIdentifier);
     }
 
-    public tryGet<T>(serviceIdentifier: identifier<T>, name?: string | number | symbol | undefined): T | undefined {
+    public tryGet<T>(
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
+        name?: string | number | symbol | undefined,
+    ): T | undefined {
         try {
             return name
                 ? this.container.getNamed<T>(serviceIdentifier, name)
@@ -91,7 +92,10 @@ export class ServiceManager implements IServiceManager {
         return undefined;
     }
 
-    public getAll<T>(serviceIdentifier: identifier<T>, name?: string | number | symbol | undefined): T[] {
+    public getAll<T>(
+        serviceIdentifier: interfaces.ServiceIdentifier<T>,
+        name?: string | number | symbol | undefined,
+    ): T[] {
         return name
             ? this.container.getAllNamed<T>(serviceIdentifier, name)
             : this.container.getAll<T>(serviceIdentifier);