handle user cancellation for variables

packages/debug/src/browser/debug-session-manager.ts

@@ -257,8 +257,7 @@ export class DebugSessionManager {
                 context: options.workspaceFolderUri ? new URI(options.workspaceFolderUri) : undefined,
                 configurationSection: 'launch',
                 commandIdVariables,
-                configuration,
-                checkAllResolved: true
+                configuration
             });
 
             if (configuration) {

packages/task/src/browser/process/process-task-resolver.ts

@@ -62,20 +62,20 @@ export class ProcessTaskResolver implements TaskResolver {
         const result: ProcessTaskConfiguration = {
             ...processTaskConfig,
             command: await this.variableResolverService.resolve(processTaskConfig.command, variableResolverOptions),
-            args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, variableResolverOptions) : undefined,
+            args: processTaskConfig.args ? await this.variableResolverService.resolve(processTaskConfig.args, variableResolverOptions) : undefined,
             windows: processTaskConfig.windows ? {
                 command: await this.variableResolverService.resolve(processTaskConfig.windows.command, variableResolverOptions),
-                args: processTaskConfig.windows.args ? await this.variableResolverService.resolveArray(processTaskConfig.windows.args, variableResolverOptions) : undefined,
+                args: processTaskConfig.windows.args ? await this.variableResolverService.resolve(processTaskConfig.windows.args, variableResolverOptions) : undefined,
                 options: processTaskConfig.windows.options
             } : undefined,
             osx: processTaskConfig.osx ? {
                 command: await this.variableResolverService.resolve(processTaskConfig.osx.command, variableResolverOptions),
-                args: processTaskConfig.osx.args ? await this.variableResolverService.resolveArray(processTaskConfig.osx.args, variableResolverOptions) : undefined,
+                args: processTaskConfig.osx.args ? await this.variableResolverService.resolve(processTaskConfig.osx.args, variableResolverOptions) : undefined,
                 options: processTaskConfig.osx.options
             } : undefined,
             linux: processTaskConfig.linux ? {
                 command: await this.variableResolverService.resolve(processTaskConfig.linux.command, variableResolverOptions),
-                args: processTaskConfig.linux.args ? await this.variableResolverService.resolveArray(processTaskConfig.linux.args, variableResolverOptions) : undefined,
+                args: processTaskConfig.linux.args ? await this.variableResolverService.resolve(processTaskConfig.linux.args, variableResolverOptions) : undefined,
                 options: processTaskConfig.linux.options
             } : undefined,
             options: {

packages/variable-resolver/src/browser/common-variable-contribution.ts

@@ -25,6 +25,7 @@ import { ResourceContextKey } from '@theia/core/lib/browser/resource-context-key
 import { VariableInput } from './variable-input';
 import { QuickInputService, QuickPickValue } from '@theia/core/lib/browser';
 import { MaybeArray, RecursivePartial } from '@theia/core/lib/common/types';
+import { cancelled } from '@theia/core/lib/common/cancellation';
 
 @injectable()
 export class CommonVariableContribution implements VariableContribution {
@@ -76,14 +77,17 @@ export class CommonVariableContribution implements VariableContribution {
         });
         variables.registerVariable({
             name: 'command',
-            resolve: async (_, name, __, commandIdVariables, configuration) => {
-                let commandId = name;
-                if (name && commandIdVariables) {
-                    const mappedValue = commandIdVariables[name];
-                    commandId = mappedValue ? mappedValue : name;
+            resolve: async (contextUri, commandId, configurationSection, commandIdVariables, configuration) => {
+                if (commandId) {
+                    if (commandIdVariables?.[commandId]) {
+                        commandId = commandIdVariables[commandId];
+                    }
+                    const result = await this.commands.executeCommand(commandId, configuration);
+                    // eslint-disable-next-line no-null/no-null
+                    if (result === null) {
+                        throw cancelled();
+                    }
                 }
-                const result = commandId && await this.commands.executeCommand(commandId, configuration);
-                return result ? result : undefined;
             }
         });
         variables.registerVariable({

packages/variable-resolver/src/browser/variable-resolver-service.spec.ts

@@ -15,10 +15,9 @@
 // *****************************************************************************
 
 import * as chai from 'chai';
-import { Container, ContainerModule } from '@theia/core/shared/inversify';
-import { ILogger } from '@theia/core/lib/common';
-import { MockLogger } from '@theia/core/lib/common/test/mock-logger';
-import { Variable, VariableRegistry } from './variable';
+import { Container } from '@theia/core/shared/inversify';
+import { cancelled } from '@theia/core/lib/common';
+import { VariableRegistry } from './variable';
 import { VariableResolverService } from './variable-resolver-service';
 
 const expect = chai.expect;
@@ -31,37 +30,25 @@ before(() => {
 describe('variable-resolver-service', () => {
 
     let testContainer: Container;
-
-    before(() => {
-        testContainer = new Container();
-        const module = new ContainerModule((bind, unbind, isBound, rebind) => {
-            bind(ILogger).to(MockLogger);
-            bind(VariableRegistry).toSelf().inSingletonScope();
-            bind(VariableResolverService).toSelf();
-        });
-        testContainer.load(module);
-    });
-
     let variableRegistry: VariableRegistry;
     let variableResolverService: VariableResolverService;
 
     beforeEach(() => {
+        testContainer = new Container();
+        testContainer.bind(VariableRegistry).toSelf().inSingletonScope();
+        testContainer.bind(VariableResolverService).toSelf().inSingletonScope();
         variableRegistry = testContainer.get(VariableRegistry);
+        variableRegistry.registerVariable({
+            name: 'file',
+            description: 'current file',
+            resolve: () => Promise.resolve('package.json')
+        });
+        variableRegistry.registerVariable({
+            name: 'lineNumber',
+            description: 'current line number',
+            resolve: () => Promise.resolve('6')
+        });
         variableResolverService = testContainer.get(VariableResolverService);
-
-        const variables: Variable[] = [
-            {
-                name: 'file',
-                description: 'current file',
-                resolve: () => Promise.resolve('package.json')
-            },
-            {
-                name: 'lineNumber',
-                description: 'current line number',
-                resolve: () => Promise.resolve('6')
-            }
-        ];
-        variables.forEach(v => variableRegistry.registerVariable(v));
     });
 
     it('should resolve known variables in a text', async () => {
@@ -81,11 +68,16 @@ describe('variable-resolver-service', () => {
         expect(resolved).is.equal('workspace: ${workspaceRoot}; file: package.json; line: 6');
     });
 
-    it('should check if all variables are resolved', async () => {
-        const options = {
-            checkAllResolved: true
-        };
-        const resolved = await variableResolverService.resolve('workspace: ${command:testCommand}; file: ${file}; line: ${lineNumber}', options);
+    it('should return undefined when a variable throws with `cancelled()` while resolving', async () => {
+        variableRegistry.registerVariable({
+            name: 'command',
+            resolve: (contextUri, commandId) => {
+                if (commandId === 'testCommand') {
+                    throw cancelled();
+                }
+            }
+        });
+        const resolved = await variableResolverService.resolve('workspace: ${command:testCommand}; file: ${file}; line: ${lineNumber}');
         expect(resolved).equal(undefined);
     });
 });

packages/variable-resolver/src/browser/variable-resolver-service.ts

@@ -19,8 +19,8 @@
 import { injectable, inject } from '@theia/core/shared/inversify';
 import { VariableRegistry } from './variable';
 import URI from '@theia/core/lib/common/uri';
-import { JSONExt, ReadonlyJSONValue } from '@theia/core/shared/@phosphor/coreutils';
 import { CommandIdVariables } from '../common/variable-types';
+import { isCancelled } from '@theia/core';
 
 export interface VariableResolveOptions {
     context?: URI;
@@ -30,8 +30,6 @@ export interface VariableResolveOptions {
     configurationSection?: string;
     commandIdVariables?: CommandIdVariables;
     configuration?: unknown;
-    // Return 'undefined' if not all variables were successfully resolved.
-    checkAllResolved?: boolean;
 }
 
 /**
@@ -46,32 +44,35 @@ export class VariableResolverService {
 
     /**
      * Resolve the variables in the given string array.
-     * @param value The array of data to resolve
-     * @param options options of the variable resolution
-     * @returns promise resolved to the provided string array with already resolved variables.
-     * Never reject.
+     * @param value The array of data to resolve variables in.
+     * @param options Options of the variable resolution.
+     * @returns Promise to array with variables resolved. Never rejects.
+     *
+     * @deprecated since 1.28.0 use {@link resolve} instead.
      */
     resolveArray(value: string[], options: VariableResolveOptions = {}): Promise<string[] | undefined> {
         return this.resolve(value, options);
     }
 
     /**
-     * Resolve the variables in the given string.
-     * @param value Data to resolve
-     * @param options options of the variable resolution
-     * @returns promise resolved to the provided string with already resolved variables.
-     * Never reject.
+     * Resolve the variables for all strings found in the object and nested objects.
+     * @param value Data to resolve variables in.
+     * @param options Options of the variable resolution
+     * @returns Promise to object with variables resolved. Returns `undefined` if a variable resolution was cancelled.
      */
     async resolve<T>(value: T, options: VariableResolveOptions = {}): Promise<T | undefined> {
         const context = new VariableResolverService.Context(this.variableRegistry, options);
-        const resolved = await this.doResolve(value, context);
-        if (options.checkAllResolved && !context.allDefined()) {
-            return undefined;
+        try {
+            return await this.doResolve(value, context);
+        } catch (error) {
+            if (isCancelled(error)) {
+                return undefined;
+            }
+            throw error;
         }
-        return resolved as any;
     }
 
-    protected async doResolve(value: Object | undefined, context: VariableResolverService.Context): Promise<Object | undefined> {
+    protected async doResolve(value: any, context: VariableResolverService.Context): Promise<any> {
         // eslint-disable-next-line no-null/no-null
         if (value === undefined || value === null) {
             return value;
@@ -128,6 +129,7 @@ export class VariableResolverService {
     }
 }
 export namespace VariableResolverService {
+
     export class Context {
 
         protected readonly resolved = new Map<string, string | undefined>();
@@ -141,45 +143,43 @@ export namespace VariableResolverService {
             return this.resolved.get(name);
         }
 
-        allDefined(): boolean {
-            for (const value of this.resolved.values()) {
-                if (value === undefined) {
-                    return false;
-                }
-            }
-            return true;
-        }
-
         async resolve(name: string): Promise<void> {
             if (this.resolved.has(name)) {
                 return;
             }
             try {
                 let variableName = name;
                 let argument: string | undefined;
-                const parts = name.split(':');
+                const parts = name.split(':', 2);
                 if (parts.length > 1) {
                     variableName = parts[0];
                     argument = parts[1];
                 }
                 const variable = this.variableRegistry.getVariable(variableName);
-                const value =
-                    variable &&
-                    (await variable.resolve(
-                        this.options.context,
-                        argument,
-                        this.options.configurationSection,
-                        this.options.commandIdVariables,
-                        this.options.configuration
-                    ));
-                // eslint-disable-next-line no-null/no-null
-                const stringValue = value !== undefined && value !== null && JSONExt.isPrimitive(value as ReadonlyJSONValue) ? String(value) : undefined;
-                this.resolved.set(name, stringValue);
+                const resolved = await variable?.resolve(
+                    this.options.context,
+                    argument,
+                    this.options.configurationSection,
+                    this.options.commandIdVariables,
+                    this.options.configuration
+                );
+                if (
+                    typeof resolved === 'bigint' ||
+                    typeof resolved === 'boolean' ||
+                    typeof resolved === 'number' ||
+                    typeof resolved === 'string'
+                ) {
+                    this.resolved.set(name, `${resolved}`);
+                } else {
+                    this.resolved.set(name, undefined);
+                }
             } catch (e) {
-                console.error(`Failed to resolved '${name}' variable`, e);
+                if (isCancelled(e)) {
+                    throw e;
+                }
                 this.resolved.set(name, undefined);
+                console.error(`Failed to resolve '${name}' variable:`, e);
             }
         }
-
     }
 }

packages/variable-resolver/src/browser/variable.ts

@@ -45,7 +45,8 @@ export interface Variable {
         configurationSection?: string,
         commandIdVariables?: CommandIdVariables,
         configuration?: unknown
-    ): MaybePromise<Object | undefined>;
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    ): MaybePromise<any>;
 }
 
 export const VariableContribution = Symbol('VariableContribution');