Skip to content

Commit

Permalink
Revert "Add regex to dedent else and friends (microsoft#6497)" (mic…
Browse files Browse the repository at this point in the history 
…rosoft#6945)

This reverts commit 2454423.
  • Loading branch information
@kimadeline
kimadeline authored Aug 12, 2019
1 parent 9c61d0e commit 2402dd5
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 93 deletions.
6 changes: 3 additions & 3 deletions src/client/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from './common/constants';
import { Commands, isTestExecution, PYTHON, 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';
Expand Down Expand Up @@ -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 { getLanguageConfiguration } from './language/languageConfiguration';
import { setLanguageConfiguration } from './language/languageConfiguration';
import { LinterCommands } from './linters/linterCommands';
import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry';
import { ILintingEngine } from './linters/types';
Expand Down Expand Up @@ -176,7 +176,7 @@ async function activateUnsafe(context: ExtensionContext): Promise<IExtensionApi>
const linterProvider = new LinterProvider(context, serviceManager);
context.subscriptions.push(linterProvider);

languages.setLanguageConfiguration(PYTHON_LANGUAGE, getLanguageConfiguration());
setLanguageConfiguration();

if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') {
const formatProvider = new PythonFormattingEditProvider(context, serviceContainer);
Expand Down
34 changes: 13 additions & 21 deletions src/client/language/languageConfiguration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,19 @@
// Licensed under the MIT License.
'use strict';

import { IndentAction } from 'vscode';
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|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_ONENTER_REGEX = /^\s*(?:break|continue|pass|(?:raise|return)\b.*)\s*(#.*)?$/;

export function getLanguageConfiguration() {
return {
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,
action: { indentAction: IndentAction.Indent }
Expand All @@ -30,13 +25,10 @@ export function getLanguageConfiguration() {
action: { indentAction: IndentAction.None, appendText: '# ' }
},
{
beforeText: OUTDENT_ONENTER_REGEX,
beforeText: /^\s+(continue|break|return)\b.*/,
afterText: /\s+$/,
action: { indentAction: IndentAction.Outdent }
}
],
indentationRules: {
increaseIndentPattern: INCREASE_INDENT_REGEX,
decreaseIndentPattern: DECREASE_INDENT_REGEX
}
};
]
});
}
73 changes: 4 additions & 69 deletions src/test/language/languageConfiguration.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,84 +5,19 @@

import { expect } from 'chai';

import { DECREASE_INDENT_REGEX, INCREASE_INDENT_REGEX, MULTILINE_SEPARATOR_INDENT_REGEX, OUTDENT_ONENTER_REGEX } from '../../client/language/languageConfiguration';
import { 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 () => {
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');
});

[
'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 not pick up lines containing the ${keyword} keyword`, async () => {
const result = DECREASE_INDENT_REGEX.test(example);
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`);
});
});

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');
});

[' 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_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_ONENTER_REGEX.test(`${example} # test comment`);
expect(result).to.be.equal(true, testWithComments);
});
expect (result).to.be.equal(true, 'Multiline separator indent regex for strings with newline separator should have matches');
});
});

0 comments on commit 2402dd5

Please sign in to comment.