From a764bc9822e113a18c954286a62b26fde6514a64 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 14:47:16 -0800 Subject: [PATCH 01/15] Undo changes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index e530be32b367..99f8582c614c 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(p.description[6:] for p in completion.params if p)) + ', '.join(self._get_param_name(p.description) for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From 9d1b2cc85c3563ccc9b7a242206ae6a5b6d644f6 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 13 Feb 2018 15:44:21 -0800 Subject: [PATCH 02/15] Test fixes --- pythonFiles/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/completion.py b/pythonFiles/completion.py index 99f8582c614c..e530be32b367 100644 --- a/pythonFiles/completion.py +++ b/pythonFiles/completion.py @@ -88,7 +88,7 @@ def _generate_signature(self, completion): return '' return '%s(%s)' % ( completion.name, - ', '.join(self._get_param_name(p.description) for p in completion.params if p)) + ', '.join(p.description[6:] for p in completion.params if p)) def _get_call_signatures(self, script): """Extract call signatures from jedi.api.Script object in failsafe way. From a91291a072bc9730252c199969e7a40a698ca2d2 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 1 Mar 2018 22:03:47 -0800 Subject: [PATCH 03/15] Increase timeout --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 135ca5d8b6c1..22ffd45a0675 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 25000, + timeout: 35000, retries: 3, grep }; From bf266af260b16e1021511a7274c3d6576182179f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:26:53 -0800 Subject: [PATCH 04/15] Remove double event listening --- src/client/providers/linterProvider.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index fb66aab3971b..27aa85ffa61f 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,7 +9,6 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; -import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -17,7 +16,6 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; - private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -31,12 +29,9 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); - this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); - this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); - this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 7bc6bd643e5ec53eb71a259ad2a5a43a643f7e33 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 16:35:39 -0800 Subject: [PATCH 05/15] Remove test --- src/test/linters/lint.provider.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 023ee86223be..51e49d3d35b9 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,16 +113,6 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); - test('Lint on change interpreters', () => { - const e = new vscode.EventEmitter(); - interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); - - // tslint:disable-next-line:no-unused-variable - const provider = new LinterProvider(context.object, serviceContainer); - e.fire(); - engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); - }); - test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From 8ce8b48db3aedb785026b2fe22071b40d2f6c048 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:12 -0800 Subject: [PATCH 06/15] Revert "Remove test" This reverts commit e240c3fd117c38b9e6fdcbdd1ba2715789fefe48. --- src/test/linters/lint.provider.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 51e49d3d35b9..023ee86223be 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -113,6 +113,16 @@ suite('Linting - Provider', () => { engine.verify(x => x.lintDocument(document.object, 'save'), TypeMoq.Times.never()); }); + test('Lint on change interpreters', () => { + const e = new vscode.EventEmitter(); + interpreterService.setup(x => x.onDidChangeInterpreter).returns(() => e.event); + + // tslint:disable-next-line:no-unused-variable + const provider = new LinterProvider(context.object, serviceContainer); + e.fire(); + engine.verify(x => x.lintOpenPythonFiles(), TypeMoq.Times.once()); + }); + test('Lint on save pylintrc', async () => { docManager.setup(x => x.onDidSaveTextDocument).returns(() => emitter.event); document.setup(x => x.uri).returns(() => vscode.Uri.file('.pylintrc')); From e3a549e58e0f888d79364e353cbcdf00d86b3416 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 6 Mar 2018 17:02:47 -0800 Subject: [PATCH 07/15] Revert "Remove double event listening" This reverts commit af573be27372a79d5589e2134002cc753bb54f2a. --- src/client/providers/linterProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/client/providers/linterProvider.ts b/src/client/providers/linterProvider.ts index 27aa85ffa61f..fb66aab3971b 100644 --- a/src/client/providers/linterProvider.ts +++ b/src/client/providers/linterProvider.ts @@ -9,6 +9,7 @@ import { ConfigSettingMonitor } from '../common/configSettingMonitor'; import { isTestExecution } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IConfigurationService } from '../common/types'; +import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { ILinterManager, ILintingEngine } from '../linters/types'; @@ -16,6 +17,7 @@ export class LinterProvider implements vscode.Disposable { private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; private configMonitor: ConfigSettingMonitor; + private interpreterService: IInterpreterService; private documents: IDocumentManager; private configuration: IConfigurationService; private linterManager: ILinterManager; @@ -29,9 +31,12 @@ export class LinterProvider implements vscode.Disposable { this.fs = serviceContainer.get(IFileSystem); this.engine = serviceContainer.get(ILintingEngine); this.linterManager = serviceContainer.get(ILinterManager); + this.interpreterService = serviceContainer.get(IInterpreterService); this.documents = serviceContainer.get(IDocumentManager); this.configuration = serviceContainer.get(IConfigurationService); + this.disposables.push(this.interpreterService.onDidChangeInterpreter(() => this.engine.lintOpenPythonFiles())); + this.documents.onDidOpenTextDocument(e => this.onDocumentOpened(e), this.context.subscriptions); this.documents.onDidCloseTextDocument(e => this.onDocumentClosed(e), this.context.subscriptions); this.documents.onDidSaveTextDocument((e) => this.onDocumentSaved(e), this.context.subscriptions); From 92e8c1ee2e264784b76a250d1f39b157ba198bbe Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 15:32:47 -0700 Subject: [PATCH 08/15] #1096 The if statement is automatically formatted incorrectly --- src/client/formatters/lineFormatter.ts | 10 ++++++---- src/test/format/extension.onEnterFormat.test.ts | 7 ++++++- src/test/pythonFiles/formatting/fileToFormatOnEnter.py | 1 + 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 4b3bff70aa8d..835e7e82b92a 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -5,14 +5,15 @@ import Char from 'typescript-char'; import { BraceCounter } from '../language/braceCounter'; import { TextBuilder } from '../language/textBuilder'; +import { TextRangeCollection } from '../language/textRangeCollection'; import { Tokenizer } from '../language/tokenizer'; import { ITextRangeCollection, IToken, TokenType } from '../language/types'; export class LineFormatter { - private builder: TextBuilder; - private tokens: ITextRangeCollection; - private braceCounter: BraceCounter; - private text: string; + private builder = new TextBuilder(); + private tokens: ITextRangeCollection = new TextRangeCollection([]); + private braceCounter = new BraceCounter(); + private text = ''; // tslint:disable-next-line:cyclomatic-complexity public formatLine(text: string): string { @@ -123,6 +124,7 @@ export class LineFormatter { if (this.isBraceType(t.type)) { this.braceCounter.countBrace(t); } + this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 74597ce19be7..23e5cbecb8fa 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -59,8 +59,13 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'x.y', 'Line ending with period was reformatted'); }); - test('Formatting line ending in string', async () => { + test('Formatting line with unknown neighboring tokens', async () => { const text = await formatAtPosition(9, 0); + assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); + }); + + test('Formatting line ending in string', async () => { + const text = await formatAtPosition(10, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index bbd025363098..67d533125ab2 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -6,4 +6,5 @@ x+1 # @x x.y +if x<=1: x+""" From b540a1dc43b376e05ddb29bbf6a16dd1fb51843e Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 15:35:31 -0700 Subject: [PATCH 09/15] Merge fix --- src/test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/index.ts b/src/test/index.ts index 22ffd45a0675..135ca5d8b6c1 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -21,7 +21,7 @@ const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined; const options: MochaSetupOptions & { retries: number } = { ui: 'tdd', useColors: true, - timeout: 35000, + timeout: 25000, retries: 3, grep }; From 7b0573ed946ec6ed34d077b0f824f2a7aaaba613 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:08:23 -0700 Subject: [PATCH 10/15] Add more tests --- src/client/formatters/lineFormatter.ts | 41 ++++++++++++++++--- .../format/extension.onEnterFormat.test.ts | 12 +++++- .../formatting/fileToFormatOnEnter.py | 2 + 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 835e7e82b92a..7684c9337755 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -73,7 +73,7 @@ export class LineFormatter { break; default: - this.handleOther(t); + this.handleOther(t, i); break; } } @@ -109,10 +109,7 @@ export class LineFormatter { if (this.braceCounter.isOpened(TokenType.OpenBrace)) { // Check if this is = in function arguments. If so, do not // add spaces around it. - const prev = this.tokens.getItemAt(index - 1); - const prevPrev = this.tokens.getItemAt(index - 2); - if (prev.type === TokenType.Identifier && - (prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) { + if (this.isEqualsInsideArguments(index)) { this.builder.append('='); return true; } @@ -120,14 +117,46 @@ export class LineFormatter { return false; } - private handleOther(t: IToken): void { + private handleOther(t: IToken, index: number): void { if (this.isBraceType(t.type)) { this.braceCounter.countBrace(t); + this.builder.append(this.text.substring(t.start, t.end)); + return; + } + + if (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)) { + // Don't insert space after (, [ or { + this.builder.append(this.text.substring(t.start, t.end)); + return; + } } + + // In general, keep tokes separated this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } + private isEqualsInsideArguments(index: number): boolean { + if (index < 2) { + return false; + } + const prev = this.tokens.getItemAt(index - 1); + const prevPrev = this.tokens.getItemAt(index - 2); + if (prev.type === TokenType.Identifier && + (prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) { + return true; + } + return false; + } + private isOpenBraceType(type: TokenType): boolean { return type === TokenType.OpenBrace || type === TokenType.OpenBracket || type === TokenType.OpenCurly; } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 23e5cbecb8fa..e34aeb0a0ed3 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -64,8 +64,18 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); }); - test('Formatting line ending in string', async () => { + test('Formatting method definition with arguments', async () => { const text = await formatAtPosition(10, 0); + assert.equal(text, 'def __init__(self, age=23)', 'Method definition with arguments was not formatted'); + }); + + test('Formatting space after open brace', async () => { + const text = await formatAtPosition(11, 0); + assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); + }); + + test('Formatting line ending in string', async () => { + const text = await formatAtPosition(12, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index 67d533125ab2..779167118ffc 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -7,4 +7,6 @@ @x x.y if x<=1: +def __init__(self, age = 23) +while(1) x+""" From facb10613596b28f82e672266f130e57b4311847 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:30:21 -0700 Subject: [PATCH 11/15] More tests --- src/test/format/extension.onEnterFormat.test.ts | 11 ++++++++--- .../pythonFiles/formatting/fileToFormatOnEnter.py | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index e34aeb0a0ed3..689129da78c8 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -64,18 +64,23 @@ suite('Formatting - OnEnter provider', () => { assert.equal(text, 'if x <= 1:', 'Line with unknown neighboring tokens was not formatted'); }); - test('Formatting method definition with arguments', async () => { + test('Formatting line with unknown neighboring tokens', async () => { const text = await formatAtPosition(10, 0); + assert.equal(text, 'if 1 <= x:', 'Line with unknown neighboring tokens was not formatted'); + }); + + test('Formatting method definition with arguments', async () => { + const text = await formatAtPosition(11, 0); assert.equal(text, 'def __init__(self, age=23)', 'Method definition with arguments was not formatted'); }); test('Formatting space after open brace', async () => { - const text = await formatAtPosition(11, 0); + const text = await formatAtPosition(12, 0); assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); }); test('Formatting line ending in string', async () => { - const text = await formatAtPosition(12, 0); + const text = await formatAtPosition(13, 0); assert.equal(text, 'x + """', 'Line ending in multiline string was not formatted'); }); diff --git a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py index 779167118ffc..8adfd1fa1233 100644 --- a/src/test/pythonFiles/formatting/fileToFormatOnEnter.py +++ b/src/test/pythonFiles/formatting/fileToFormatOnEnter.py @@ -7,6 +7,7 @@ @x x.y if x<=1: +if 1<=x: def __init__(self, age = 23) while(1) x+""" From f113881370f26a52f159862bb822f0119225013c Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Tue, 27 Mar 2018 16:32:06 -0700 Subject: [PATCH 12/15] Typo --- src/client/formatters/lineFormatter.ts | 2 +- src/test/format/extension.onEnterFormat.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 7684c9337755..56e9b6cf7237 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -139,7 +139,7 @@ export class LineFormatter { } } - // In general, keep tokes separated + // In general, keep tokens separated this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); } diff --git a/src/test/format/extension.onEnterFormat.test.ts b/src/test/format/extension.onEnterFormat.test.ts index 689129da78c8..8f594d5e2559 100644 --- a/src/test/format/extension.onEnterFormat.test.ts +++ b/src/test/format/extension.onEnterFormat.test.ts @@ -76,7 +76,7 @@ suite('Formatting - OnEnter provider', () => { test('Formatting space after open brace', async () => { const text = await formatAtPosition(12, 0); - assert.equal(text, 'while(1)', 'Method definition with arguments was not formatted'); + assert.equal(text, 'while(1)', 'Space after open brace was not formatted'); }); test('Formatting line ending in string', async () => { From 3e76718c097e23b52a9ffbe5de27f18f512f3f5f Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 27 Mar 2018 18:15:11 -0700 Subject: [PATCH 13/15] Test --- src/client/formatters/lineFormatter.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 56e9b6cf7237..4c30d8c17baa 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -72,6 +72,10 @@ export class LineFormatter { this.builder.append(this.text.substring(t.start, t.end)); break; + case TokenType.Semicolon: + this.builder.append(';'); + break; + default: this.handleOther(t, i); break; @@ -132,7 +136,7 @@ export class LineFormatter { if (index > 0) { const prev = this.tokens.getItemAt(index - 1); - if (this.isOpenBraceType(prev.type)) { + 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; From 6e85dc68a3516175546fc6c65cddede03a58bcea Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Tue, 27 Mar 2018 21:47:51 -0700 Subject: [PATCH 14/15] Also better handle multiline arguments --- src/client/formatters/lineFormatter.ts | 47 ++++++++++++++----- .../format/extension.lineFormatter.test.ts | 16 +++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 4c30d8c17baa..694af69ea5ff 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -90,7 +90,7 @@ export class LineFormatter { const opCode = this.text.charCodeAt(t.start); switch (opCode) { case Char.Equal: - if (index >= 2 && this.handleEqual(t, index)) { + if (this.handleEqual(t, index)) { return; } break; @@ -110,13 +110,13 @@ export class LineFormatter { } private handleEqual(t: IToken, index: number): boolean { - if (this.braceCounter.isOpened(TokenType.OpenBrace)) { - // Check if this is = in function arguments. If so, do not - // add spaces around it. - if (this.isEqualsInsideArguments(index)) { - this.builder.append('='); - return true; - } + if (this.isMultipleStatements(index) && !this.braceCounter.isOpened(TokenType.OpenBrace)) { + return false; // x = 1; x, y = y, x + } + // Check if this is = in function arguments. If so, do not add spaces around it. + if (this.isEqualsInsideArguments(index)) { + this.builder.append('='); + return true; } return false; } @@ -149,14 +149,23 @@ export class LineFormatter { } private isEqualsInsideArguments(index: number): boolean { - if (index < 2) { + if (index < 1) { return false; } const prev = this.tokens.getItemAt(index - 1); - const prevPrev = this.tokens.getItemAt(index - 2); - if (prev.type === TokenType.Identifier && - (prevPrev.type === TokenType.Comma || prevPrev.type === TokenType.OpenBrace)) { - return true; + 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; + } + } } return false; } @@ -170,4 +179,16 @@ export class LineFormatter { private isBraceType(type: TokenType): boolean { return this.isOpenBraceType(type) || this.isCloseBraceType(type); } + private isValueType(type: TokenType): boolean { + return type === TokenType.Identifier || type === TokenType.Unknown || + type === TokenType.Number || type === TokenType.String; + } + private isMultipleStatements(index: number): boolean { + for (let i = index; i >= 0; i -= 1) { + if (this.tokens.getItemAt(i).type === TokenType.Semicolon) { + return true; + } + } + return false; + } } diff --git a/src/test/format/extension.lineFormatter.test.ts b/src/test/format/extension.lineFormatter.test.ts index 842cb02d735d..79de72c5774a 100644 --- a/src/test/format/extension.lineFormatter.test.ts +++ b/src/test/format/extension.lineFormatter.test.ts @@ -65,4 +65,20 @@ suite('Formatting - line formatter', () => { const actual = formatter.formatLine(' # comment'); assert.equal(actual, ' # comment'); }); + test('Equals in first argument', () => { + const actual = formatter.formatLine('foo(x =0)'); + assert.equal(actual, 'foo(x=0)'); + }); + test('Equals in second argument', () => { + const actual = formatter.formatLine('foo(x,y= \"a\",'); + assert.equal(actual, 'foo(x, y=\"a\",'); + }); + test('Equals in multiline arguments', () => { + const actual = formatter.formatLine('x = 1,y =-2)'); + assert.equal(actual, 'x=1, y=-2)'); + }); + test('Equals in multiline arguments starting comma', () => { + const actual = formatter.formatLine(',x = 1,y =m)'); + assert.equal(actual, ', x=1, y=m)'); + }); }); From 99e037c0a2533c37faa3e7da54125f7fe1d7ae27 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 28 Mar 2018 10:09:43 -0700 Subject: [PATCH 15/15] Add a couple missing periods [skip ci] --- src/client/formatters/lineFormatter.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/formatters/lineFormatter.ts b/src/client/formatters/lineFormatter.ts index 694af69ea5ff..fc347235a525 100644 --- a/src/client/formatters/lineFormatter.ts +++ b/src/client/formatters/lineFormatter.ts @@ -28,7 +28,7 @@ export class LineFormatter { const ws = this.text.substr(0, this.tokens.getItemAt(0).start); if (ws.length > 0) { - this.builder.append(ws); // Preserve leading indentation + this.builder.append(ws); // Preserve leading indentation. } for (let i = 0; i < this.tokens.count; i += 1) { @@ -56,16 +56,16 @@ export class LineFormatter { break; case TokenType.Colon: - // x: 1 if not in slice, x[1:y] if inside the slice + // x: 1 if not in slice, x[1:y] if inside the slice. this.builder.append(':'); if (!this.braceCounter.isOpened(TokenType.OpenBracket) && (next && next.type !== TokenType.Colon)) { - // Not inside opened [[ ... ] sequence + // Not inside opened [[ ... ] sequence. this.builder.softAppendSpace(); } break; case TokenType.Comment: - // add space before in-line comment + // Add space before in-line comment. if (prev) { this.builder.softAppendSpace(); } @@ -129,7 +129,7 @@ export class LineFormatter { } if (this.isEqualsInsideArguments(index - 1)) { - // Don't add space around = inside function arguments + // Don't add space around = inside function arguments. this.builder.append(this.text.substring(t.start, t.end)); return; } @@ -137,13 +137,13 @@ export class LineFormatter { 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 { + // Don't insert space after (, [ or { . this.builder.append(this.text.substring(t.start, t.end)); return; } } - // In general, keep tokens separated + // In general, keep tokens separated. this.builder.softAppendSpace(); this.builder.append(this.text.substring(t.start, t.end)); }