Skip to content

Fixes multiple issues with formatting on type #1450

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 51 commits into from
Apr 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
a764bc9
Undo changes
Feb 13, 2018
9d1b2cc
Test fixes
Feb 13, 2018
a91291a
Increase timeout
Mar 2, 2018
bf266af
Remove double event listening
Mar 7, 2018
7bc6bd6
Remove test
Mar 7, 2018
8ce8b48
Revert "Remove test"
Mar 7, 2018
e3a549e
Revert "Remove double event listening"
Mar 7, 2018
92e8c1e
#1096 The if statement is automatically formatted incorrectly
Mar 27, 2018
b540a1d
Merge fix
Mar 27, 2018
7b0573e
Add more tests
Mar 27, 2018
facb106
More tests
Mar 27, 2018
f113881
Typo
Mar 27, 2018
3e76718
Test
Mar 28, 2018
6e85dc6
Also better handle multiline arguments
Mar 28, 2018
99e037c
Add a couple missing periods
brettcannon Mar 28, 2018
3caeab7
Undo changes
Feb 13, 2018
eeb1f11
Test fixes
Feb 13, 2018
f5f78c7
Increase timeout
Mar 2, 2018
88744da
Remove double event listening
Mar 7, 2018
65dde44
Remove test
Mar 7, 2018
c513f71
Revert "Remove test"
Mar 7, 2018
ccb3886
Revert "Remove double event listening"
Mar 7, 2018
106f4db
Merge fix
Mar 27, 2018
9e5cb43
Merge branch 'master' of https://github.com/MikhailArkhipov/vscode-py…
Apr 5, 2018
e1da6a6
#1257 On type formatting errors for args and kwargs
Apr 5, 2018
e78f0fb
Handle f-strings
Apr 5, 2018
725cf71
Stop importing from test code
Apr 5, 2018
5cd6d45
#1308 Single line statements leading to an indentation on the next line
Apr 5, 2018
27613db
#726 editing python after inline if statement invalid indent
Apr 5, 2018
8061a20
Undo change
Apr 5, 2018
17dc292
Move constant
Apr 5, 2018
65964b9
Harden LS startup error checks
Apr 10, 2018
4bf5a4c
#1364 Intellisense doesn't work after specific const string
Apr 10, 2018
6f7212c
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Apr 12, 2018
ddbd295
Telemetry for the analysis enging
Apr 12, 2018
ffd1d3f
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Apr 12, 2018
d4afb6c
PR feedback
Apr 13, 2018
12186b8
Fix typo
Apr 16, 2018
ca90529
Test baseline update
Apr 16, 2018
a7267b5
Jedi 0.12
Apr 16, 2018
cfee109
Priority to goto_defition
Apr 16, 2018
1285789
Merge branch 'master' of https://github.com/Microsoft/vscode-python i…
Apr 17, 2018
d1ff1d9
News
Apr 17, 2018
1bd1651
Replace unzip
Apr 17, 2018
a69b6fd
Merge branch 'master' of https://github.com/Microsoft/vscode-python i…
Apr 17, 2018
f916ace
Linux flavors + test
Apr 18, 2018
28ca25f
Grammar check
Apr 19, 2018
ad9a3c9
Grammar test
Apr 20, 2018
ff8dd35
Test baselines
Apr 20, 2018
26726f8
Merge branch 'master' of https://github.com/Microsoft/vscode-python i…
Apr 20, 2018
0b3f316
Pin dependency
brettcannon Apr 23, 2018
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
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,12 @@
"description": "Automatically add brackets for functions.",
"scope": "resource"
},
"python.autoComplete.showAdvancedMembers": {
"type": "boolean",
"default": false,
"description": "Controls appearance of methods with double underscores in the completion list.",
"scope": "resource"
},
"python.workspaceSymbols.tagFilePath": {
"type": "string",
"default": "${workspaceFolder}/.vscode/tags",
Expand Down
2 changes: 1 addition & 1 deletion src/client/activation/analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class AnalysisExtensionActivator implements IExtensionActivator {
this.appShell = this.services.get<IApplicationShell>(IApplicationShell);
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.fs = this.services.get<IFileSystem>(IFileSystem);
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService));
this.platformData = new PlatformData(services.get<IPlatformService>(IPlatformService), this.fs);
}

public async activate(context: ExtensionContext): Promise<boolean> {
Expand Down
15 changes: 11 additions & 4 deletions src/client/activation/analysisEngineHashes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@

// This file will be replaced by a generated one during the release build
// with actual hashes of the uploaded packages.
export const analysis_engine_win_x86_sha512 = '';
export const analysis_engine_win_x64_sha512 = '';
export const analysis_engine_osx_x64_sha512 = '';
export const analysis_engine_linux_x64_sha512 = '';
// Values are for test purposes only
export const analysis_engine_win_x86_sha512 = 'win-x86';
export const analysis_engine_win_x64_sha512 = 'win-x64';
export const analysis_engine_osx_x64_sha512 = 'osx-x64';
export const analysis_engine_centos_x64_sha512 = 'centos-x64';
export const analysis_engine_debian_x64_sha512 = 'debian-x64';
export const analysis_engine_fedora_x64_sha512 = 'fedora-x64';
export const analysis_engine_ol_x64_sha512 = 'ol-x64';
export const analysis_engine_opensuse_x64_sha512 = 'opensuse-x64';
export const analysis_engine_rhel_x64_sha512 = 'rhel-x64';
export const analysis_engine_ubuntu_x64_sha512 = 'ubuntu-x64';
8 changes: 4 additions & 4 deletions src/client/activation/downloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ExtensionContext, OutputChannel, ProgressLocation, window } from 'vscod
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
import { noop } from '../common/core.utils';
import { createDeferred, createTemporaryFile } from '../common/helpers';
import { IPlatformService } from '../common/platform/types';
import { IFileSystem, IPlatformService } from '../common/platform/types';
import { IOutputChannel } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { HashVerifier } from './hashVerifier';
Expand All @@ -31,7 +31,7 @@ export class AnalysisEngineDownloader {
constructor(private readonly services: IServiceContainer, private engineFolder: string) {
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.platform = this.services.get<IPlatformService>(IPlatformService);
this.platformData = new PlatformData(this.platform);
this.platformData = new PlatformData(this.platform, this.services.get<IFileSystem>(IFileSystem));
}

public async downloadAnalysisEngine(context: ExtensionContext): Promise<void> {
Expand All @@ -49,7 +49,7 @@ export class AnalysisEngineDownloader {
}

private async downloadFile(): Promise<string> {
const platformString = this.platformData.getPlatformDesignator();
const platformString = await this.platformData.getPlatformName();
const remoteFileName = `${downloadBaseFileName}-${platformString}.${downloadVersion}${downloadFileExtension}`;
const uri = `${downloadUriPrefix}/${remoteFileName}`;
this.output.append(`Downloading ${uri}... `);
Expand Down Expand Up @@ -98,7 +98,7 @@ export class AnalysisEngineDownloader {
this.output.appendLine('');
this.output.append('Verifying download... ');
const verifier = new HashVerifier();
if (!await verifier.verifyHash(filePath, this.platformData.getExpectedHash())) {
if (!await verifier.verifyHash(filePath, await this.platformData.getExpectedHash())) {
throw new Error('Hash of the downloaded file does not match.');
}
this.output.append('valid.');
Expand Down
67 changes: 58 additions & 9 deletions src/client/activation/platformData.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,54 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { IPlatformService } from '../common/platform/types';
import { IFileSystem, IPlatformService } from '../common/platform/types';
import {
analysis_engine_linux_x64_sha512,
analysis_engine_centos_x64_sha512,
analysis_engine_debian_x64_sha512,
analysis_engine_fedora_x64_sha512,
analysis_engine_ol_x64_sha512,
analysis_engine_opensuse_x64_sha512,
analysis_engine_osx_x64_sha512,
analysis_engine_rhel_x64_sha512,
analysis_engine_ubuntu_x64_sha512,
analysis_engine_win_x64_sha512,
analysis_engine_win_x86_sha512
} from './analysisEngineHashes';

// '/etc/os-release', ID=flavor
const supportedLinuxFlavors = [
'centos',
'debian',
'fedora',
'ol',
'opensuse',
'rhel',
'ubuntu'
];

export class PlatformData {
constructor(private platform: IPlatformService) { }
public getPlatformDesignator(): string {
constructor(private platform: IPlatformService, private fs: IFileSystem) { }
public async getPlatformName(): Promise<string> {
if (this.platform.isWindows) {
return this.platform.is64bit ? 'win-x64' : 'win-x86';
}
if (this.platform.isMac) {
return 'osx-x64';
}
if (this.platform.isLinux && this.platform.is64bit) {
return 'linux-x64';
if (this.platform.isLinux) {
if (!this.platform.is64bit) {
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
}
const linuxFlavor = await this.getLinuxFlavor();
if (linuxFlavor.length === 0) {
throw new Error('Unable to determine Linux flavor from /etc/os-release.');
}
if (supportedLinuxFlavors.indexOf(linuxFlavor) < 0) {
throw new Error(`${linuxFlavor} is not supported.`);
}
return `${linuxFlavor}-x64`;
}
throw new Error('Python Analysis Engine does not support 32-bit Linux.');
throw new Error('Unknown OS platform.');
}

public getEngineDllName(): string {
Expand All @@ -34,16 +61,38 @@ export class PlatformData {
: 'Microsoft.PythonTools.VsCode';
}

public getExpectedHash(): string {
public async getExpectedHash(): Promise<string> {
if (this.platform.isWindows) {
return this.platform.is64bit ? analysis_engine_win_x64_sha512 : analysis_engine_win_x86_sha512;
}
if (this.platform.isMac) {
return analysis_engine_osx_x64_sha512;
}
if (this.platform.isLinux && this.platform.is64bit) {
return analysis_engine_linux_x64_sha512;
const linuxFlavor = await this.getLinuxFlavor();
// tslint:disable-next-line:switch-default
switch (linuxFlavor) {
case 'centos': return analysis_engine_centos_x64_sha512;
case 'debian': return analysis_engine_debian_x64_sha512;
case 'fedora': return analysis_engine_fedora_x64_sha512;
case 'ol': return analysis_engine_ol_x64_sha512;
case 'opensuse': return analysis_engine_opensuse_x64_sha512;
case 'rhel': return analysis_engine_rhel_x64_sha512;
case 'ubuntu': return analysis_engine_ubuntu_x64_sha512;
}
}
throw new Error('Unknown platform.');
}

private async getLinuxFlavor(): Promise<string> {
const verFile = '/etc/os-release';
const data = await this.fs.readFile(verFile);
if (data) {
const res = /ID=(.*)/.exec(data);
if (res && res.length > 1) {
return res[1];
}
}
return '';
}
}
132 changes: 101 additions & 31 deletions src/client/formatters/lineFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class LineFormatter {

case TokenType.Comma:
this.builder.append(',');
if (next && !this.isCloseBraceType(next.type)) {
if (next && !this.isCloseBraceType(next.type) && next.type !== TokenType.Colon) {
this.builder.softAppendSpace();
}
break;
Expand All @@ -52,7 +52,12 @@ export class LineFormatter {
if (prev && !this.isOpenBraceType(prev.type) && prev.type !== TokenType.Colon && prev.type !== TokenType.Operator) {
this.builder.softAppendSpace();
}
this.builder.append(this.text.substring(t.start, t.end));
const id = this.text.substring(t.start, t.end);
this.builder.append(id);
if (this.keywordWithSpaceAfter(id) && next && this.isOpenBraceType(next.type)) {
// for x in ()
this.builder.softAppendSpace();
}
break;

case TokenType.Colon:
Expand Down Expand Up @@ -84,8 +89,10 @@ export class LineFormatter {
return this.builder.getText();
}

// tslint:disable-next-line:cyclomatic-complexity
private handleOperator(index: number): void {
const t = this.tokens.getItemAt(index);
const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined;
if (t.length === 1) {
const opCode = this.text.charCodeAt(t.start);
switch (opCode) {
Expand All @@ -99,18 +106,36 @@ export class LineFormatter {
case Char.ExclamationMark:
this.builder.append(this.text[t.start]);
return;
case Char.Asterisk:
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
this.builder.softAppendSpace();
this.builder.append('*');
return;
}
break;
default:
break;
}
} else if (t.length === 2) {
if (this.text.charCodeAt(t.start) === Char.Asterisk && this.text.charCodeAt(t.start + 1) === Char.Asterisk) {
if (!prev || (prev.type !== TokenType.Identifier && prev.type !== TokenType.Number)) {
this.builder.append('**');
return;
}
if (prev && prev.type === TokenType.Identifier && prev.length === 6 && this.text.substr(prev.start, prev.length) === 'lambda') {
this.builder.softAppendSpace();
this.builder.append('**');
return;
}
}
}

// Do not append space if operator is preceded by '(' or ',' as in foo(**kwarg)
if (index > 0) {
const prev = this.tokens.getItemAt(index - 1);
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma) {
this.builder.append(this.text.substring(t.start, t.end));
return;
}
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Comma)) {
this.builder.append(this.text.substring(t.start, t.end));
return;
}

this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
this.builder.softAppendSpace();
Expand All @@ -135,43 +160,82 @@ export class LineFormatter {
return;
}

if (this.isEqualsInsideArguments(index - 1)) {
const prev = index > 0 ? this.tokens.getItemAt(index - 1) : undefined;
if (prev && prev.length === 1 && this.text.charCodeAt(prev.start) === Char.Equal && this.isEqualsInsideArguments(index - 1)) {
// Don't add space around = inside function arguments.
this.builder.append(this.text.substring(t.start, t.end));
return;
}

if (index > 0) {
const prev = this.tokens.getItemAt(index - 1);
if (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon) {
// Don't insert space after (, [ or { .
this.builder.append(this.text.substring(t.start, t.end));
return;
}
if (prev && (this.isOpenBraceType(prev.type) || prev.type === TokenType.Colon)) {
// Don't insert space after (, [ or { .
this.builder.append(this.text.substring(t.start, t.end));
return;
}

// In general, keep tokens separated.
this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
if (t.type === TokenType.Unknown) {
this.handleUnknown(t);
} else {
// In general, keep tokens separated.
this.builder.softAppendSpace();
this.builder.append(this.text.substring(t.start, t.end));
}
}

private handleUnknown(t: IToken): void {
const prevChar = t.start > 0 ? this.text.charCodeAt(t.start - 1) : 0;
if (prevChar === Char.Space || prevChar === Char.Tab) {
this.builder.softAppendSpace();
}
this.builder.append(this.text.substring(t.start, t.end));

const nextChar = t.end < this.text.length - 1 ? this.text.charCodeAt(t.end) : 0;
if (nextChar === Char.Space || nextChar === Char.Tab) {
this.builder.softAppendSpace();
}
}
private isEqualsInsideArguments(index: number): boolean {
// Since we don't have complete statement, this is mostly heuristics.
// Therefore the code may not be handling all possible ways of the
// argument list continuation.
if (index < 1) {
return false;
}

const prev = this.tokens.getItemAt(index - 1);
if (prev.type === TokenType.Identifier) {
if (index >= 2) {
// (x=1 or ,x=1
const prevPrev = this.tokens.getItemAt(index - 2);
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
} else if (index < this.tokens.count - 2) {
const next = this.tokens.getItemAt(index + 1);
const nextNext = this.tokens.getItemAt(index + 2);
// x=1, or x=1)
if (this.isValueType(next.type)) {
return nextNext.type === TokenType.Comma || nextNext.type === TokenType.CloseBrace;
}
if (prev.type !== TokenType.Identifier) {
return false;
}

const first = this.tokens.getItemAt(0);
if (first.type === TokenType.Comma) {
return true; // Line starts with commma
}

const last = this.tokens.getItemAt(this.tokens.count - 1);
if (last.type === TokenType.Comma) {
return true; // Line ends in comma
}

if (index >= 2) {
// (x=1 or ,x=1
const prevPrev = this.tokens.getItemAt(index - 2);
return prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace;
}

if (index >= this.tokens.count - 2) {
return false;
}

const next = this.tokens.getItemAt(index + 1);
const nextNext = this.tokens.getItemAt(index + 2);
// x=1, or x=1)
if (this.isValueType(next.type)) {
if (nextNext.type === TokenType.CloseBrace) {
return true;
}
if (nextNext.type === TokenType.Comma) {
return last.type === TokenType.CloseBrace;
}
}
return false;
Expand All @@ -198,4 +262,10 @@ export class LineFormatter {
}
return false;
}
private keywordWithSpaceAfter(s: string): boolean {
return s === 'in' || s === 'return' || s === 'and' ||
s === 'or' || s === 'not' || s === 'from' ||
s === 'import' || s === 'except' || s === 'for' ||
s === 'as' || s === 'is';
}
}
Loading