Skip to content

Show formatter extension install prompt and not the old prompt #21138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/client/formatters/baseFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import { IPythonToolExecutionService } from '../common/process/types';
import { IDisposableRegistry, IInstaller, Product } from '../common/types';
import { isNotebookCell } from '../common/utils/misc';
import { IServiceContainer } from '../ioc/types';
import { traceError, traceLog } from '../logging';
import { traceError } from '../logging';
import { getTempFileWithDocumentContents, getTextEditsFromPatch } from './../common/editor';
import { IFormatterHelper } from './types';
import { IInstallFormatterPrompt } from '../providers/prompts/types';

export abstract class BaseFormatter {
protected readonly workspace: IWorkspaceService;
private readonly helper: IFormatterHelper;
private errorShown: boolean = false;

constructor(public Id: string, private product: Product, protected serviceContainer: IServiceContainer) {
this.helper = serviceContainer.get<IFormatterHelper>(IFormatterHelper);
Expand Down Expand Up @@ -101,23 +102,22 @@ export abstract class BaseFormatter {
}

protected async handleError(_expectedFileName: string, error: Error, resource?: vscode.Uri) {
let customError = `Formatting with ${this.Id} failed.`;

if (isNotInstalledError(error)) {
const prompt = this.serviceContainer.get<IInstallFormatterPrompt>(IInstallFormatterPrompt);
if (!(await prompt.showInstallFormatterPrompt(resource))) {
const installer = this.serviceContainer.get<IInstaller>(IInstaller);
const isInstalled = await installer.isInstalled(this.product, resource);
if (!isInstalled) {
customError += `\nYou could either install the '${this.Id}' formatter, turn it off or use another formatter.`;
installer
.promptToInstall(this.product, resource)
.catch((ex) => traceError('Python Extension: promptToInstall', ex));
if (!isInstalled && !this.errorShown) {
traceError(
`\nPlease install '${this.Id}' into your environment.`,
"\nIf you don't want to use it you can turn it off or use another formatter in the settings.",
);
this.errorShown = true;
}
}
}

traceLog(`\n${customError}\n${error}`);
traceError(`Formatting with ${this.Id} failed:\n${error}`);
}

/**
Expand Down
25 changes: 19 additions & 6 deletions src/client/providers/prompts/installFormatterPrompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@ const SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY = 'showFormatterExtensionInsta

@injectable()
export class InstallFormatterPrompt implements IInstallFormatterPrompt {
private shownThisSession = false;
private currentlyShown = false;

constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) {}

/*
* This method is called when the user saves a python file or a cell.
* Returns true if an extension was selected. Otherwise returns false.
*/
public async showInstallFormatterPrompt(resource?: Uri): Promise<boolean> {
if (!inFormatterExtensionExperiment(this.serviceContainer)) {
return false;
}

const promptState = doNotShowPromptState(SHOW_FORMATTER_INSTALL_PROMPT_DONOTSHOW_KEY, this.serviceContainer);
if (this.shownThisSession || promptState.value) {
if (this.currentlyShown || promptState.value) {
return false;
}

Expand All @@ -53,7 +57,7 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt {
let selection: string | undefined;

if (black || autopep8) {
this.shownThisSession = true;
this.currentlyShown = true;
if (black && autopep8) {
selection = await showInformationMessage(
ToolsExtensions.selectMultipleFormattersPrompt,
Expand Down Expand Up @@ -81,15 +85,15 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt {
}
}
} else if (formatter === 'black' && !black) {
this.shownThisSession = true;
this.currentlyShown = true;
selection = await showInformationMessage(
ToolsExtensions.installBlackFormatterPrompt,
'Black',
'Autopep8',
Common.doNotShowAgain,
);
} else if (formatter === 'autopep8' && !autopep8) {
this.shownThisSession = true;
this.currentlyShown = true;
selection = await showInformationMessage(
ToolsExtensions.installAutopep8FormatterPrompt,
'Black',
Expand All @@ -98,23 +102,32 @@ export class InstallFormatterPrompt implements IInstallFormatterPrompt {
);
}

let userSelectedAnExtension = false;
if (selection === 'Black') {
if (black) {
userSelectedAnExtension = true;
await updateDefaultFormatter(BLACK_EXTENSION, resource);
} else {
userSelectedAnExtension = true;
await installFormatterExtension(BLACK_EXTENSION, resource);
}
} else if (selection === 'Autopep8') {
if (autopep8) {
userSelectedAnExtension = true;
await updateDefaultFormatter(AUTOPEP8_EXTENSION, resource);
} else {
userSelectedAnExtension = true;
await installFormatterExtension(AUTOPEP8_EXTENSION, resource);
}
} else if (selection === Common.doNotShowAgain) {
userSelectedAnExtension = false;
await promptState.updateValue(true);
} else {
userSelectedAnExtension = false;
}

return this.shownThisSession;
this.currentlyShown = false;
return userSelectedAnExtension;
}
}

Expand Down