From acbe13080ae47fe7833467279946eaad86df70af Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Jul 2019 16:14:48 -0700 Subject: [PATCH 01/21] Dedent on elif|else --- src/client/extension.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 3a22ca96ec65..6e3e08399c22 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -177,10 +177,6 @@ async function activateUnsafe(context: ExtensionContext): Promise // tslint:disable-next-line:no-non-null-assertion languages.setLanguageConfiguration(PYTHON_LANGUAGE, { onEnterRules: [ - { - beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, - action: { indentAction: IndentAction.Indent } - }, { beforeText: /^(?!\s+\\)[^#\n]+\\\s*/, action: { indentAction: IndentAction.Indent } @@ -195,7 +191,11 @@ async function activateUnsafe(context: ExtensionContext): Promise afterText: /\s+$/, action: { indentAction: IndentAction.Outdent } } - ] + ], + indentationRules: { + increaseIndentPattern: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, + decreaseIndentPattern: /^\s*(?:elif|else)\b.*:\s*/ + } }); if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') { From c7f72152cb764fabf258c66fab989d9e28f27a72 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 4 Jul 2019 14:08:28 -0700 Subject: [PATCH 02/21] Add unit tests (will conflict with #4241) --- src/client/extension.ts | 7 ++- src/test/extension.unit.test.ts | 87 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 6e3e08399c22..72d7012f91ea 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -112,6 +112,9 @@ durations.codeLoadingTime = stopWatch.elapsedTime; const activationDeferred = createDeferred(); let activatedServiceContainer: ServiceContainer | undefined; +export const INCREASE_INDENT_REGEX = /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/; +export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else)\b.*:\s*/; + export async function activate(context: ExtensionContext): Promise { try { return await activateUnsafe(context); @@ -193,8 +196,8 @@ async function activateUnsafe(context: ExtensionContext): Promise } ], indentationRules: { - increaseIndentPattern: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, - decreaseIndentPattern: /^\s*(?:elif|else)\b.*:\s*/ + increaseIndentPattern: INCREASE_INDENT_REGEX, + decreaseIndentPattern: DECREASE_INDENT_REGEX } }); diff --git a/src/test/extension.unit.test.ts b/src/test/extension.unit.test.ts index 553be3f7121e..ad153865635c 100644 --- a/src/test/extension.unit.test.ts +++ b/src/test/extension.unit.test.ts @@ -6,9 +6,16 @@ // tslint:disable:no-any import { expect } from 'chai'; +import * as sinon from 'sinon'; import { buildApi } from '../client/api'; import { EXTENSION_ROOT_DIR } from '../client/common/constants'; +// Stub sourceMapSupport.initialize before we import from extension.ts (we don't actually need it) +// tslint:disable-next-line: no-require-imports no-var-requires +const sourceMapSupport = require('../client/sourceMapSupport'); +sinon.stub(sourceMapSupport, 'initialize'); +import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX } from '../client/extension'; + const expectedPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`; suite('Extension API Debugger', () => { @@ -22,4 +29,84 @@ suite('Extension API Debugger', () => { const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait']; expect(args).to.be.deep.equal(expectedArgs); }); + [ + { + keyword: 'def', + line: 'def foo:', + dedent: false + }, + { + keyword: 'class', + line: 'class TestClass:', + dedent: false + }, + { + keyword: 'for', + line: 'for item in items:', + dedent: false + }, + { + keyword: 'if', + line: 'if foo is None:', + dedent: false + }, + { + keyword: 'elif', + line: 'elif x < 5:', + dedent: true + }, + { + keyword: 'else', + line: 'else:', + dedent: true + }, + { + keyword: 'while', + line: 'while \'::\' in macaddress:', + dedent: false + }, + { + keyword: 'try', + line: 'try:', + dedent: false + }, + { + keyword: 'with', + line: 'with self.test:', + dedent: false + }, + { + keyword: 'finally', + line: 'finally:', + dedent: false + }, + { + keyword: 'except', + line: 'except TestError:', + dedent: false + }, + { + keyword: 'async', + line: 'async def test(self):', + dedent: false + } + ].forEach(({keyword, line, dedent}) => { + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = INCREASE_INDENT_REGEX.test(line); + expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); + }); + + test(`Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`, async () => { + const result = DECREASE_INDENT_REGEX.test(line); + expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); + }); + }); + test('Increase indent regex should not pick up lines without keywords', async () => { + const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); + }); + test('Decrease indent regex should not pick up lines without keywords', async () => { + const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); + }); }); From a47b1913606087d0e2528c5b3c0bdc00d6e8b503 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 4 Jul 2019 14:22:38 -0700 Subject: [PATCH 03/21] More descriptive comment (same as f5daf78) --- src/test/extension.unit.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/extension.unit.test.ts b/src/test/extension.unit.test.ts index ad153865635c..38008c47214e 100644 --- a/src/test/extension.unit.test.ts +++ b/src/test/extension.unit.test.ts @@ -10,7 +10,8 @@ import * as sinon from 'sinon'; import { buildApi } from '../client/api'; import { EXTENSION_ROOT_DIR } from '../client/common/constants'; -// Stub sourceMapSupport.initialize before we import from extension.ts (we don't actually need it) +// Stub sourceMapSupport.initialize before we import from extension.ts +// (it's called at the top of the file but we don't need it here) // tslint:disable-next-line: no-require-imports no-var-requires const sourceMapSupport = require('../client/sourceMapSupport'); sinon.stub(sourceMapSupport, 'initialize'); From a5c56a2d36fa684e360beb9346e87db40da14494 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 14:23:21 -0700 Subject: [PATCH 04/21] Move tests from extension to language config file --- src/client/language/languageConfiguration.ts | 14 +-- src/test/extension.unit.test.ts | 88 ------------------- .../languageConfiguration.unit.test.ts | 86 +++++++++++++++++- 3 files changed, 93 insertions(+), 95 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 19eebd8d0131..0bd9bdf093ea 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -6,17 +6,15 @@ import { IndentAction, languages } from 'vscode'; import { PYTHON_LANGUAGE } from '../common/constants'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; +export const INCREASE_INDENT_REGEX = /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/; +export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else)\b.*:\s*/; export function setLanguageConfiguration() { // Enable indentAction languages.setLanguageConfiguration(PYTHON_LANGUAGE, { onEnterRules: [ { - beforeText: /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/, - action: { indentAction: IndentAction.Indent } - }, - { - beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, + beforeText: /^(?!\s+\\)[^#\n]+\\\s*/, action: { indentAction: IndentAction.Indent } }, { @@ -29,6 +27,10 @@ export function setLanguageConfiguration() { afterText: /\s+$/, action: { indentAction: IndentAction.Outdent } } - ] + ], + indentationRules: { + increaseIndentPattern: INCREASE_INDENT_REGEX, + decreaseIndentPattern: DECREASE_INDENT_REGEX + } }); } diff --git a/src/test/extension.unit.test.ts b/src/test/extension.unit.test.ts index 38008c47214e..553be3f7121e 100644 --- a/src/test/extension.unit.test.ts +++ b/src/test/extension.unit.test.ts @@ -6,17 +6,9 @@ // tslint:disable:no-any import { expect } from 'chai'; -import * as sinon from 'sinon'; import { buildApi } from '../client/api'; import { EXTENSION_ROOT_DIR } from '../client/common/constants'; -// Stub sourceMapSupport.initialize before we import from extension.ts -// (it's called at the top of the file but we don't need it here) -// tslint:disable-next-line: no-require-imports no-var-requires -const sourceMapSupport = require('../client/sourceMapSupport'); -sinon.stub(sourceMapSupport, 'initialize'); -import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX } from '../client/extension'; - const expectedPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`; suite('Extension API Debugger', () => { @@ -30,84 +22,4 @@ suite('Extension API Debugger', () => { const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait']; expect(args).to.be.deep.equal(expectedArgs); }); - [ - { - keyword: 'def', - line: 'def foo:', - dedent: false - }, - { - keyword: 'class', - line: 'class TestClass:', - dedent: false - }, - { - keyword: 'for', - line: 'for item in items:', - dedent: false - }, - { - keyword: 'if', - line: 'if foo is None:', - dedent: false - }, - { - keyword: 'elif', - line: 'elif x < 5:', - dedent: true - }, - { - keyword: 'else', - line: 'else:', - dedent: true - }, - { - keyword: 'while', - line: 'while \'::\' in macaddress:', - dedent: false - }, - { - keyword: 'try', - line: 'try:', - dedent: false - }, - { - keyword: 'with', - line: 'with self.test:', - dedent: false - }, - { - keyword: 'finally', - line: 'finally:', - dedent: false - }, - { - keyword: 'except', - line: 'except TestError:', - dedent: false - }, - { - keyword: 'async', - line: 'async def test(self):', - dedent: false - } - ].forEach(({keyword, line, dedent}) => { - test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { - const result = INCREASE_INDENT_REGEX.test(line); - expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); - }); - - test(`Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`, async () => { - const result = DECREASE_INDENT_REGEX.test(line); - expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); - }); - }); - test('Increase indent regex should not pick up lines without keywords', async () => { - const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); - expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); - }); - test('Decrease indent regex should not pick up lines without keywords', async () => { - const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); - expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); - }); }); diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 1356995cfdf4..56f094749958 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -5,7 +5,11 @@ import { expect } from 'chai'; -import { MULTILINE_SEPARATOR_INDENT_REGEX } from '../../client/language/languageConfiguration'; +import { + DECREASE_INDENT_REGEX, + INCREASE_INDENT_REGEX, + MULTILINE_SEPARATOR_INDENT_REGEX +} from '../../client/language/languageConfiguration'; suite('Language configuration regexes', () => { test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { @@ -20,4 +24,84 @@ suite('Language configuration regexes', () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); expect (result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); }); + [ + { + keyword: 'def', + line: 'def foo:', + dedent: false + }, + { + keyword: 'class', + line: 'class TestClass:', + dedent: false + }, + { + keyword: 'for', + line: 'for item in items:', + dedent: false + }, + { + keyword: 'if', + line: 'if foo is None:', + dedent: false + }, + { + keyword: 'elif', + line: 'elif x < 5:', + dedent: true + }, + { + keyword: 'else', + line: 'else:', + dedent: true + }, + { + keyword: 'while', + line: 'while \'::\' in macaddress:', + dedent: false + }, + { + keyword: 'try', + line: 'try:', + dedent: false + }, + { + keyword: 'with', + line: 'with self.test:', + dedent: false + }, + { + keyword: 'finally', + line: 'finally:', + dedent: false + }, + { + keyword: 'except', + line: 'except TestError:', + dedent: false + }, + { + keyword: 'async', + line: 'async def test(self):', + dedent: false + } + ].forEach(({keyword, line, dedent}) => { + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = INCREASE_INDENT_REGEX.test(line); + expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); + }); + + test(`Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`, async () => { + const result = DECREASE_INDENT_REGEX.test(line); + expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); + }); + }); + test('Increase indent regex should not pick up lines without keywords', async () => { + const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); + }); + test('Decrease indent regex should not pick up lines without keywords', async () => { + const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); + expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); + }); }); From b0f4a28b95cfcfbc48a5ee2ead623c6c136a30a6 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 14:26:41 -0700 Subject: [PATCH 05/21] Add news file --- news/2 Fixes/6333.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/6333.md diff --git a/news/2 Fixes/6333.md b/news/2 Fixes/6333.md new file mode 100644 index 000000000000..9e16ab7c6400 --- /dev/null +++ b/news/2 Fixes/6333.md @@ -0,0 +1 @@ +Add regex to dedent elif and else keywords. From 24804b66d113f4f04eeef77f1870e900fe67f2a4 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 14:30:23 -0700 Subject: [PATCH 06/21] Use regex we just introduced --- src/client/language/languageConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 0bd9bdf093ea..cff44474c138 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -14,7 +14,7 @@ export function setLanguageConfiguration() { languages.setLanguageConfiguration(PYTHON_LANGUAGE, { onEnterRules: [ { - beforeText: /^(?!\s+\\)[^#\n]+\\\s*/, + beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, action: { indentAction: IndentAction.Indent } }, { From 2d4d30c250172e435b468fa5413c23d3d10fe326 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Mon, 8 Jul 2019 14:33:13 -0700 Subject: [PATCH 07/21] Space is free --- src/test/language/languageConfiguration.unit.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 56f094749958..7c6f34e5cdbe 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -16,14 +16,17 @@ suite('Language configuration regexes', () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); expect (result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); }); + test('Multiline separator indent regex should not pick up strings with escaped characters', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'hello \\n\''); expect (result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); }); + test('Multiline separator indent regex should pick up strings ending with a multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); expect (result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); }); + [ { keyword: 'def', @@ -85,7 +88,7 @@ suite('Language configuration regexes', () => { line: 'async def test(self):', dedent: false } - ].forEach(({keyword, line, dedent}) => { + ].forEach(({ keyword, line, dedent }) => { test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { const result = INCREASE_INDENT_REGEX.test(line); expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); @@ -96,10 +99,12 @@ suite('Language configuration regexes', () => { expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); }); }); + test('Increase indent regex should not pick up lines without keywords', async () => { const result = INCREASE_INDENT_REGEX.test('a = \'hello \\n \''); expect(result).to.be.equal(false, 'Increase indent regex should not pick up lines without keywords'); }); + test('Decrease indent regex should not pick up lines without keywords', async () => { const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); From 7dfd00e16611224da306d2504a99ba0a90c05107 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 9 Jul 2019 15:20:01 -0700 Subject: [PATCH 08/21] Update regexes and existing tests (so they pass) --- src/client/language/languageConfiguration.ts | 14 ++++++++++---- .../language/languageConfiguration.unit.test.ts | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index cff44474c138..9a7dc52a81cf 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -6,8 +6,10 @@ import { IndentAction, languages } from 'vscode'; import { PYTHON_LANGUAGE } from '../common/constants'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; -export const INCREASE_INDENT_REGEX = /^\s*(?:def|class|for|if|elif|else|while|try|with|finally|except|async)\b.*:\s*/; -export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else)\b.*:\s*/; +export const INCREASE_INDENT_REGEX = /^\s*(?:async|class|def|elif|else|except|finally|if|for|try|while|with)\b.*:\s*(#.*)?$/; +export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else|except|finally)\b.*:\s*(#.*)?$/; +export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s+(break|continue|pass|raise)\b.*(#.*)?$/; +export const OUTDENT_RETURN_REGEX = /^\s+(return)\b([^\[]|(\[.*\]))*(#.*)?$/; export function setLanguageConfiguration() { // Enable indentAction @@ -23,8 +25,12 @@ export function setLanguageConfiguration() { action: { indentAction: IndentAction.None, appendText: '# ' } }, { - beforeText: /^\s+(continue|break|return)\b.*/, - afterText: /\s+$/, + beforeText: OUTDENT_SINGLE_KEYWORD_REGEX, + action: { indentAction: IndentAction.Outdent } + }, + { + // Outdent the line following he return statement only if there is no dangling open bracket before the cursor. + beforeText: OUTDENT_RETURN_REGEX, action: { indentAction: IndentAction.Outdent } } ], diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 7c6f34e5cdbe..7b493a57e0e9 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -76,12 +76,12 @@ suite('Language configuration regexes', () => { { keyword: 'finally', line: 'finally:', - dedent: false + dedent: true }, { keyword: 'except', line: 'except TestError:', - dedent: false + dedent: true }, { keyword: 'async', From da32278f0f14d0855f50e1acc670a113ea20a56c Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 9 Jul 2019 16:01:50 -0700 Subject: [PATCH 09/21] Add support for open tuples and dictionaries --- src/client/language/languageConfiguration.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 9a7dc52a81cf..c7b70171f7f0 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -6,10 +6,10 @@ import { IndentAction, languages } from 'vscode'; import { PYTHON_LANGUAGE } from '../common/constants'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; -export const INCREASE_INDENT_REGEX = /^\s*(?:async|class|def|elif|else|except|finally|if|for|try|while|with)\b.*:\s*(#.*)?$/; +export const INCREASE_INDENT_REGEX = /^\s*(?:async|class|def|elif|else|except|finally|for|if|try|while|with)\b.*:\s*(#.*)?$/; export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else|except|finally)\b.*:\s*(#.*)?$/; -export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s+(break|continue|pass|raise)\b.*(#.*)?$/; -export const OUTDENT_RETURN_REGEX = /^\s+(return)\b([^\[]|(\[.*\]))*(#.*)?$/; +export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise)\b.*(#.*)?$/; +export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^\[\(\{})]|([\[\(\{].*[\]\)\}]))*(#.*)?$/; export function setLanguageConfiguration() { // Enable indentAction From 2e8e5780b155d28ca2f8f55409784504ce1364f9 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 9 Jul 2019 16:02:02 -0700 Subject: [PATCH 10/21] Shuffle and add tests --- .../languageConfiguration.unit.test.ts | 171 ++++++++++++++---- 1 file changed, 137 insertions(+), 34 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 7b493a57e0e9..cc7c47b64838 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -8,94 +8,97 @@ import { expect } from 'chai'; import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, - MULTILINE_SEPARATOR_INDENT_REGEX + MULTILINE_SEPARATOR_INDENT_REGEX, + OUTDENT_RETURN_REGEX, + OUTDENT_SINGLE_KEYWORD_REGEX } from '../../client/language/languageConfiguration'; +// tslint:disable-next-line: max-func-body-length suite('Language configuration regexes', () => { test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); - expect (result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); + expect(result).to.be.equal(false, 'Multiline separator indent regex for regular strings should not have matches'); }); test('Multiline separator indent regex should not pick up strings with escaped characters', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'hello \\n\''); - expect (result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); + expect(result).to.be.equal(false, 'Multiline separator indent regex for strings with escaped characters should not have matches'); }); test('Multiline separator indent regex should pick up strings ending with a multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = \'multiline \\'); - expect (result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); + expect(result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches'); }); [ { - keyword: 'def', - line: 'def foo:', + keyword: 'async', + example: 'async def test(self):', dedent: false }, { keyword: 'class', - line: 'class TestClass:', + example: 'class TestClass:', dedent: false }, { - keyword: 'for', - line: 'for item in items:', - dedent: false - }, - { - keyword: 'if', - line: 'if foo is None:', + keyword: 'def', + example: 'def foo(self, node, namespace=""):', dedent: false }, { keyword: 'elif', - line: 'elif x < 5:', + example: 'elif x < 5:', dedent: true }, { keyword: 'else', - line: 'else:', + example: 'else:', dedent: true }, { - keyword: 'while', - line: 'while \'::\' in macaddress:', - dedent: false + keyword: 'except', + example: 'except TestError:', + dedent: true }, { - keyword: 'try', - line: 'try:', + keyword: 'finally', + example: 'finally:', + dedent: true + }, + { + keyword: 'for', + example: 'for item in items:', dedent: false }, { - keyword: 'with', - line: 'with self.test:', + keyword: 'if', + example: 'if foo is None:', dedent: false }, { - keyword: 'finally', - line: 'finally:', - dedent: true + keyword: 'try', + example: 'try:', + dedent: false }, { - keyword: 'except', - line: 'except TestError:', - dedent: true + keyword: 'while', + example: 'while \'::\' in macaddress:', + dedent: false }, { - keyword: 'async', - line: 'async def test(self):', + keyword: 'with', + example: 'with self.test:', dedent: false } - ].forEach(({ keyword, line, dedent }) => { + ].forEach(({ keyword, example, dedent }) => { test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { - const result = INCREASE_INDENT_REGEX.test(line); + const result = INCREASE_INDENT_REGEX.test(example); expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); }); test(`Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`, async () => { - const result = DECREASE_INDENT_REGEX.test(line); + const result = DECREASE_INDENT_REGEX.test(example); expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); }); }); @@ -109,4 +112,104 @@ suite('Language configuration regexes', () => { const result = DECREASE_INDENT_REGEX.test('a = \'hello \\n \''); expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); }); + + [ + { + keyword: 'break', + example: ' break' + }, + { + keyword: 'continue', + example: '\t\t continue' + }, + { + keyword: 'pass', + example: ' pass' + }, + { + keyword: 'raise', + example: 'raise Exception(\'Unknown Exception\'' + } + ].forEach(({ keyword, example }) => { + const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; + test(testWithoutComments, () => { + const result = OUTDENT_SINGLE_KEYWORD_REGEX.test(example); + expect(result).to.be.equal(true, testWithoutComments); + }); + + const testWithComments = `Outdent regex on enter should pick up lines containing the ${keyword} keyword and ending with comments`; + test(testWithComments, () => { + const result = OUTDENT_SINGLE_KEYWORD_REGEX.test(`${example} # test comment`); + expect(result).to.be.equal(true, testWithComments); + }); + }); + + [ + { + type: 'number', + value: 3, + hasComment: true, + match: true + }, + { + type: 'boolean', + value: 'True', + hasComment: false, + match: true + }, + { + type: 'string', + value: '\'test\'', + hasComment: false, + match: true + }, + { + type: 'variable name', + value: 'hello', + hasComment: true, + match: true + }, + { + type: 'closed array', + value: '[ 1, 2, 3 ]', + hasComment: true, + match: true + }, + { + type: 'closed dictionary', + value: '{ "id": 23, "enabled": True }', + hasComment: true, + match: true + }, + { + type: 'closed tuple', + value: '( "test", 23, False )', + hasComment: false, + match: true + }, + { + type: 'dangling [', + value: '[', + hasComment: false, + match: false + }, + { + type: 'dangling {', + value: '{', + hasComment: false, + match: false + }, + { + type: 'dangling (', + value: '(', + hasComment: true, + match: false + } + ].forEach(({ type, value, hasComment, match }) => { + const testTitle = `Outdent return regex on enter should ${match ? '' : 'not '}pick up lines containing the return statement followed by a ${type}`; + test(testTitle, () => { + const result = OUTDENT_RETURN_REGEX.test(`return ${value} ${hasComment ? '# test comment' : ''}`); + expect(result).to.be.equal(match, testTitle); + }); + }); }); From 638d7d57ef88d07535033feed974a70bcd09fb11 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 9 Jul 2019 16:05:20 -0700 Subject: [PATCH 11/21] Typo + more descriptive key name --- src/client/language/languageConfiguration.ts | 2 +- .../languageConfiguration.unit.test.ts | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index c7b70171f7f0..488b38b6c0ba 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -29,7 +29,7 @@ export function setLanguageConfiguration() { action: { indentAction: IndentAction.Outdent } }, { - // Outdent the line following he return statement only if there is no dangling open bracket before the cursor. + // Outdent the line following the return statement only if there is no dangling open bracket before the cursor. beforeText: OUTDENT_RETURN_REGEX, action: { indentAction: IndentAction.Outdent } } diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index cc7c47b64838..be2dcd461676 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -147,68 +147,68 @@ suite('Language configuration regexes', () => { [ { type: 'number', - value: 3, + returnValue: 3, hasComment: true, match: true }, { type: 'boolean', - value: 'True', + returnValue: 'True', hasComment: false, match: true }, { type: 'string', - value: '\'test\'', + returnValue: '\'test\'', hasComment: false, match: true }, { type: 'variable name', - value: 'hello', + returnValue: 'hello', hasComment: true, match: true }, { type: 'closed array', - value: '[ 1, 2, 3 ]', + returnValue: '[ 1, 2, 3 ]', hasComment: true, match: true }, { type: 'closed dictionary', - value: '{ "id": 23, "enabled": True }', + returnValue: '{ "id": 23, "enabled": True }', hasComment: true, match: true }, { type: 'closed tuple', - value: '( "test", 23, False )', + returnValue: '( "test", 23, False )', hasComment: false, match: true }, { type: 'dangling [', - value: '[', + returnValue: '[', hasComment: false, match: false }, { type: 'dangling {', - value: '{', + returnValue: '{', hasComment: false, match: false }, { type: 'dangling (', - value: '(', + returnValue: '(', hasComment: true, match: false } - ].forEach(({ type, value, hasComment, match }) => { + ].forEach(({ type, returnValue, hasComment, match }) => { const testTitle = `Outdent return regex on enter should ${match ? '' : 'not '}pick up lines containing the return statement followed by a ${type}`; test(testTitle, () => { - const result = OUTDENT_RETURN_REGEX.test(`return ${value} ${hasComment ? '# test comment' : ''}`); + const result = OUTDENT_RETURN_REGEX.test(`return ${returnValue} ${hasComment ? '# test comment' : ''}`); expect(result).to.be.equal(match, testTitle); }); }); From 4dd0556c839e7ffcb95ec8f713a7fe157185724b Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 9 Jul 2019 16:10:01 -0700 Subject: [PATCH 12/21] Updated news file after new changes --- news/2 Fixes/6333.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/2 Fixes/6333.md b/news/2 Fixes/6333.md index 9e16ab7c6400..73755d0d5ce1 100644 --- a/news/2 Fixes/6333.md +++ b/news/2 Fixes/6333.md @@ -1 +1 @@ -Add regex to dedent elif and else keywords. +Clarify regexes used for decreasing indentation. From 722f8c6b03eaf908f6e603a48edb346531bd64e3 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Thu, 11 Jul 2019 12:02:28 -0700 Subject: [PATCH 13/21] Apply suggestions from code review (first batch) Co-Authored-By: Eric Snow --- src/client/language/languageConfiguration.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 488b38b6c0ba..224b23f1e6dc 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -6,9 +6,17 @@ import { IndentAction, languages } from 'vscode'; import { PYTHON_LANGUAGE } from '../common/constants'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; +/* +This does not handle all cases. However, it does handle nearly all usage. +Here's what it does not cover: + * the statement is split over multiple lines (and hence the ":" is on a different line) + * the code block is inlined (after the ":") + * there are multiple statements on the line (separated by semicolons) +Also note that `lambda` is purposefully excluded. +*/ export const INCREASE_INDENT_REGEX = /^\s*(?:async|class|def|elif|else|except|finally|for|if|try|while|with)\b.*:\s*(#.*)?$/; -export const DECREASE_INDENT_REGEX = /^\s*(?:elif|else|except|finally)\b.*:\s*(#.*)?$/; -export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise)\b.*(#.*)?$/; +export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; +export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise\b.*)\s*(#.*)?$/; export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^\[\(\{})]|([\[\(\{].*[\]\)\}]))*(#.*)?$/; export function setLanguageConfiguration() { From c8ec4ac07e540e4a9ffa252af28955ee2eb77bb2 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Thu, 11 Jul 2019 12:02:51 -0700 Subject: [PATCH 14/21] Update src/client/language/languageConfiguration.ts Co-Authored-By: Eric Snow --- src/client/language/languageConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 224b23f1e6dc..016b492b6979 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -14,7 +14,7 @@ Here's what it does not cover: * there are multiple statements on the line (separated by semicolons) Also note that `lambda` is purposefully excluded. */ -export const INCREASE_INDENT_REGEX = /^\s*(?:async|class|def|elif|else|except|finally|for|if|try|while|with)\b.*:\s*(#.*)?$/; +export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|else|finally|try))\s*:\s*(#.*)?$/; export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise\b.*)\s*(#.*)?$/; export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^\[\(\{})]|([\[\(\{].*[\]\)\}]))*(#.*)?$/; From c7b0f267e6b3153e487bd2ff07683c483f7c77fa Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 13:04:29 -0700 Subject: [PATCH 15/21] Third batch of review fixes --- src/client/extension.ts | 6 ++-- src/client/language/languageConfiguration.ts | 30 +++++++++----------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index a017a0ffb0b3..adf1dbfed115 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -40,7 +40,7 @@ import { registerTypes as appRegisterTypes } from './application/serviceRegistry import { IApplicationDiagnostics } from './application/types'; import { DebugService } from './common/application/debugService'; import { IApplicationShell, ICommandManager, IWorkspaceService } from './common/application/types'; -import { Commands, isTestExecution, PYTHON, STANDARD_OUTPUT_CHANNEL } from './common/constants'; +import { Commands, isTestExecution, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants'; import { registerTypes as registerDotNetTypes } from './common/dotnet/serviceRegistry'; import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; import { traceError } from './common/logger'; @@ -85,7 +85,7 @@ import { registerTypes as interpretersRegisterTypes } from './interpreter/servic import { ServiceContainer } from './ioc/container'; import { ServiceManager } from './ioc/serviceManager'; import { IServiceContainer, IServiceManager } from './ioc/types'; -import { setLanguageConfiguration } from './language/languageConfiguration'; +import { getLanguageConfiguration } from './language/languageConfiguration'; import { LinterCommands } from './linters/linterCommands'; import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry'; import { ILintingEngine } from './linters/types'; @@ -173,7 +173,7 @@ async function activateUnsafe(context: ExtensionContext): Promise const linterProvider = new LinterProvider(context, serviceManager); context.subscriptions.push(linterProvider); - setLanguageConfiguration(); + languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration()); if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') { const formatProvider = new PythonFormattingEditProvider(context, serviceContainer); diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 016b492b6979..44712a2e5950 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -2,26 +2,24 @@ // Licensed under the MIT License. 'use strict'; -import { IndentAction, languages } from 'vscode'; -import { PYTHON_LANGUAGE } from '../common/constants'; +import { IndentAction } from 'vscode'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; -/* -This does not handle all cases. However, it does handle nearly all usage. -Here's what it does not cover: - * the statement is split over multiple lines (and hence the ":" is on a different line) - * the code block is inlined (after the ":") - * there are multiple statements on the line (separated by semicolons) -Also note that `lambda` is purposefully excluded. -*/ -export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|else|finally|try))\s*:\s*(#.*)?$/; +/** + * This does not handle all cases. However, it does handle nearly all usage. + * Here's what it does not cover: + * - the statement is split over multiple lines (and hence the ":" is on a different line) + * - the code block is inlined (after the ":") + * - there are multiple statements on the line (separated by semicolons) + * Also note that `lambda` is purposefully excluded. + */ +export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/; export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise\b.*)\s*(#.*)?$/; -export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^\[\(\{})]|([\[\(\{].*[\]\)\}]))*(#.*)?$/; +export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^[({]|([[({].*[\])}]))*(#.*)?$/; -export function setLanguageConfiguration() { - // Enable indentAction - languages.setLanguageConfiguration(PYTHON_LANGUAGE, { +export function getLanguageConfiguration() { + return { onEnterRules: [ { beforeText: MULTILINE_SEPARATOR_INDENT_REGEX, @@ -46,5 +44,5 @@ export function setLanguageConfiguration() { increaseIndentPattern: INCREASE_INDENT_REGEX, decreaseIndentPattern: DECREASE_INDENT_REGEX } - }); + }; } From 5b52d4c70d1c14fd7ad376b6792648603c22c6dd Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 14:03:01 -0700 Subject: [PATCH 16/21] Add some comments --- src/client/language/languageConfiguration.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 44712a2e5950..45b124de126a 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -6,7 +6,7 @@ import { IndentAction } from 'vscode'; export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; /** - * This does not handle all cases. However, it does handle nearly all usage. + * This does not handle all cases. However, it does handle nearly all usage. * Here's what it does not cover: * - the statement is split over multiple lines (and hence the ":" is on a different line) * - the code block is inlined (after the ":") @@ -16,6 +16,10 @@ export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/; export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise\b.*)\s*(#.*)?$/; +/** + * Dedent the line following a return statement only if there is no dangling open array, tuple or dictionary before the cursor. + * A line with a closed array, tuple or dictionary will be dedented correctly. + */ export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^[({]|([[({].*[\])}]))*(#.*)?$/; export function getLanguageConfiguration() { @@ -35,7 +39,6 @@ export function getLanguageConfiguration() { action: { indentAction: IndentAction.Outdent } }, { - // Outdent the line following the return statement only if there is no dangling open bracket before the cursor. beforeText: OUTDENT_RETURN_REGEX, action: { indentAction: IndentAction.Outdent } } From 5afb5deac6a79bec0e34a3d482381ce65656a54a Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 14:10:50 -0700 Subject: [PATCH 17/21] Separate unit tests --- .../languageConfiguration.unit.test.ts | 139 +++++++----------- 1 file changed, 52 insertions(+), 87 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index be2dcd461676..5915a2eabbd6 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -31,75 +31,39 @@ suite('Language configuration regexes', () => { }); [ - { - keyword: 'async', - example: 'async def test(self):', - dedent: false - }, - { - keyword: 'class', - example: 'class TestClass:', - dedent: false - }, - { - keyword: 'def', - example: 'def foo(self, node, namespace=""):', - dedent: false - }, - { - keyword: 'elif', - example: 'elif x < 5:', - dedent: true - }, - { - keyword: 'else', - example: 'else:', - dedent: true - }, - { - keyword: 'except', - example: 'except TestError:', - dedent: true - }, - { - keyword: 'finally', - example: 'finally:', - dedent: true - }, - { - keyword: 'for', - example: 'for item in items:', - dedent: false - }, - { - keyword: 'if', - example: 'if foo is None:', - dedent: false - }, - { - keyword: 'try', - example: 'try:', - dedent: false - }, - { - keyword: 'while', - example: 'while \'::\' in macaddress:', - dedent: false - }, - { - keyword: 'with', - example: 'with self.test:', - dedent: false - } - ].forEach(({ keyword, example, dedent }) => { + 'async def test(self):', + 'class TestClass:', + 'def foo(self, node, namespace=""):', + 'for item in items:', + 'if foo is None:', + 'try:', + 'while \'::\' in macaddress:', + 'with self.test:' + ].forEach(example => { + const keyword = example.split(' ')[0]; + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { const result = INCREASE_INDENT_REGEX.test(example); expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); }); - test(`Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`, async () => { + test(`Decrease indent regex should not pick up lines containing the ${keyword} keyword`, async () => { const result = DECREASE_INDENT_REGEX.test(example); - expect(result).to.be.equal(dedent, `Decrease indent regex should ${dedent ? '' : 'not '}pick up lines containing the ${keyword} keyword`); + expect(result).to.be.equal(false, `Decrease indent regex should not pick up lines containing the ${keyword} keyword`); + }); + }); + + ['elif x < 5:', 'else:', 'except TestError:', 'finally:'].forEach(example => { + const keyword = example.split(' ')[0]; + + test(`Increase indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = INCREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(true, `Increase indent regex should pick up lines containing the ${keyword} keyword`); + }); + + test(`Decrease indent regex should pick up lines containing the ${keyword} keyword`, async () => { + const result = DECREASE_INDENT_REGEX.test(example); + expect(result).to.be.equal(true, `Decrease indent regex should pick up lines containing the ${keyword} keyword`); }); }); @@ -148,68 +112,69 @@ suite('Language configuration regexes', () => { { type: 'number', returnValue: 3, - hasComment: true, - match: true + hasComment: true }, { type: 'boolean', returnValue: 'True', - hasComment: false, - match: true + hasComment: false }, { type: 'string', returnValue: '\'test\'', - hasComment: false, - match: true + hasComment: false }, { type: 'variable name', returnValue: 'hello', - hasComment: true, - match: true + hasComment: true }, { type: 'closed array', returnValue: '[ 1, 2, 3 ]', - hasComment: true, - match: true + hasComment: true }, { type: 'closed dictionary', returnValue: '{ "id": 23, "enabled": True }', - hasComment: true, - match: true + hasComment: true }, { type: 'closed tuple', returnValue: '( "test", 23, False )', - hasComment: false, - match: true - }, + hasComment: false + } + ].forEach(({ type, returnValue, hasComment }) => { + const testTitle = `Outdent return regex on enter should pick up lines containing the return statement followed by a ${type}`; + test(testTitle, () => { + const result = OUTDENT_RETURN_REGEX.test(`return ${returnValue} ${hasComment ? '# test comment' : ''}`); + expect(result).to.be.equal(true, testTitle); + }); + }); + + [ { - type: 'dangling [', - returnValue: '[', + returnValue: '[ 1, 2', hasComment: false, match: false }, { - type: 'dangling {', - returnValue: '{', + returnValue: '{ ', hasComment: false, match: false }, { - type: 'dangling (', - returnValue: '(', + returnValue: '( False', hasComment: true, match: false } - ].forEach(({ type, returnValue, hasComment, match }) => { - const testTitle = `Outdent return regex on enter should ${match ? '' : 'not '}pick up lines containing the return statement followed by a ${type}`; + ].forEach(({ returnValue, hasComment }) => { + const type = returnValue[0]; + + const testTitle = `Outdent return regex on enter should not pick up lines containing the return statement followed by a dangling ${type}`; test(testTitle, () => { const result = OUTDENT_RETURN_REGEX.test(`return ${returnValue} ${hasComment ? '# test comment' : ''}`); - expect(result).to.be.equal(match, testTitle); + expect(result).to.be.equal(false, testTitle); }); }); }); From 9123ab832ab09987535223cf057e2761b64662fc Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 14:15:20 -0700 Subject: [PATCH 18/21] More test simplification --- .../languageConfiguration.unit.test.ts | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 5915a2eabbd6..fe6426842254 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -77,24 +77,9 @@ suite('Language configuration regexes', () => { expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); }); - [ - { - keyword: 'break', - example: ' break' - }, - { - keyword: 'continue', - example: '\t\t continue' - }, - { - keyword: 'pass', - example: ' pass' - }, - { - keyword: 'raise', - example: 'raise Exception(\'Unknown Exception\'' - } - ].forEach(({ keyword, example }) => { + [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\''].forEach(example => { + const keyword = example.trim().split(' ')[0]; + const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; test(testWithoutComments, () => { const result = OUTDENT_SINGLE_KEYWORD_REGEX.test(example); From 278596cfcf8ac312878ae1c66bb250696b1ab28d Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 15:47:41 -0700 Subject: [PATCH 19/21] Simplify return regex --- src/client/language/languageConfiguration.ts | 13 +-- .../languageConfiguration.unit.test.ts | 84 +------------------ 2 files changed, 6 insertions(+), 91 deletions(-) diff --git a/src/client/language/languageConfiguration.ts b/src/client/language/languageConfiguration.ts index 45b124de126a..fa8f86d966f3 100644 --- a/src/client/language/languageConfiguration.ts +++ b/src/client/language/languageConfiguration.ts @@ -15,12 +15,7 @@ export const MULTILINE_SEPARATOR_INDENT_REGEX = /^(?!\s+\\)[^#\n]+\\$/; */ export const INCREASE_INDENT_REGEX = /^\s*(?:(?:async|class|def|elif|except|for|if|while|with)\b.*|(else|finally|try))\s*:\s*(#.*)?$/; export const DECREASE_INDENT_REGEX = /^\s*(?:else|finally|(?:elif|except)\b.*)\s*:\s*(#.*)?$/; -export const OUTDENT_SINGLE_KEYWORD_REGEX = /^\s*(break|continue|pass|raise\b.*)\s*(#.*)?$/; -/** - * Dedent the line following a return statement only if there is no dangling open array, tuple or dictionary before the cursor. - * A line with a closed array, tuple or dictionary will be dedented correctly. - */ -export const OUTDENT_RETURN_REGEX = /^\s*(return)\b([^[({]|([[({].*[\])}]))*(#.*)?$/; +export const OUTDENT_ONENTER_REGEX = /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/; export function getLanguageConfiguration() { return { @@ -35,11 +30,7 @@ export function getLanguageConfiguration() { action: { indentAction: IndentAction.None, appendText: '# ' } }, { - beforeText: OUTDENT_SINGLE_KEYWORD_REGEX, - action: { indentAction: IndentAction.Outdent } - }, - { - beforeText: OUTDENT_RETURN_REGEX, + beforeText: OUTDENT_ONENTER_REGEX, action: { indentAction: IndentAction.Outdent } } ], diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index fe6426842254..3a198f52a9f3 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -5,13 +5,7 @@ import { expect } from 'chai'; -import { - DECREASE_INDENT_REGEX, - INCREASE_INDENT_REGEX, - MULTILINE_SEPARATOR_INDENT_REGEX, - OUTDENT_RETURN_REGEX, - OUTDENT_SINGLE_KEYWORD_REGEX -} from '../../client/language/languageConfiguration'; +import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, MULTILINE_SEPARATOR_INDENT_REGEX, OUTDENT_ONENTER_REGEX } from '../../client/language/languageConfiguration'; // tslint:disable-next-line: max-func-body-length suite('Language configuration regexes', () => { @@ -77,89 +71,19 @@ suite('Language configuration regexes', () => { expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); }); - [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\''].forEach(example => { + [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', ' return [ True, False, False ]'].forEach(example => { const keyword = example.trim().split(' ')[0]; const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`; test(testWithoutComments, () => { - const result = OUTDENT_SINGLE_KEYWORD_REGEX.test(example); + const result = OUTDENT_ONENTER_REGEX.test(example); expect(result).to.be.equal(true, testWithoutComments); }); const testWithComments = `Outdent regex on enter should pick up lines containing the ${keyword} keyword and ending with comments`; test(testWithComments, () => { - const result = OUTDENT_SINGLE_KEYWORD_REGEX.test(`${example} # test comment`); + const result = OUTDENT_ONENTER_REGEX.test(`${example} # test comment`); expect(result).to.be.equal(true, testWithComments); }); }); - - [ - { - type: 'number', - returnValue: 3, - hasComment: true - }, - { - type: 'boolean', - returnValue: 'True', - hasComment: false - }, - { - type: 'string', - returnValue: '\'test\'', - hasComment: false - }, - { - type: 'variable name', - returnValue: 'hello', - hasComment: true - }, - { - type: 'closed array', - returnValue: '[ 1, 2, 3 ]', - hasComment: true - }, - { - type: 'closed dictionary', - returnValue: '{ "id": 23, "enabled": True }', - hasComment: true - }, - { - type: 'closed tuple', - returnValue: '( "test", 23, False )', - hasComment: false - } - ].forEach(({ type, returnValue, hasComment }) => { - const testTitle = `Outdent return regex on enter should pick up lines containing the return statement followed by a ${type}`; - test(testTitle, () => { - const result = OUTDENT_RETURN_REGEX.test(`return ${returnValue} ${hasComment ? '# test comment' : ''}`); - expect(result).to.be.equal(true, testTitle); - }); - }); - - [ - { - returnValue: '[ 1, 2', - hasComment: false, - match: false - }, - { - returnValue: '{ ', - hasComment: false, - match: false - }, - { - returnValue: '( False', - hasComment: true, - match: false - } - ].forEach(({ returnValue, hasComment }) => { - const type = returnValue[0]; - - const testTitle = `Outdent return regex on enter should not pick up lines containing the return statement followed by a dangling ${type}`; - test(testTitle, () => { - const result = OUTDENT_RETURN_REGEX.test(`return ${returnValue} ${hasComment ? '# test comment' : ''}`); - expect(result).to.be.equal(false, testTitle); - }); - }); }); From 43aa0e2c1a6714e056474fe46565fcab395fcd8d Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Jul 2019 16:10:29 -0700 Subject: [PATCH 20/21] Remove unnecessary tslint disable rule --- src/test/language/languageConfiguration.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 3a198f52a9f3..2d8779b4bfcc 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -7,7 +7,6 @@ import { expect } from 'chai'; import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, MULTILINE_SEPARATOR_INDENT_REGEX, OUTDENT_ONENTER_REGEX } from '../../client/language/languageConfiguration'; -// tslint:disable-next-line: max-func-body-length suite('Language configuration regexes', () => { test('Multiline separator indent regex should not pick up strings with no multiline separator', async () => { const result = MULTILINE_SEPARATOR_INDENT_REGEX.test('a = "test"'); From 0b3e34c0065706b54f1b8cb1126903592fcaf576 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 12 Jul 2019 08:56:11 -0700 Subject: [PATCH 21/21] pipeline is stuck --- src/test/language/languageConfiguration.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/language/languageConfiguration.unit.test.ts b/src/test/language/languageConfiguration.unit.test.ts index 2d8779b4bfcc..37433879daa8 100644 --- a/src/test/language/languageConfiguration.unit.test.ts +++ b/src/test/language/languageConfiguration.unit.test.ts @@ -70,7 +70,7 @@ suite('Language configuration regexes', () => { expect(result).to.be.equal(false, 'Decrease indent regex should not pick up lines without keywords'); }); - [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', ' return [ True, False, False ]'].forEach(example => { + [' break', '\t\t continue', ' pass', 'raise Exception(\'Unknown Exception\'', ' return [ True, False, False ]'].forEach(example => { const keyword = example.trim().split(' ')[0]; const testWithoutComments = `Outdent regex for on enter rule should pick up lines containing the ${keyword} keyword`;