Skip to content

Commit

Permalink
debug/variables: autofill input from an LRU (#224531)
Browse files Browse the repository at this point in the history 
* debug: improve behavior for slow-running prelaunch tasks

Shows this notification if a prelaunch task has been running for more
than 10 seconds:

![](https://memes.peet.io/img/24-07-be15cd9d-c9cd-40dc-8937-9d62466e5c96.png)

Also does a little cleanup of some probably-leaking disposables in the task runner.

Fixes #218267

* hygenie

* debug/variables: autofill input from an LRU

This keeps a small LRU in the config resolver and storage service to provide default values, when not otherwise specified, for inputs in e.g. debugging.

I added this after I was annoyed by having to keep autofilling the same ephemeral value in some debug configs earlier.
  • Loading branch information
@connor4312
connor4312 authored Aug 1, 2024
1 parent a3bdeb3 commit 930be4a
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 16 deletions.
43 changes: 39 additions & 4 deletions src/vs/workbench/services/configurationResolver/browser/baseConfigurationResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { Queue } from 'vs/base/common/async';
import { IStringDictionary } from 'vs/base/common/collections';
import { LRUCache } from 'vs/base/common/map';
import { Schemas } from 'vs/base/common/network';
import { IProcessEnvironment } from 'vs/base/common/platform';
import * as Types from 'vs/base/common/types';
Expand All @@ -14,6 +15,7 @@ import { ICommandService } from 'vs/platform/commands/common/commands';
import { ConfigurationTarget, IConfigurationOverrides, IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { ILabelService } from 'vs/platform/label/common/label';
import { IInputOptions, IPickOptions, IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
import { IWorkspaceContextService, IWorkspaceFolder, WorkbenchState } from 'vs/platform/workspace/common/workspace';
import { EditorResourceAccessor, SideBySideEditor } from 'vs/workbench/common/editor';
import { ConfiguredInput } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
Expand All @@ -22,6 +24,9 @@ import { IEditorService } from 'vs/workbench/services/editor/common/editorServic
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IPathService } from 'vs/workbench/services/path/common/pathService';

const LAST_INPUT_STORAGE_KEY = 'configResolveInputLru';
const LAST_INPUT_CACHE_SIZE = 5;

export abstract class BaseConfigurationResolverService extends AbstractVariableResolverService {

static readonly INPUT_OR_COMMAND_VARIABLES_PATTERN = /\${((input|command):(.*?))}/g;
Expand All @@ -42,6 +47,7 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
private readonly labelService: ILabelService,
private readonly pathService: IPathService,
extensionService: IExtensionService,
private readonly storageService: IStorageService,
) {
super({
getFolderUri: (folderName: string): uri | undefined => {
Expand Down Expand Up @@ -214,7 +220,7 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
switch (type) {

case 'input':
result = await this.showUserInput(name, inputs);
result = await this.showUserInput(section, name, inputs);
break;

case 'command': {
Expand Down Expand Up @@ -282,7 +288,7 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
* @param variable Name of the input variable.
* @param inputInfos Information about each possible input variable.
*/
private showUserInput(variable: string, inputInfos: ConfiguredInput[]): Promise<string | undefined> {
private showUserInput(section: string | undefined, variable: string, inputInfos: ConfiguredInput[]): Promise<string | undefined> {

if (!inputInfos) {
return Promise.reject(new Error(nls.localize('inputVariable.noInputSection', "Variable '{0}' must be defined in an '{1}' section of the debug or task configuration.", variable, 'inputs')));
Expand All @@ -296,20 +302,27 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
throw new Error(nls.localize('inputVariable.missingAttribute', "Input variable '{0}' is of type '{1}' and must include '{2}'.", variable, info.type, attrName));
};

const defaultValueMap = this.readInputLru();
const defaultValueKey = `${section}.${variable}`;
const previousPickedValue = defaultValueMap.get(defaultValueKey);

switch (info.type) {

case 'promptString': {
if (!Types.isString(info.description)) {
missingAttribute('description');
}
const inputOptions: IInputOptions = { prompt: info.description, ignoreFocusLost: true };
const inputOptions: IInputOptions = { prompt: info.description, ignoreFocusLost: true, value: previousPickedValue };
if (info.default) {
inputOptions.value = info.default;
}
if (info.password) {
inputOptions.password = info.password;
}
return this.userInputAccessQueue.queue(() => this.quickInputService.input(inputOptions)).then(resolvedInput => {
if (typeof resolvedInput === 'string') {
this.storeInputLru(defaultValueMap.set(defaultValueKey, resolvedInput));
}
return resolvedInput as string;
});
}
Expand Down Expand Up @@ -344,14 +357,18 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
if (value === info.default) {
item.description = nls.localize('inputVariable.defaultInputValue', "(Default)");
picks.unshift(item);
} else if (!info.default && value === previousPickedValue) {
picks.unshift(item);
} else {
picks.push(item);
}
}
const pickOptions: IPickOptions<PickStringItem> = { placeHolder: info.description, matchOnDetail: true, ignoreFocusLost: true };
return this.userInputAccessQueue.queue(() => this.quickInputService.pick(picks, pickOptions, undefined)).then(resolvedInput => {
if (resolvedInput) {
return (resolvedInput as PickStringItem).value;
const value = (resolvedInput as PickStringItem).value;
this.storeInputLru(defaultValueMap.set(defaultValueKey, value));
return value;
}
return undefined;
});
Expand All @@ -375,4 +392,22 @@ export abstract class BaseConfigurationResolverService extends AbstractVariableR
}
return Promise.reject(new Error(nls.localize('inputVariable.undefinedVariable', "Undefined input variable '{0}' encountered. Remove or define '{0}' to continue.", variable)));
}

private storeInputLru(lru: LRUCache<string, string>): void {
this.storageService.store(LAST_INPUT_STORAGE_KEY, JSON.stringify(lru.toJSON()), StorageScope.WORKSPACE, StorageTarget.MACHINE);
}

private readInputLru(): LRUCache<string, string> {
const contents = this.storageService.get(LAST_INPUT_STORAGE_KEY, StorageScope.WORKSPACE);
const lru = new LRUCache<string, string>(LAST_INPUT_CACHE_SIZE);
try {
if (contents) {
lru.fromJSON(JSON.parse(contents));
}
} catch {
// ignored
}

return lru;
}
}
4 changes: 3 additions & 1 deletion src/vs/workbench/services/configurationResolver/browser/configurationResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { ILabelService } from 'vs/platform/label/common/label';
import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput';
import { IStorageService } from 'vs/platform/storage/common/storage';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { BaseConfigurationResolverService } from 'vs/workbench/services/configurationResolver/browser/baseConfigurationResolverService';
import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
Expand All @@ -26,10 +27,11 @@ export class ConfigurationResolverService extends BaseConfigurationResolverServi
@ILabelService labelService: ILabelService,
@IPathService pathService: IPathService,
@IExtensionService extensionService: IExtensionService,
@IStorageService storageService: IStorageService,
) {
super({ getAppRoot: () => undefined, getExecPath: () => undefined },
Promise.resolve(Object.create(null)), editorService, configurationService,
commandService, workspaceContextService, quickInputService, labelService, pathService, extensionService);
commandService, workspaceContextService, quickInputService, labelService, pathService, extensionService, storageService);
}
}

Expand Down
4 changes: 3 additions & 1 deletion ...workbench/services/configurationResolver/electron-sandbox/configurationResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ILabelService } from 'vs/platform/label/common/label';
import { IShellEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/shellEnvironmentService';
import { IPathService } from 'vs/workbench/services/path/common/pathService';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IStorageService } from 'vs/platform/storage/common/storage';

export class ConfigurationResolverService extends BaseConfigurationResolverService {

Expand All @@ -30,6 +31,7 @@ export class ConfigurationResolverService extends BaseConfigurationResolverServi
@IShellEnvironmentService shellEnvironmentService: IShellEnvironmentService,
@IPathService pathService: IPathService,
@IExtensionService extensionService: IExtensionService,
@IStorageService storageService: IStorageService,
) {
super({
getAppRoot: (): string | undefined => {
Expand All @@ -39,7 +41,7 @@ export class ConfigurationResolverService extends BaseConfigurationResolverServi
return environmentService.execPath;
},
}, shellEnvironmentService.getShellEnv(), editorService, configurationService, commandService,
workspaceContextService, quickInputService, labelService, pathService, extensionService);
workspaceContextService, quickInputService, labelService, pathService, extensionService, storageService);
}
}

Expand Down
20 changes: 10 additions & 10 deletions ...services/configurationResolver/test/electron-sandbox/configurationResolverService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { IConfigurationResolverService } from 'vs/workbench/services/configurati
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IPathService } from 'vs/workbench/services/path/common/pathService';
import { TestEditorService, TestQuickInputService } from 'vs/workbench/test/browser/workbenchTestServices';
import { TestContextService, TestExtensionService } from 'vs/workbench/test/common/workbenchTestServices';
import { TestContextService, TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';

const mockLineNumber = 10;
class TestEditorServiceWithActiveEditor extends TestEditorService {
Expand Down Expand Up @@ -84,7 +84,7 @@ suite('Configuration Resolver Service', () => {
extensionService = new TestExtensionService();
containingWorkspace = testWorkspace(URI.parse('file:///VSCode/workspaceLocation'));
workspace = containingWorkspace.folders[0];
configurationResolverService = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), editorService, new MockInputsConfigurationService(), mockCommandService, new TestContextService(containingWorkspace), quickInputService, labelService, pathService, extensionService);
configurationResolverService = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), editorService, new MockInputsConfigurationService(), mockCommandService, new TestContextService(containingWorkspace), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
});

teardown(() => {
Expand Down Expand Up @@ -230,7 +230,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${config:editor.fontFamily} xyz'), 'abc foo xyz');
});

Expand All @@ -241,7 +241,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
assert.strictEqual(await service.resolveAsync(undefined, 'abc ${config:editor.fontFamily} xyz'), 'abc foo xyz');
});

Expand All @@ -257,7 +257,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${config:editor.fontFamily} ${config:terminal.integrated.fontFamily} xyz'), 'abc foo bar xyz');
});

Expand All @@ -273,7 +273,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
if (platform.isWindows) {
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${config:editor.fontFamily} ${workspaceFolder} ${env:key1} xyz'), 'abc foo \\VSCode\\workspaceLocation Value for key1 xyz');
} else {
Expand All @@ -293,7 +293,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
if (platform.isWindows) {
assert.strictEqual(await service.resolveAsync(workspace, '${config:editor.fontFamily} ${config:terminal.integrated.fontFamily} ${workspaceFolder} - ${workspaceFolder} ${env:key1} - ${env:key2}'), 'foo bar \\VSCode\\workspaceLocation - \\VSCode\\workspaceLocation Value for key1 - Value for key2');
} else {
Expand Down Expand Up @@ -326,7 +326,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${config:editor.fontFamily} ${config:editor.lineNumbers} ${config:editor.insertSpaces} xyz'), 'abc foo 123 false xyz');
});

Expand All @@ -335,7 +335,7 @@ suite('Configuration Resolver Service', () => {
editor: {}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${unknownVariable} xyz'), 'abc ${unknownVariable} xyz');
assert.strictEqual(await service.resolveAsync(workspace, 'abc ${env:unknownVariable} xyz'), 'abc xyz');
});
Expand All @@ -347,7 +347,7 @@ suite('Configuration Resolver Service', () => {
}
});

const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService);
const service = new TestConfigurationResolverService(nullContext, Promise.resolve(envVariables), disposables.add(new TestEditorServiceWithActiveEditor()), configurationService, mockCommandService, new TestContextService(), quickInputService, labelService, pathService, extensionService, disposables.add(new TestStorageService()));

assert.rejects(async () => await service.resolveAsync(workspace, 'abc ${env} xyz'));
assert.rejects(async () => await service.resolveAsync(workspace, 'abc ${env:} xyz'));
Expand Down

0 comments on commit 930be4a

Please sign in to comment.