Skip to content

Commit

Permalink
Add button to clear list and refresh in interpreters quickpick (#19628)
Browse files Browse the repository at this point in the history 
* Change refresh icon when interpreter list is refreshing

* Add tests

* Minor tweaks

* Fix situation if dialog box is cancelled

* Fix tests

* Improve ignoreErrors() typing

* Update to not use custom svg, instead use built VSCode icon

* Add vscode mock

* Add button to clear interpreters list and refresh in quickpick
  • Loading branch information
Kartik Raj authored Aug 4, 2022
1 parent 93fd4e8 commit 2379238
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 46 deletions.
6 changes: 6 additions & 0 deletions src/client/common/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ export namespace Octicons {
export const Lightbulb = '$(lightbulb)';
}

/**
* Look at https://code.visualstudio.com/api/references/icons-in-labels#icon-listing for ThemeIcon ids.
* Using a theme icon is preferred over a custom icon as it gives product theme authors the possibility
* to change the icons.
*/
export namespace ThemeIcons {
export const ClearAll = 'clear-all';
export const Refresh = 'refresh';
export const SpinningLoader = 'loading~spin';
}
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/utils/localize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ export namespace InterpreterQuickPickList {
'InterpreterQuickPickList.refreshInterpreterList',
'Refresh Interpreter list',
);
export const clearAllAndRefreshInterpreterList = localize(
'InterpreterQuickPickList.clearAllAndRefreshInterpreterList',
'Clear all and Refresh Interpreter list',
);
export const refreshingInterpreterList = localize(
'InterpreterQuickPickList.refreshingInterpreterList',
'Refreshing Interpreter list...',
Expand Down
21 changes: 14 additions & 7 deletions src/client/common/utils/multiStepInput.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export interface IQuickPickParameters<T extends QuickPickItem, E = any> {
items: T[];
activeItem?: T;
placeholder: string;
customButtonSetup?: QuickInputButtonSetup;
customButtonSetups?: QuickInputButtonSetup[];
matchOnDescription?: boolean;
matchOnDetail?: boolean;
keepScrollPosition?: boolean;
Expand Down Expand Up @@ -86,7 +86,7 @@ export interface IMultiStepInput<S> {
items,
activeItem,
placeholder,
customButtonSetup,
customButtonSetups,
}: P): Promise<MultiStepInputQuickPicResponseType<T, P>>;
showInputBox<P extends InputBoxParameters>({
title,
Expand Down Expand Up @@ -117,7 +117,7 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
items,
activeItem,
placeholder,
customButtonSetup,
customButtonSetups,
matchOnDescription,
matchOnDetail,
acceptFilterBoxTextAsSelection,
Expand Down Expand Up @@ -145,15 +145,22 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
input.activeItems = [];
}
input.buttons = this.steps.length > 1 ? [QuickInputButtons.Back] : [];
if (customButtonSetup) {
input.buttons = [...input.buttons, customButtonSetup.button];
if (customButtonSetups) {
for (const customButtonSetup of customButtonSetups) {
input.buttons = [...input.buttons, customButtonSetup.button];
}
}
disposables.push(
input.onDidTriggerButton(async (item) => {
if (item === QuickInputButtons.Back) {
reject(InputFlowAction.back);
} else if (JSON.stringify(item) === JSON.stringify(customButtonSetup?.button)) {
await customButtonSetup?.callback(input);
}
if (customButtonSetups) {
for (const customButtonSetup of customButtonSetups) {
if (JSON.stringify(item) === JSON.stringify(customButtonSetup?.button)) {
await customButtonSetup?.callback(input);
}
}
}
}),
input.onDidChangeSelection((selectedItems) => resolve(selectedItems[0])),
Expand Down
58 changes: 38 additions & 20 deletions src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
alwaysShow: true,
};

private readonly refreshButton = {
iconPath: new ThemeIcon(ThemeIcons.Refresh),
tooltip: InterpreterQuickPickList.refreshInterpreterList,
};

private readonly hardRefreshButton = {
iconPath: new ThemeIcon(ThemeIcons.ClearAll),
tooltip: InterpreterQuickPickList.clearAllAndRefreshInterpreterList,
};

private readonly noPythonInstalled: ISpecialQuickPickItem = {
label: `${Octicons.Error} ${InterpreterQuickPickList.noPythonInstalled}`,
detail: InterpreterQuickPickList.clickForInstructions,
Expand Down Expand Up @@ -126,10 +136,6 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
// times so that the visible items do not change.
const preserveOrderWhenFiltering = !!this.interpreterService.refreshPromise;
const suggestions = this._getItems(state.workspace);
const refreshButton = {
iconPath: new ThemeIcon(ThemeIcons.Refresh),
tooltip: InterpreterQuickPickList.refreshInterpreterList,
};
state.path = undefined;
const currentInterpreterPathDisplay = this.pathUtils.getDisplayName(
this.configurationService.getSettings(state.workspace).pythonPath,
Expand All @@ -148,23 +154,20 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
matchOnDetail: true,
matchOnDescription: true,
title: InterpreterQuickPickList.browsePath.openButtonLabel,
customButtonSetup: {
button: refreshButton,
callback: (quickpickInput) => {
quickpickInput.buttons = [
{
iconPath: new ThemeIcon(ThemeIcons.SpinningLoader),
tooltip: InterpreterQuickPickList.refreshingInterpreterList,
},
];
this.interpreterService
.triggerRefresh()
.finally(() => {
quickpickInput.buttons = [refreshButton];
})
.ignoreErrors();
customButtonSetups: [
{
button: this.hardRefreshButton,
callback: (quickpickInput) => {
this.refreshButtonCallback(quickpickInput, true);
},
},
},
{
button: this.refreshButton,
callback: (quickpickInput) => {
this.refreshButtonCallback(quickpickInput, false);
},
},
],
initialize: () => {
// Note discovery is no longer guranteed to be auto-triggered on extension load, so trigger it when
// user interacts with the interpreter picker but only once per session. Users can rely on the
Expand Down Expand Up @@ -415,6 +418,21 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
}
}

private refreshButtonCallback(input: QuickPick<QuickPickItem>, clearCache: boolean) {
input.buttons = [
{
iconPath: new ThemeIcon(ThemeIcons.SpinningLoader),
tooltip: InterpreterQuickPickList.refreshingInterpreterList,
},
];
this.interpreterService
.triggerRefresh(undefined, { clearCache })
.finally(() => {
input.buttons = [this.hardRefreshButton, this.refreshButton];
})
.ignoreErrors();
}

@captureTelemetry(EventName.SELECT_INTERPRETER_ENTER_BUTTON)
public async _enterOrBrowseInterpreterPath(
input: IMultiStepInput<InterpreterStateArgs>,
Expand Down
45 changes: 26 additions & 19 deletions src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
WorkspaceFolder,
} from 'vscode';
import { cloneDeep } from 'lodash';
import { anything, instance, mock, verify, when } from 'ts-mockito';
import { anything, instance, mock, when } from 'ts-mockito';
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../../client/common/application/types';
import { PathUtils } from '../../../../client/common/platform/pathUtils';
import { IPlatformService } from '../../../../client/common/platform/types';
Expand Down Expand Up @@ -261,10 +261,10 @@ suite('Set Interpreter Command', () => {
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);

expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
const refreshButtonCallback = actualParameters!.customButtonSetup?.callback;
expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set');
const refreshButtons = actualParameters!.customButtonSetups;
expect(refreshButtons).to.not.equal(undefined, 'Callback not set');
delete actualParameters!.initialize;
delete actualParameters!.customButtonSetup;
delete actualParameters!.customButtonSetups;
delete actualParameters!.onChangeItem;
assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
});
Expand Down Expand Up @@ -302,10 +302,10 @@ suite('Set Interpreter Command', () => {
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);

expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
const refreshButtonCallback = actualParameters!.customButtonSetup?.callback;
expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set');
const refreshButtons = actualParameters!.customButtonSetups;
expect(refreshButtons).to.not.equal(undefined, 'Callback not set');
delete actualParameters!.initialize;
delete actualParameters!.customButtonSetup;
delete actualParameters!.customButtonSetups;
delete actualParameters!.onChangeItem;
assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
});
Expand Down Expand Up @@ -458,10 +458,10 @@ suite('Set Interpreter Command', () => {
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);

expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
const refreshButtonCallback = actualParameters!.customButtonSetup?.callback;
expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set');
const refreshButtons = actualParameters!.customButtonSetups;
expect(refreshButtons).to.not.equal(undefined, 'Callback not set');
delete actualParameters!.initialize;
delete actualParameters!.customButtonSetup;
delete actualParameters!.customButtonSetups;
delete actualParameters!.onChangeItem;
assert.deepStrictEqual(actualParameters?.items, expectedParameters.items, 'Params not equal');
});
Expand Down Expand Up @@ -542,11 +542,11 @@ suite('Set Interpreter Command', () => {
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);

expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
const refreshButtonCallback = actualParameters!.customButtonSetup?.callback;
expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set');
const refreshButtons = actualParameters!.customButtonSetups;
expect(refreshButtons).to.not.equal(undefined, 'Callback not set');

delete actualParameters!.initialize;
delete actualParameters!.customButtonSetup;
delete actualParameters!.customButtonSetups;
delete actualParameters!.onChangeItem;

assert.deepStrictEqual(actualParameters, expectedParameters, 'Params not equal');
Expand All @@ -566,12 +566,19 @@ suite('Set Interpreter Command', () => {
await setInterpreterCommand._pickInterpreter(multiStepInput.object, state);

expect(actualParameters).to.not.equal(undefined, 'Parameters not set');
const refreshButtonCallback = actualParameters!.customButtonSetup?.callback;
expect(refreshButtonCallback).to.not.equal(undefined, 'Callback not set');

when(interpreterService.triggerRefresh()).thenResolve();
await refreshButtonCallback!({} as QuickPick<QuickPickItem>); // Invoke callback, meaning that the refresh button is clicked.
verify(interpreterService.triggerRefresh()).once();
const refreshButtons = actualParameters!.customButtonSetups;
expect(refreshButtons).to.not.equal(undefined, 'Callback not set');

expect(refreshButtons?.length).to.equal(2);
let arg;
when(interpreterService.triggerRefresh(undefined, anything())).thenCall((_, _arg) => {
arg = _arg;
return Promise.resolve();
});
await refreshButtons![0].callback!({} as QuickPick<QuickPickItem>); // Invoke callback, meaning that the refresh button is clicked.
expect(arg).to.deep.equal({ clearCache: true });
await refreshButtons![1].callback!({} as QuickPick<QuickPickItem>); // Invoke callback, meaning that the refresh button is clicked.
expect(arg).to.deep.equal({ clearCache: false });
});

test('Events to update quickpick updates the quickpick accordingly', async () => {
Expand Down

0 comments on commit 2379238

Please sign in to comment.