Skip to content

Support the Black formatter #1611

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 8 commits into from
May 5, 2018
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
2 changes: 2 additions & 0 deletions news/1 Enhancements/1153.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add support for the [Black formatter](https://pypi.org/project/black/)
(thanks to [Josh Smeaton](https://github.com/jarshwah) for the initial patch)
18 changes: 17 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1200,14 +1200,30 @@
"python.formatting.provider": {
"type": "string",
"default": "autopep8",
"description": "Provider for formatting. Possible options include 'autopep8' and 'yapf'.",
"description": "Provider for formatting. Possible options include 'autopep8', 'black', and 'yapf'.",
"enum": [
"autopep8",
"black",
"yapf",
"none"
],
"scope": "resource"
},
"python.formatting.blackArgs": {
"type": "array",
"description": "Arguments passed in. Each argument is a separate item in the array.",
"default": [],
"items": {
"type": "string"
},
"scope": "resource"
},
"python.formatting.blackPath": {
"type": "string",
"default": "black",
"description": "Path to Black, you can use a custom version of Black by modifying this setting to include the full path.",
"scope": "resource"
},
"python.formatting.yapfArgs": {
"type": "array",
"description": "Arguments passed in. Each argument is a separate item in the array.",
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# but flake8 has a tighter pinning.
flake8
autopep8
black ; python_version>='3.6'
yapf
pylint
pep8
Expand Down
7 changes: 4 additions & 3 deletions src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
public venvFolders: string[] = [];
public devOptions: string[] = [];
public linting?: ILintingSettings;
public formatting?: IFormattingSettings;
public formatting!: IFormattingSettings;
public autoComplete?: IAutoCompleteSettings;
public unitTest?: IUnitTestSettings;
public unitTest!: IUnitTestSettings;
public terminal!: ITerminalSettings;
public sortImports?: ISortImportSettings;
public workspaceSymbols?: IWorkspaceSymbolSettings;
public workspaceSymbols!: IWorkspaceSymbolSettings;
public disableInstallationChecks = false;
public globalModuleInstallation = false;

Expand Down Expand Up @@ -213,6 +213,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
this.formatting = this.formatting ? this.formatting : {
autopep8Args: [], autopep8Path: 'autopep8',
provider: 'autopep8',
blackArgs: [], blackPath: 'black',
yapfArgs: [], yapfPath: 'yapf'
};
this.formatting.autopep8Path = getAbsolutePath(systemVariables.resolveAny(this.formatting.autopep8Path), workspaceRoot);
Expand Down
72 changes: 41 additions & 31 deletions src/client/common/installer/productInstaller.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { inject, injectable, named } from 'inversify';
import * as os from 'os';
import * as path from 'path';
import { OutputChannel, Uri } from 'vscode';
import * as vscode from 'vscode';
import { IFormatterHelper } from '../../formatters/types';
import { IServiceContainer } from '../../ioc/types';
Expand Down Expand Up @@ -33,14 +32,14 @@ abstract class BaseInstaller {
protected appShell: IApplicationShell;
protected configService: IConfigurationService;

constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) {
constructor(protected serviceContainer: IServiceContainer, protected outputChannel: vscode.OutputChannel) {
this.appShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
}

public abstract promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse>;
public abstract promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse>;

public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
if (product === Product.unittest) {
return InstallerResponse.Installed;
}
Expand All @@ -60,7 +59,7 @@ abstract class BaseInstaller {
.then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore);
}

public async isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
if (product === Product.unittest) {
return true;
}
Expand All @@ -85,22 +84,22 @@ abstract class BaseInstaller {
}
}

protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
throw new Error('getExecutableNameFromSettings is not supported on this object');
}
}

class CTagsInstaller extends BaseInstaller {
constructor(serviceContainer: IServiceContainer, outputChannel: OutputChannel) {
constructor(serviceContainer: IServiceContainer, outputChannel: vscode.OutputChannel) {
super(serviceContainer, outputChannel);
}

public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
const item = await this.appShell.showErrorMessage('Install CTags to enable Python workspace symbols?', 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}

public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
if (this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows) {
this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols');
this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.');
Expand All @@ -117,32 +116,41 @@ class CTagsInstaller extends BaseInstaller {
return InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
const settings = this.configService.getSettings(resource);
return settings.workspaceSymbols.ctagsPath;
}
}

class FormatterInstaller extends BaseInstaller {
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
// Hard-coded on purpose because the UI won't necessarily work having
// another formatter.
const formatters = [Product.autopep8, Product.black, Product.yapf];
const formatterNames = formatters.map((formatter) => ProductNames.get(formatter)!);
const productName = ProductNames.get(product)!;
formatterNames.splice(formatterNames.indexOf(productName), 1);
const useOptions = formatterNames.map((name) => `Use ${name}`);
const yesChoice = 'Yes';

const installThis = `Install ${productName}`;
const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8';
const useOtherFormatter = `Use '${alternateFormatter}' formatter`;
const item = await this.appShell.showErrorMessage(`Formatter ${productName} is not installed.`, installThis, useOtherFormatter);

if (item === installThis) {
const item = await this.appShell.showErrorMessage(`Formatter ${productName} is not installed. Install?`, yesChoice, ...useOptions);
if (item === yesChoice) {
return this.install(product, resource);
} else if (typeof item === 'string') {
for (const formatter of formatters) {
const formatterName = ProductNames.get(formatter)!;

if (item.endsWith(formatterName)) {
await this.configService.updateSettingAsync('formatting.provider', formatterName, resource);
return this.install(formatter, resource);
}
}
}
if (item === useOtherFormatter) {
await this.configService.updateSettingAsync('formatting.provider', alternateFormatter, resource);
return InstallerResponse.Installed;
}

return InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
const settings = this.configService.getSettings(resource);
const formatHelper = this.serviceContainer.get<IFormatterHelper>(IFormatterHelper);
const settingsPropNames = formatHelper.getSettingsPropertyNames(product);
Expand All @@ -152,7 +160,7 @@ class FormatterInstaller extends BaseInstaller {

// tslint:disable-next-line:max-classes-per-file
class LinterInstaller extends BaseInstaller {
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const install = 'Install';
const disableAllLinting = 'Disable linting';
Expand All @@ -173,21 +181,21 @@ class LinterInstaller extends BaseInstaller {
}
return InstallerResponse.Ignore;
}
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
const linterManager = this.serviceContainer.get<ILinterManager>(ILinterManager);
return linterManager.getLinterInfo(product).pathName(resource);
}
}

// tslint:disable-next-line:max-classes-per-file
class TestFrameworkInstaller extends BaseInstaller {
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const item = await this.appShell.showErrorMessage(`Test framework ${productName} is not installed. Install?`, 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}

protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
const testHelper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
const settingsPropNames = testHelper.getSettingsPropertyNames(product);
if (!settingsPropNames.pathName) {
Expand All @@ -201,12 +209,12 @@ class TestFrameworkInstaller extends BaseInstaller {

// tslint:disable-next-line:max-classes-per-file
class RefactoringLibraryInstaller extends BaseInstaller {
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
const productName = ProductNames.get(product)!;
const item = await this.appShell.showErrorMessage(`Refactoring library ${productName} is not installed. Install?`, 'Yes', 'No');
return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore;
}
protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string {
return translateProductToModule(product, ModuleNamePurpose.run);
}
}
Expand All @@ -230,19 +238,20 @@ export class ProductInstaller implements IInstaller {
this.ProductTypes.set(Product.pytest, ProductType.TestFramework);
this.ProductTypes.set(Product.unittest, ProductType.TestFramework);
this.ProductTypes.set(Product.autopep8, ProductType.Formatter);
this.ProductTypes.set(Product.black, ProductType.Formatter);
this.ProductTypes.set(Product.yapf, ProductType.Formatter);
this.ProductTypes.set(Product.rope, ProductType.RefactoringLibrary);
}

// tslint:disable-next-line:no-empty
public dispose() { }
public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
return this.createInstaller(product).promptToInstall(product, resource);
}
public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
return this.createInstaller(product).install(product, resource);
}
public async isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
return this.createInstaller(product).isInstalled(product, resource);
}
public translateProductToModuleName(product: Product, purpose: ModuleNamePurpose): string {
Expand Down Expand Up @@ -280,6 +289,7 @@ function translateProductToModule(product: Product, purpose: ModuleNamePurpose):
case Product.pylint: return 'pylint';
case Product.pytest: return 'pytest';
case Product.autopep8: return 'autopep8';
case Product.black: return 'black';
case Product.pep8: return 'pep8';
case Product.pydocstyle: return 'pydocstyle';
case Product.yapf: return 'yapf';
Expand Down
1 change: 1 addition & 0 deletions src/client/common/installer/productNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Product } from '../types';
// tslint:disable-next-line:variable-name
export const ProductNames = new Map<Product, string>();
ProductNames.set(Product.autopep8, 'autopep8');
ProductNames.set(Product.black, 'black');
ProductNames.set(Product.flake8, 'flake8');
ProductNames.set(Product.mypy, 'mypy');
ProductNames.set(Product.nosetest, 'nosetest');
Expand Down
11 changes: 7 additions & 4 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ export enum Product {
unittest = 12,
ctags = 13,
rope = 14,
isort = 15
isort = 15,
black = 16
}

export enum ModuleNamePurpose {
Expand Down Expand Up @@ -103,12 +104,12 @@ export interface IPythonSettings {
readonly jediMemoryLimit: number;
readonly devOptions: string[];
readonly linting?: ILintingSettings;
readonly formatting?: IFormattingSettings;
readonly unitTest?: IUnitTestSettings;
readonly formatting: IFormattingSettings;
readonly unitTest: IUnitTestSettings;
readonly autoComplete?: IAutoCompleteSettings;
readonly terminal: ITerminalSettings;
readonly sortImports?: ISortImportSettings;
readonly workspaceSymbols?: IWorkspaceSymbolSettings;
readonly workspaceSymbols: IWorkspaceSymbolSettings;
readonly envFile: string;
readonly disablePromptForFeatures: string[];
readonly disableInstallationChecks: boolean;
Expand Down Expand Up @@ -191,6 +192,8 @@ export interface IFormattingSettings {
readonly provider: string;
autopep8Path: string;
readonly autopep8Args: string[];
blackPath: string;
readonly blackArgs: string[];
yapfPath: string;
readonly yapfArgs: string[];
}
Expand Down
19 changes: 9 additions & 10 deletions src/client/formatters/baseFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import * as vscode from 'vscode';
import { OutputChannel, TextEdit, Uri } from 'vscode';
import { IWorkspaceService } from '../common/application/types';
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
import '../common/extensions';
Expand All @@ -13,12 +12,12 @@ import { getTempFileWithDocumentContents, getTextEditsFromPatch } from './../com
import { IFormatterHelper } from './types';

export abstract class BaseFormatter {
protected readonly outputChannel: OutputChannel;
protected readonly outputChannel: vscode.OutputChannel;
protected readonly workspace: IWorkspaceService;
private readonly helper: IFormatterHelper;

constructor(public Id: string, private product: Product, protected serviceContainer: IServiceContainer) {
this.outputChannel = serviceContainer.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.outputChannel = serviceContainer.get<vscode.OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.helper = serviceContainer.get<IFormatterHelper>(IFormatterHelper);
this.workspace = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
}
Expand Down Expand Up @@ -49,8 +48,8 @@ export abstract class BaseFormatter {

// autopep8 and yapf have the ability to read from the process input stream and return the formatted code out of the output stream.
// However they don't support returning the diff of the formatted text when reading data from the input stream.
// Yes getting text formatted that way avoids having to create a temporary file, however the diffing will have
// to be done here in node (extension), i.e. extension cpu, i.e. les responsive solution.
// Yet getting text formatted that way avoids having to create a temporary file, however the diffing will have
// to be done here in node (extension), i.e. extension CPU, i.e. less responsive solution.
const tempFile = await this.createTempFile(document);
if (this.checkCancellation(document.fileName, tempFile, token)) {
return [];
Expand All @@ -63,17 +62,17 @@ export abstract class BaseFormatter {
.then(output => output.stdout)
.then(data => {
if (this.checkCancellation(document.fileName, tempFile, token)) {
return [] as TextEdit[];
return [] as vscode.TextEdit[];
}
return getTextEditsFromPatch(document.getText(), data);
})
.catch(error => {
if (this.checkCancellation(document.fileName, tempFile, token)) {
return [] as TextEdit[];
return [] as vscode.TextEdit[];
}
// tslint:disable-next-line:no-empty
this.handleError(this.Id, error, document.uri).catch(() => { });
return [] as TextEdit[];
return [] as vscode.TextEdit[];
})
.then(edits => {
this.deleteTempFile(document.fileName, tempFile).ignoreErrors();
Expand All @@ -83,7 +82,7 @@ export abstract class BaseFormatter {
return promise;
}

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

if (isNotInstalledError(error)) {
Expand All @@ -100,7 +99,7 @@ export abstract class BaseFormatter {

private async createTempFile(document: vscode.TextDocument): Promise<string> {
return document.isDirty
? await getTempFileWithDocumentContents(document)
? getTempFileWithDocumentContents(document)
: document.fileName;
}

Expand Down
Loading