From d1da48b6f5a02203afcb4a4c90eea31c77aaeaf6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 3 Oct 2017 14:29:33 -0700 Subject: [PATCH 01/24] update to use latest api --- .gitignore | 2 ++ .vscode/launch.json | 9 ++++++--- package.json | 14 +++++++------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 81f31ac0a202..58ce4158ddbe 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ node_modules src/test/.vscode/** **/testFiles/**/.cache/** *.noseids +.vscode-test +__pycache__ diff --git a/.vscode/launch.json b/.vscode/launch.json index 0d64d8b63590..284cb90bbaa5 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -30,7 +30,9 @@ ], "stopOnEntry": false, "sourceMaps": true, - "outDir": "${workspaceRoot}/out", + "outFiles": [ + "${workspaceRoot}/out/**/*.js" + ], "preLaunchTask": "npm" }, { @@ -43,7 +45,9 @@ "--server=4711" ], "sourceMaps": true, - "outDir": "${workspaceRoot}/out/client", + "outFiles": [ + "${workspaceRoot}/out/client/**/*.js" + ], "cwd": "${workspaceRoot}" }, { @@ -58,7 +62,6 @@ ], "stopOnEntry": false, "sourceMaps": true, - "xxoutDir": "${workspaceRoot}/out/test", "outFiles": [ "${workspaceRoot}/out/**/*.js" ], diff --git a/package.json b/package.json index 3307ab32c2ed..bb660b4206c0 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.9.0" + "vscode": "^1.15.0" }, "recommendations": [ "donjayamanne.jupyter" @@ -1537,11 +1537,12 @@ "untildify": "^3.0.2", "vscode-debugadapter": "^1.0.1", "vscode-debugprotocol": "^1.0.1", - "vscode-extension-telemetry": "0.0.5", - "vscode-languageclient": "^1.1.0", - "vscode-languageserver": "^1.1.0", + "vscode-extension-telemetry": "^0.0.5", + "vscode-languageclient": "^3.1.0", + "vscode-languageserver": "^3.1.0", "winreg": "^1.2.4", - "xml2js": "^0.4.17" + "xml2js": "^0.4.17", + "vscode": "^1.0.3" }, "devDependencies": { "@types/fs-extra": "^4.0.2", @@ -1560,13 +1561,12 @@ "babel-loader": "^6.2.5", "babel-preset-es2015": "^6.14.0", "ignore-loader": "^0.1.1", - "mocha": "^2.3.3", + "mocha": "^3.2.0", "retyped-diff-match-patch-tsd-ambient": "^1.0.0-0", "sinon": "^2.3.6", "ts-loader": "^0.8.2", "tslint": "^3.15.1", "typescript": "^2.3.2", - "vscode": "^1.0.3", "webpack": "^1.13.2" } } From b305a72e0601ae1a9e3a30680fac6a2803dc36af Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 5 Oct 2017 15:47:14 -0700 Subject: [PATCH 02/24] config changes for multiroot workspace --- package.json | 243 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 162 insertions(+), 81 deletions(-) diff --git a/package.json b/package.json index d027214ce152..e19878709162 100644 --- a/package.json +++ b/package.json @@ -922,32 +922,38 @@ "python.promptToInstallJupyter": { "type": "boolean", "default": true, - "description": "Display prompt to install Jupyter Extension." + "description": "Display prompt to install Jupyter Extension.", + "scope": "resource" }, "python.pythonPath": { "type": "string", "default": "python", - "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path." + "description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.", + "scope": "resource" }, "python.venvPath": { "type": "string", "default": "", - "description": "Path to folder with a list of Virtual Environments (e.g. ~/.pyenv, ~/Envs, ~/.virtualenvs)." + "description": "Path to folder with a list of Virtual Environments (e.g. ~/.pyenv, ~/Envs, ~/.virtualenvs).", + "scope": "resource" }, "python.envFile": { "type": "string", "description": "Absolute path to a file containing environment variable definitions.", - "default": "${workspaceRoot}/.env" + "default": "${workspaceRoot}/.env", + "scope": "resource" }, "python.jediPath": { "type": "string", "default": "", - "description": "Path to directory containing the Jedi library (this path will contain the 'Jedi' sub directory)." + "description": "Path to directory containing the Jedi library (this path will contain the 'Jedi' sub directory).", + "scope": "resource" }, "python.sortImports.path": { "type": "string", "description": "Path to isort script, default using inner version", - "default": "" + "default": "", + "scope": "resource" }, "python.sortImports.args": { "type": "array", @@ -955,7 +961,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.disablePromptForFeatures": { "type": "array", @@ -974,62 +981,74 @@ "pydocstyle", "pylint" ] - } + }, + "scope": "resource" }, "python.linting.enabled": { "type": "boolean", "default": true, - "description": "Whether to lint Python files." + "description": "Whether to lint Python files.", + "scope": "resource" }, "python.linting.enabledWithoutWorkspace": { "type": "boolean", "default": true, - "description": "Whether to lint Python files when no workspace is opened." + "description": "Whether to lint Python files when no workspace is opened.", + "scope": "resource" }, "python.linting.prospectorEnabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using prospector." + "description": "Whether to lint Python files using prospector.", + "scope": "resource" }, "python.linting.pylintEnabled": { "type": "boolean", "default": true, - "description": "Whether to lint Python files using pylint." + "description": "Whether to lint Python files using pylint.", + "scope": "resource" }, "python.linting.pep8Enabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using pep8" + "description": "Whether to lint Python files using pep8", + "scope": "resource" }, "python.linting.flake8Enabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using flake8" + "description": "Whether to lint Python files using flake8", + "scope": "resource" }, "python.linting.pydocstyleEnabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using pydocstyle" + "description": "Whether to lint Python files using pydocstyle", + "scope": "resource" }, "python.linting.mypyEnabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using mypy." + "description": "Whether to lint Python files using mypy.", + "scope": "resource" }, "python.linting.lintOnTextChange": { "type": "boolean", "default": true, - "description": "Whether to lint Python files when modified." + "description": "Whether to lint Python files when modified.", + "scope": "resource" }, "python.linting.lintOnSave": { "type": "boolean", "default": true, - "description": "Whether to lint Python files when saved." + "description": "Whether to lint Python files when saved.", + "scope": "resource" }, "python.linting.maxNumberOfProblems": { "type": "number", "default": 100, - "description": "Controls the maximum number of problems produced by the server." + "description": "Controls the maximum number of problems produced by the server.", + "scope": "resource" }, "python.linting.pylintCategorySeverity.convention": { "type": "string", @@ -1040,7 +1059,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pylintCategorySeverity.refactor": { "type": "string", @@ -1051,7 +1071,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pylintCategorySeverity.warning": { "type": "string", @@ -1062,7 +1083,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pylintCategorySeverity.error": { "type": "string", @@ -1073,7 +1095,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pylintCategorySeverity.fatal": { "type": "string", @@ -1084,7 +1107,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pep8CategorySeverity.W": { "type": "string", @@ -1095,7 +1119,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.pep8CategorySeverity.E": { "type": "string", @@ -1106,7 +1131,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.flake8CategorySeverity.F": { "type": "string", @@ -1117,7 +1143,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.flake8CategorySeverity.E": { "type": "string", @@ -1128,7 +1155,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.flake8CategorySeverity.W": { "type": "string", @@ -1139,7 +1167,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.mypyCategorySeverity.error": { "type": "string", @@ -1150,7 +1179,8 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.mypyCategorySeverity.note": { "type": "string", @@ -1161,37 +1191,44 @@ "Error", "Information", "Warning" - ] + ], + "scope": "resource" }, "python.linting.prospectorPath": { "type": "string", "default": "prospector", - "description": "Path to Prospector, you can use a custom version of prospector by modifying this setting to include the full path." + "description": "Path to Prospector, you can use a custom version of prospector by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.pylintPath": { "type": "string", "default": "pylint", - "description": "Path to Pylint, you can use a custom version of pylint by modifying this setting to include the full path." + "description": "Path to Pylint, you can use a custom version of pylint by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.pep8Path": { "type": "string", "default": "pep8", - "description": "Path to pep8, you can use a custom version of pep8 by modifying this setting to include the full path." + "description": "Path to pep8, you can use a custom version of pep8 by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.flake8Path": { "type": "string", "default": "flake8", - "description": "Path to flake8, you can use a custom version of flake8 by modifying this setting to include the full path." + "description": "Path to flake8, you can use a custom version of flake8 by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.pydocstylePath": { "type": "string", "default": "pydocstyle", - "description": "Path to pydocstyle, you can use a custom version of pydocstyle by modifying this setting to include the full path." + "description": "Path to pydocstyle, you can use a custom version of pydocstyle by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.mypyPath": { "type": "string", "default": "mypy", - "description": "Path to mypy, you can use a custom version of mypy by modifying this setting to include the full path." + "description": "Path to mypy, you can use a custom version of mypy by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.prospectorArgs": { "type": "array", @@ -1199,7 +1236,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.pylintArgs": { "type": "array", @@ -1207,7 +1245,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.pep8Args": { "type": "array", @@ -1215,7 +1254,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.flake8Args": { "type": "array", @@ -1223,7 +1263,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.pydocstyleArgs": { "type": "array", @@ -1231,7 +1272,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.mypyArgs": { "type": "array", @@ -1242,12 +1284,14 @@ ], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.outputWindow": { "type": "string", "default": "Python", - "description": "The output window name for the linting messages, defaults to Python output window." + "description": "The output window name for the linting messages, defaults to Python output window.", + "scope": "resource" }, "python.formatting.provider": { "type": "string", @@ -1257,17 +1301,20 @@ "autopep8", "yapf", "none" - ] + ], + "scope": "resource" }, "python.formatting.autopep8Path": { "type": "string", "default": "autopep8", - "description": "Path to autopep8, you can use a custom version of autopep8 by modifying this setting to include the full path." + "description": "Path to autopep8, you can use a custom version of autopep8 by modifying this setting to include the full path.", + "scope": "resource" }, "python.formatting.yapfPath": { "type": "string", "default": "yapf", - "description": "Path to yapf, you can use a custom version of yapf by modifying this setting to include the full path." + "description": "Path to yapf, you can use a custom version of yapf by modifying this setting to include the full path.", + "scope": "resource" }, "python.formatting.autopep8Args": { "type": "array", @@ -1275,7 +1322,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.formatting.yapfArgs": { "type": "array", @@ -1283,17 +1331,20 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.formatting.formatOnSave": { "type": "boolean", "default": false, - "description": "Format the document upon saving." + "description": "Format the document upon saving.", + "scope": "resource" }, "python.formatting.outputWindow": { "type": "string", "default": "Python", - "description": "The output window name for the formatting messages, defaults to Python output window." + "description": "The output window name for the formatting messages, defaults to Python output window.", + "scope": "resource" }, "python.autoComplete.preloadModules": { "type": "array", @@ -1301,42 +1352,50 @@ "type": "string" }, "default": [], - "description": "Comma delimited list of modules preloaded to speed up Auto Complete (e.g. add Numpy, Pandas, etc, items slow to load when autocompleting)." + "description": "Comma delimited list of modules preloaded to speed up Auto Complete (e.g. add Numpy, Pandas, etc, items slow to load when autocompleting).", + "scope": "resource" }, "python.autoComplete.extraPaths": { "type": "array", "default": [], - "description": "List of paths to libraries and the like that need to be imported by auto complete engine. E.g. when using Google App SDK, the paths are not in system path, hence need to be added into this list." + "description": "List of paths to libraries and the like that need to be imported by auto complete engine. E.g. when using Google App SDK, the paths are not in system path, hence need to be added into this list.", + "scope": "resource" }, "python.autoComplete.addBrackets": { "type": "boolean", "default": false, - "description": "Automatically add brackets for functions." + "description": "Automatically add brackets for functions.", + "scope": "resource" }, "python.workspaceSymbols.tagFilePath": { "type": "string", "default": "${workspaceRoot}/.vscode/tags", - "description": "Fully qualified path to tag file (exuberant ctag file), used to provide workspace symbols." + "description": "Fully qualified path to tag file (exuberant ctag file), used to provide workspace symbols.", + "scope": "resource" }, "python.workspaceSymbols.enabled": { "type": "boolean", "default": true, - "description": "Set to 'false' to disable Workspace Symbol provider using ctags." + "description": "Set to 'false' to disable Workspace Symbol provider using ctags.", + "scope": "resource" }, "python.workspaceSymbols.rebuildOnStart": { "type": "boolean", "default": true, - "description": "Whether to re-build the tags file on start (defaults to true)." + "description": "Whether to re-build the tags file on start (defaults to true).", + "scope": "resource" }, "python.workspaceSymbols.rebuildOnFileSave": { "type": "boolean", "default": true, - "description": "Whether to re-build the tags file on when changes made to python files are saved." + "description": "Whether to re-build the tags file on when changes made to python files are saved.", + "scope": "resource" }, "python.workspaceSymbols.ctagsPath": { "type": "string", "default": "ctags", - "description": "Fully qualilified path to the ctags executable (else leave as ctags, assuming it is in current path)." + "description": "Fully qualilified path to the ctags executable (else leave as ctags, assuming it is in current path).", + "scope": "resource" }, "python.workspaceSymbols.exclusionPatterns": { "type": "array", @@ -1346,42 +1405,50 @@ "items": { "type": "string" }, - "description": "Pattern used to exclude files and folders from ctags See http://ctags.sourceforge.net/ctags.html." + "description": "Pattern used to exclude files and folders from ctags See http://ctags.sourceforge.net/ctags.html.", + "scope": "resource" }, "python.unitTest.promptToConfigure": { "type": "boolean", "default": true, - "description": "Where to prompt to configure a test framework if potential tests directories are discovered." + "description": "Where to prompt to configure a test framework if potential tests directories are discovered.", + "scope": "resource" }, "python.unitTest.debugPort": { "type": "number", "default": 3000, - "description": "Port number used for debugging of unittests." + "description": "Port number used for debugging of unittests.", + "scope": "resource" }, "python.unitTest.cwd": { "type": "string", "default": null, - "description": "Optional working directory for unit tests." + "description": "Optional working directory for unit tests.", + "scope": "resource" }, "python.unitTest.nosetestsEnabled": { "type": "boolean", "default": false, - "description": "Whether to enable or disable unit testing using nosetests." + "description": "Whether to enable or disable unit testing using nosetests.", + "scope": "resource" }, "python.unitTest.nosetestPath": { "type": "string", "default": "nosetests", - "description": "Path to nosetests, you can use a custom version of nosetests by modifying this setting to include the full path." + "description": "Path to nosetests, you can use a custom version of nosetests by modifying this setting to include the full path.", + "scope": "resource" }, "python.unitTest.pyTestEnabled": { "type": "boolean", "default": false, - "description": "Whether to enable or disable unit testing using pytest." + "description": "Whether to enable or disable unit testing using pytest.", + "scope": "resource" }, "python.unitTest.pyTestPath": { "type": "string", "default": "py.test", - "description": "Path to pytest (py.test), you can use a custom version of pytest by modifying this setting to include the full path." + "description": "Path to pytest (py.test), you can use a custom version of pytest by modifying this setting to include the full path.", + "scope": "resource" }, "python.unitTest.nosetestArgs": { "type": "array", @@ -1389,7 +1456,8 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.unitTest.pyTestArgs": { "type": "array", @@ -1397,12 +1465,14 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.unitTest.unittestEnabled": { "type": "boolean", "default": false, - "description": "Whether to enable or disable unit testing using unittest." + "description": "Whether to enable or disable unit testing using unittest.", + "scope": "resource" }, "python.unitTest.unittestArgs": { "type": "array", @@ -1416,7 +1486,8 @@ ], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.ignorePatterns": { "type": "array", @@ -1427,17 +1498,20 @@ ], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.linting.pylamaEnabled": { "type": "boolean", "default": false, - "description": "Whether to lint Python files using pylama." + "description": "Whether to lint Python files using pylama.", + "scope": "resource" }, "python.linting.pylamaPath": { "type": "string", "default": "pylama", - "description": "Path to pylama, you can use a custom version of pylama by modifying this setting to include the full path." + "description": "Path to pylama, you can use a custom version of pylama by modifying this setting to include the full path.", + "scope": "resource" }, "python.linting.pylamaArgs": { "type": "array", @@ -1445,32 +1519,38 @@ "default": [], "items": { "type": "string" - } + }, + "scope": "resource" }, "python.unitTest.outputWindow": { "type": "string", "default": "Python Test Log", - "description": "The output window name for the unit test messages, defaults to Python output window." + "description": "The output window name for the unit test messages, defaults to Python output window.", + "scope": "resource" }, "python.terminal.executeInFileDir": { "type": "boolean", "default": false, - "description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder." + "description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", + "scope": "resource" }, "python.terminal.launchArgs": { "type": "array", "default": [], - "description": "Python launch arguments to use when executing a file in the terminal." + "description": "Python launch arguments to use when executing a file in the terminal.", + "scope": "resource" }, "python.jupyter.appendResults": { "type": "boolean", "default": true, - "description": "Whether to appen the results to results window, else clear and display." + "description": "Whether to appen the results to results window, else clear and display.", + "scope": "resource" }, "python.jupyter.defaultKernel": { "type": "string", "default": "", - "description": "Default kernel to be used. By default the first available kernel is used." + "description": "Default kernel to be used. By default the first available kernel is used.", + "scope": "resource" }, "python.jupyter.startupCode": { "type": "array", @@ -1480,7 +1560,8 @@ "default": [ "%matplotlib inline" ], - "description": "Code executed when the kernel starts. Such as the default of '%matplotlib inline'. Individual lines can be placed in separate items of the array." + "description": "Code executed when the kernel starts. Such as the default of '%matplotlib inline'. Individual lines can be placed in separate items of the array.", + "scope": "resource" } } }, From 674da9eefa0d1a93c281c82a6f614303ee523f3c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 5 Oct 2017 15:48:06 -0700 Subject: [PATCH 03/24] linting support with multi roots --- src/client/linters/baseLinter.ts | 10 +++++----- src/client/linters/flake8.ts | 6 +++--- src/client/linters/main.ts | 18 +++++++++--------- src/client/linters/mypy.ts | 8 ++++---- src/client/linters/pep8Linter.ts | 6 +++--- src/client/linters/prospector.ts | 6 +++--- src/client/linters/pydocstyle.ts | 6 +++--- src/client/linters/pylama.ts | 6 +++--- src/client/linters/pylint.ts | 8 ++++---- src/client/providers/lintProvider.ts | 6 ++++-- src/test/linters/lint.test.ts | 26 +++++++++++++------------- 11 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index f4c80314c1f2..b4cd6ca154ed 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -50,15 +50,15 @@ export function matchNamedRegEx(data, regex): IRegexGroup { export abstract class BaseLinter { public Id: string; protected pythonSettings: settings.IPythonSettings; - private _workspaceRootPath: string; protected _columnOffset = 0; private _errorHandler: ErrorHandler; - protected get workspaceRootPath(): string { - return typeof this._workspaceRootPath === 'string' ? this._workspaceRootPath : vscode.workspace.rootPath; + protected getWorkspaceRootPath(document: vscode.TextDocument): string { + const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined; + return typeof workspaceRootPath === 'string' ? workspaceRootPath : __dirname; } - constructor(id: string, public product: Product, protected outputChannel: OutputChannel, workspaceRootPath: string) { + constructor(id: string, public product: Product, protected outputChannel: OutputChannel) { this.Id = id; - this._workspaceRootPath = workspaceRootPath; this.pythonSettings = settings.PythonSettings.getInstance(); this._errorHandler = new ErrorHandler(this.Id, product, new Installer(), this.outputChannel); } diff --git a/src/client/linters/flake8.ts b/src/client/linters/flake8.ts index e2d574c63d88..b141b3a3a86c 100644 --- a/src/client/linters/flake8.ts +++ b/src/client/linters/flake8.ts @@ -8,8 +8,8 @@ import { TextDocument, CancellationToken } from 'vscode'; export class Linter extends baseLinter.BaseLinter { _columnOffset = 1; - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('flake8', Product.flake8, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('flake8', Product.flake8, outputChannel); } public isEnabled(): Boolean { @@ -29,7 +29,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise((resolve, reject) => { - this.run(flake8Path, flake8Args.concat(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath]), document, this.workspaceRootPath, cancellation).then(messages => { + this.run(flake8Path, flake8Args.concat(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation).then(messages => { messages.forEach(msg => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.flake8CategorySeverity); }); diff --git a/src/client/linters/main.ts b/src/client/linters/main.ts index a3d8691c3508..a54d75fc7df9 100644 --- a/src/client/linters/main.ts +++ b/src/client/linters/main.ts @@ -12,32 +12,32 @@ import * as pydocstyle from './../linters/pydocstyle'; import * as mypy from './../linters/mypy'; export class LinterFactor { - public static createLinter(product: Product, outputChannel: OutputChannel, workspaceRootPath: string = workspace.rootPath): BaseLinter { + public static createLinter(product: Product, outputChannel: OutputChannel): BaseLinter { switch (product) { case Product.flake8: { - return new flake8.Linter(outputChannel, workspaceRootPath); + return new flake8.Linter(outputChannel); } case Product.mypy: { - return new mypy.Linter(outputChannel, workspaceRootPath); + return new mypy.Linter(outputChannel); } case Product.pep8: { - return new pep8.Linter(outputChannel, workspaceRootPath); + return new pep8.Linter(outputChannel); } case Product.prospector: { - return new prospector.Linter(outputChannel, workspaceRootPath); + return new prospector.Linter(outputChannel); } case Product.pydocstyle: { - return new pydocstyle.Linter(outputChannel, workspaceRootPath); + return new pydocstyle.Linter(outputChannel); } case Product.pylama: { - return new pylama.Linter(outputChannel, workspaceRootPath); + return new pylama.Linter(outputChannel); } case Product.pylint: { - return new pylint.Linter(outputChannel, workspaceRootPath); + return new pylint.Linter(outputChannel); } default: { throw new Error(`Invalid Linter '${Product[product]}''`); } } } -} \ No newline at end of file +} diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts index db9d9d4c5edb..407c218bc08f 100644 --- a/src/client/linters/mypy.ts +++ b/src/client/linters/mypy.ts @@ -8,8 +8,8 @@ import { TextDocument, CancellationToken } from 'vscode'; const REGEX = '(?.py):(?\\d+): (?\\w+): (?.*)\\r?(\\n|$)'; export class Linter extends baseLinter.BaseLinter { - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('mypy', Product.mypy, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('mypy', Product.mypy, outputChannel); } public isEnabled(): Boolean { @@ -29,7 +29,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise((resolve, reject) => { - this.run(mypyPath, mypyArgs.concat([document.uri.fsPath]), document, this.workspaceRootPath, cancellation, REGEX).then(messages => { + this.run(mypyPath, mypyArgs.concat([document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation, REGEX).then(messages => { messages.forEach(msg => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity); msg.code = msg.type; @@ -39,4 +39,4 @@ export class Linter extends baseLinter.BaseLinter { }, reject); }); } -} \ No newline at end of file +} diff --git a/src/client/linters/pep8Linter.ts b/src/client/linters/pep8Linter.ts index 51a4170b1e34..bb42f3c30834 100644 --- a/src/client/linters/pep8Linter.ts +++ b/src/client/linters/pep8Linter.ts @@ -8,8 +8,8 @@ import { TextDocument, CancellationToken } from 'vscode'; export class Linter extends baseLinter.BaseLinter { _columnOffset = 1; - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('pep8', Product.pep8, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('pep8', Product.pep8, outputChannel); } public isEnabled(): Boolean { @@ -29,7 +29,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise(resolve => { - this.run(pep8Path, pep8Args.concat(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath]), document, this.workspaceRootPath, cancellation).then(messages => { + this.run(pep8Path, pep8Args.concat(['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation).then(messages => { messages.forEach(msg => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.pep8CategorySeverity); }); diff --git a/src/client/linters/prospector.ts b/src/client/linters/prospector.ts index 1d8a16544bbe..42c14dfd76e2 100644 --- a/src/client/linters/prospector.ts +++ b/src/client/linters/prospector.ts @@ -24,8 +24,8 @@ interface IProspectorLocation { } export class Linter extends baseLinter.BaseLinter { - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('prospector', Product.prospector, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('prospector', Product.prospector, outputChannel); } public isEnabled(): Boolean { @@ -46,7 +46,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise((resolve, reject) => { - execPythonFile(prospectorPath, prospectorArgs.concat(['--absolute-paths', '--output-format=json', document.uri.fsPath]), this.workspaceRootPath, false, null, cancellation).then(data => { + execPythonFile(prospectorPath, prospectorArgs.concat(['--absolute-paths', '--output-format=json', document.uri.fsPath]), this.getWorkspaceRootPath(document), false, null, cancellation).then(data => { let parsedData: IProspectorResponse; try { parsedData = JSON.parse(data); diff --git a/src/client/linters/pydocstyle.ts b/src/client/linters/pydocstyle.ts index 98f32bbe210b..b361bf1640ec 100644 --- a/src/client/linters/pydocstyle.ts +++ b/src/client/linters/pydocstyle.ts @@ -9,8 +9,8 @@ import { Product, ProductExecutableAndArgs } from '../common/installer'; import { TextDocument, CancellationToken } from 'vscode'; export class Linter extends baseLinter.BaseLinter { - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('pydocstyle', Product.pydocstyle, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('pydocstyle', Product.pydocstyle, outputChannel); } public isEnabled(): Boolean { @@ -45,7 +45,7 @@ export class Linter extends baseLinter.BaseLinter { let outputChannel = this.outputChannel; return new Promise((resolve, reject) => { - execPythonFile(commandLine, args, this.workspaceRootPath, true, null, cancellation).then(data => { + execPythonFile(commandLine, args, this.getWorkspaceRootPath(document), true, null, cancellation).then(data => { outputChannel.append('#'.repeat(10) + 'Linting Output - ' + this.Id + '#'.repeat(10) + '\n'); outputChannel.append(data); let outputLines = data.split(/\r?\n/g); diff --git a/src/client/linters/pylama.ts b/src/client/linters/pylama.ts index 9faba1adeb33..718ec652adf4 100644 --- a/src/client/linters/pylama.ts +++ b/src/client/linters/pylama.ts @@ -10,8 +10,8 @@ const REGEX = '(?.py):(?\\d+):(?\\d+): \\[(?\\w+)\\] ( export class Linter extends baseLinter.BaseLinter { _columnOffset = 1; - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('pylama', Product.pylama, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('pylama', Product.pylama, outputChannel); } public isEnabled(): Boolean { @@ -31,7 +31,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise(resolve => { - this.run(pylamaPath, pylamaArgs.concat(['--format=parsable', document.uri.fsPath]), document, this.workspaceRootPath, cancellation, REGEX).then(messages => { + this.run(pylamaPath, pylamaArgs.concat(['--format=parsable', document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation, REGEX).then(messages => { // All messages in pylama are treated as warnings for now messages.forEach(msg => { msg.severity = baseLinter.LintMessageSeverity.Information; diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index 5dfe6aaf0502..d485f7764ba1 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -6,8 +6,8 @@ import { Product, ProductExecutableAndArgs } from '../common/installer'; import { TextDocument, CancellationToken } from 'vscode'; export class Linter extends baseLinter.BaseLinter { - constructor(outputChannel: OutputChannel, workspaceRootPath?: string) { - super('pylint', Product.pylint, outputChannel, workspaceRootPath); + constructor(outputChannel: OutputChannel) { + super('pylint', Product.pylint, outputChannel); } public isEnabled(): Boolean { @@ -27,7 +27,7 @@ export class Linter extends baseLinter.BaseLinter { } return new Promise((resolve, reject) => { - this.run(pylintPath, pylintArgs.concat(['--msg-template=\'{line},{column},{category},{msg_id}:{msg}\'', '--reports=n', '--output-format=text', document.uri.fsPath]), document, this.workspaceRootPath, cancellation).then(messages => { + this.run(pylintPath, pylintArgs.concat(['--msg-template=\'{line},{column},{category},{msg_id}:{msg}\'', '--reports=n', '--output-format=text', document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation).then(messages => { messages.forEach(msg => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.pylintCategorySeverity); }); @@ -36,4 +36,4 @@ export class Linter extends baseLinter.BaseLinter { }, reject); }); } -} \ No newline at end of file +} diff --git a/src/client/providers/lintProvider.ts b/src/client/providers/lintProvider.ts index 830fc96e771f..165055bfccc8 100644 --- a/src/client/providers/lintProvider.ts +++ b/src/client/providers/lintProvider.ts @@ -135,7 +135,9 @@ export class LintProvider extends vscode.Disposable { private onLintDocument(document: vscode.TextDocument): void { // Check if we need to lint this document - const relativeFileName = typeof vscode.workspace.rootPath === 'string' ? path.relative(vscode.workspace.rootPath, document.fileName) : document.fileName; + const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined; + const relativeFileName = typeof workspaceRootPath === 'string' ? path.relative(workspaceRootPath, document.fileName) : document.fileName; if (this.ignoreMinmatches.some(matcher => matcher.match(document.fileName) || matcher.match(relativeFileName))) { return; } @@ -154,7 +156,7 @@ export class LintProvider extends vscode.Disposable { this.pendingLintings.set(document.uri.fsPath, cancelToken); this.outputChannel.clear(); let promises: Promise[] = this.linters.map(linter => { - if (!vscode.workspace.rootPath && !this.settings.linting.enabledWithoutWorkspace) { + if (typeof workspaceRootPath !== 'string' && !this.settings.linting.enabledWithoutWorkspace) { return Promise.resolve([]); } if (!linter.isEnabled()) { diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index 3f52789584cf..45657a478476 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -152,23 +152,23 @@ suite('Linting', () => { } test('Enable and Disable Pylint', () => { let ch = new MockOutputChannel('Lint'); - testEnablingDisablingOfLinter(new pyLint.Linter(ch, pythoFilesPath), 'pylintEnabled'); + testEnablingDisablingOfLinter(new pyLint.Linter(ch), 'pylintEnabled'); }); test('Enable and Disable Pep8', () => { let ch = new MockOutputChannel('Lint'); - testEnablingDisablingOfLinter(new pep8.Linter(ch, pythoFilesPath), 'pep8Enabled'); + testEnablingDisablingOfLinter(new pep8.Linter(ch), 'pep8Enabled'); }); test('Enable and Disable Flake8', () => { let ch = new MockOutputChannel('Lint'); - testEnablingDisablingOfLinter(new flake8.Linter(ch, pythoFilesPath), 'flake8Enabled'); + testEnablingDisablingOfLinter(new flake8.Linter(ch), 'flake8Enabled'); }); test('Enable and Disable Prospector', () => { let ch = new MockOutputChannel('Lint'); - testEnablingDisablingOfLinter(new prospector.Linter(ch, pythoFilesPath), 'prospectorEnabled'); + testEnablingDisablingOfLinter(new prospector.Linter(ch), 'prospectorEnabled'); }); test('Enable and Disable Pydocstyle', () => { let ch = new MockOutputChannel('Lint'); - testEnablingDisablingOfLinter(new pydocstyle.Linter(ch, pythoFilesPath), 'pydocstyleEnabled'); + testEnablingDisablingOfLinter(new pydocstyle.Linter(ch), 'pydocstyleEnabled'); }); function disableAllButThisLinter(linterToEnable: Product) { @@ -215,23 +215,23 @@ suite('Linting', () => { } test('PyLint', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pyLint.Linter(ch, pythoFilesPath); + let linter = new pyLint.Linter(ch); testLinterMessages(linter, ch, fileToLint, pylintMessagesToBeReturned).then(done, done); }); test('Flake8', done => { let ch = new MockOutputChannel('Lint'); - let linter = new flake8.Linter(ch, pythoFilesPath); + let linter = new flake8.Linter(ch); testLinterMessages(linter, ch, fileToLint, flake8MessagesToBeReturned).then(done, done); }); test('Pep8', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pep8.Linter(ch, pythoFilesPath); + let linter = new pep8.Linter(ch); testLinterMessages(linter, ch, fileToLint, pep8MessagesToBeReturned).then(done, done); }); if (!isPython3) { test('Pydocstyle', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pydocstyle.Linter(ch, pythoFilesPath); + let linter = new pydocstyle.Linter(ch); testLinterMessages(linter, ch, fileToLint, pydocstyleMessagseToBeReturned).then(done, done); }); } @@ -242,26 +242,26 @@ suite('Linting', () => { const messagesToBeReturned = value ? filteredPylint3MessagesToBeReturned : filteredPylintMessagesToBeReturned; test('PyLint with config in root', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pyLint.Linter(ch, pylintConfigPath); + let linter = new pyLint.Linter(ch); testLinterMessages(linter, ch, path.join(pylintConfigPath, 'file.py'), messagesToBeReturned).then(done, done); }); }); } test('Flake8 with config in root', done => { let ch = new MockOutputChannel('Lint'); - let linter = new flake8.Linter(ch, flake8ConfigPath); + let linter = new flake8.Linter(ch); testLinterMessages(linter, ch, path.join(flake8ConfigPath, 'file.py'), filteredFlake8MessagesToBeReturned).then(done, done); }); test('Pep8 with config in root', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pep8.Linter(ch, pep8ConfigPath); + let linter = new pep8.Linter(ch); testLinterMessages(linter, ch, path.join(pep8ConfigPath, 'file.py'), filteredPep88MessagesToBeReturned).then(done, done); }); isPython3.then(value => { const messagesToBeReturned = value ? [] : fiteredPydocstyleMessagseToBeReturned; test('Pydocstyle with config in root', done => { let ch = new MockOutputChannel('Lint'); - let linter = new pydocstyle.Linter(ch, pydocstyleConfigPath27); + let linter = new pydocstyle.Linter(ch); testLinterMessages(linter, ch, path.join(pydocstyleConfigPath27, 'file.py'), messagesToBeReturned).then(done, done); }); }); From a549fc67229b149d99bab2aaf7b7bfce020e967a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 6 Oct 2017 08:43:28 -0700 Subject: [PATCH 04/24] multi root support for formatters --- src/client/formatters/autoPep8Formatter.ts | 6 +++--- src/client/formatters/baseFormatter.ts | 11 ++++++++--- src/client/formatters/dummyFormatter.ts | 6 +++--- src/client/formatters/yapfFormatter.ts | 6 +++--- src/client/providers/formatOnSaveProvider.ts | 11 +++++++---- src/test/format/extension.format.test.ts | 17 ++++++++++------- src/test/initialize.ts | 2 +- 7 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/client/formatters/autoPep8Formatter.ts b/src/client/formatters/autoPep8Formatter.ts index 8a81c1dc01be..35b496719278 100644 --- a/src/client/formatters/autoPep8Formatter.ts +++ b/src/client/formatters/autoPep8Formatter.ts @@ -6,8 +6,8 @@ import * as settings from '../common/configSettings'; import { Product } from '../common/installer'; export class AutoPep8Formatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings, workspaceRootPath?: string) { - super('autopep8', Product.autopep8, outputChannel, pythonSettings, workspaceRootPath); + constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { + super('autopep8', Product.autopep8, outputChannel, pythonSettings); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { @@ -19,4 +19,4 @@ export class AutoPep8Formatter extends BaseFormatter { } return super.provideDocumentFormattingEdits(document, options, token, autopep8Path, autoPep8Args); } -} \ No newline at end of file +} diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 032f9bf61619..b60dc9b7dfd1 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -2,15 +2,16 @@ import * as vscode from 'vscode'; import * as fs from 'fs'; -import { execPythonFile } from './../common/utils'; +import * as path from 'path'; import * as settings from './../common/configSettings'; +import { execPythonFile } from './../common/utils'; import { getTextEditsFromPatch, getTempFileWithDocumentContents } from './../common/editor'; import { isNotInstalledError } from '../common/helpers'; import { Installer, Product } from '../common/installer'; export abstract class BaseFormatter { private installer: Installer; - constructor(public Id: string, private product: Product, protected outputChannel: vscode.OutputChannel, protected pythonSettings: settings.IPythonSettings, protected workspaceRootPath?: string) { + constructor(public Id: string, private product: Product, protected outputChannel: vscode.OutputChannel, protected pythonSettings: settings.IPythonSettings) { this.installer = new Installer(); } @@ -18,7 +19,11 @@ export abstract class BaseFormatter { protected provideDocumentFormattingEdits(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, command: string, args: string[], cwd: string = null): Thenable { this.outputChannel.clear(); - cwd = typeof cwd === 'string' && cwd.length > 0 ? cwd : (this.workspaceRootPath ? this.workspaceRootPath : vscode.workspace.rootPath); + if (typeof cwd !== 'string' || cwd.length === 0) { + const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : path.dirname(document.uri.fsPath); + cwd = workspaceRootPath; + } // autopep8 and yapf have the ability to read from the process input stream and return the formatted code out of the output stream // However they don't support returning the diff of the formatted text when reading data from the input stream diff --git a/src/client/formatters/dummyFormatter.ts b/src/client/formatters/dummyFormatter.ts index a008987fde92..252fdd685209 100644 --- a/src/client/formatters/dummyFormatter.ts +++ b/src/client/formatters/dummyFormatter.ts @@ -6,11 +6,11 @@ import * as settings from './../common/configSettings'; import { Product } from '../common/installer'; export class DummyFormatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings, workspaceRootPath?: string) { - super('none', Product.yapf, outputChannel, pythonSettings, workspaceRootPath); + constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { + super('none', Product.yapf, outputChannel, pythonSettings); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { return Promise.resolve([]); } -} \ No newline at end of file +} diff --git a/src/client/formatters/yapfFormatter.ts b/src/client/formatters/yapfFormatter.ts index 7a858bd9f869..f9474ea841f9 100644 --- a/src/client/formatters/yapfFormatter.ts +++ b/src/client/formatters/yapfFormatter.ts @@ -7,8 +7,8 @@ import { Product } from '../common/installer'; import * as path from 'path'; export class YapfFormatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings, workspaceRootPath?: string) { - super('yapf', Product.yapf, outputChannel, pythonSettings, workspaceRootPath); + constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { + super('yapf', Product.yapf, outputChannel, pythonSettings); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { @@ -22,4 +22,4 @@ export class YapfFormatter extends BaseFormatter { let cwd = path.dirname(document.fileName); return super.provideDocumentFormattingEdits(document, options, token, yapfPath, yapfArgs, cwd); } -} \ No newline at end of file +} diff --git a/src/client/providers/formatOnSaveProvider.ts b/src/client/providers/formatOnSaveProvider.ts index 4e972ff38c9b..b3a2ec3753bf 100644 --- a/src/client/providers/formatOnSaveProvider.ts +++ b/src/client/providers/formatOnSaveProvider.ts @@ -6,17 +6,20 @@ import * as vscode from "vscode"; import { BaseFormatter } from "./../formatters/baseFormatter"; import { YapfFormatter } from "./../formatters/yapfFormatter"; import { AutoPep8Formatter } from "./../formatters/autoPep8Formatter"; +import { DummyFormatter } from "./../formatters/dummyFormatter"; import * as settings from "./../common/configSettings"; -export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilter, settings: settings.IPythonSettings, outputChannel: vscode.OutputChannel, workspaceRootPath?: string): vscode.Disposable { +export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilter, settings: settings.IPythonSettings, outputChannel: vscode.OutputChannel): vscode.Disposable { let formatters = new Map(); let pythonSettings = settings; - let yapfFormatter = new YapfFormatter(outputChannel, settings, workspaceRootPath); - let autoPep8 = new AutoPep8Formatter(outputChannel, settings, workspaceRootPath); + let yapfFormatter = new YapfFormatter(outputChannel, settings); + let autoPep8 = new AutoPep8Formatter(outputChannel, settings); + const dummyFormatter = new DummyFormatter(outputChannel, settings); formatters.set(yapfFormatter.Id, yapfFormatter); formatters.set(autoPep8.Id, autoPep8); + formatters.set(dummyFormatter.Id, dummyFormatter); return vscode.workspace.onWillSaveTextDocument(e => { const document = e.document; @@ -31,4 +34,4 @@ export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilt e.waitUntil(formatter.formatDocument(document, null, null)); } }, null, null); -} \ No newline at end of file +} diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index 5cfa45dabd86..e764976bcf80 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -12,6 +12,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; import * as settings from '../../client/common/configSettings'; import * as fs from 'fs-extra'; +import { EOL } from 'os'; import { AutoPep8Formatter } from '../../client/formatters/autoPep8Formatter'; import { initialize, IS_TRAVIS, closeActiveWindows } from '../initialize'; import { YapfFormatter } from '../../client/formatters/yapfFormatter'; @@ -38,8 +39,9 @@ suite('Formatting', () => { fs.copySync(originalUnformattedFile, file, { overwrite: true }); }); fs.ensureDirSync(path.dirname(autoPep8FileToFormat)); - const yapf = execPythonFile('yapf', [originalUnformattedFile], pythoFilesPath, false); - const autoPep8 = execPythonFile('autopep8', [originalUnformattedFile], pythoFilesPath, false); + const workspaceRoot = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(originalUnformattedFile)).uri.fsPath; + const yapf = execPythonFile('yapf', [originalUnformattedFile], workspaceRoot, false); + const autoPep8 = execPythonFile('autopep8', [originalUnformattedFile], workspaceRoot, false); await Promise.all([yapf, autoPep8]).then(formattedResults => { formattedYapf = formattedResults[0]; formattedAutoPep8 = formattedResults[1]; @@ -76,11 +78,11 @@ suite('Formatting', () => { }); } test('AutoPep8', done => { - testFormatting(new AutoPep8Formatter(ch, pythonSettings, pythoFilesPath), formattedAutoPep8, autoPep8FileToFormat).then(done, done); + testFormatting(new AutoPep8Formatter(ch, pythonSettings), formattedAutoPep8, autoPep8FileToFormat).then(done, done); }); test('Yapf', done => { - testFormatting(new YapfFormatter(ch, pythonSettings, pythoFilesPath), formattedYapf, yapfFileToFormat).then(done, done); + testFormatting(new YapfFormatter(ch, pythonSettings), formattedYapf, yapfFileToFormat).then(done, done); }); function testAutoFormatting(formatter: string, formattedContents: string, fileToFormat: string): PromiseLike { @@ -104,17 +106,18 @@ suite('Formatting', () => { }, 5000); }); }).then(() => { - assert.equal(textDocument.getText(), formattedContents, 'Formatted contents are not the same'); + const text = textDocument.getText(); + assert.equal(text === formattedContents, true, 'Formatted contents are not the same'); }); } test('AutoPep8 autoformat on save', done => { - testAutoFormatting('autopep8', '#\n' + formattedAutoPep8, autoPep8FileToAutoFormat).then(done, done); + testAutoFormatting('autopep8', `#${EOL}` + formattedAutoPep8, autoPep8FileToAutoFormat).then(done, done); }); // For some reason doesn't ever work on travis if (!IS_TRAVIS) { test('Yapf autoformat on save', done => { - testAutoFormatting('yapf', '#\n' + formattedYapf, yapfFileToAutoFormat).then(done, done); + testAutoFormatting('yapf', `#${EOL}` + formattedYapf, yapfFileToAutoFormat).then(done, done); }); } }); diff --git a/src/test/initialize.ts b/src/test/initialize.ts index a298c234c4a9..1dcd6e247fd4 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -71,7 +71,7 @@ function getPythonPath(): string { const pythonPaths = ['/home/travis/virtualenv/python3.5.2/bin/python', '/xUsers/travis/.pyenv/versions/3.5.1/envs/MYVERSION/bin/python', '/xUsers/donjayamanne/Projects/PythonEnvs/p361/bin/python', - 'C:/Users/dojayama/nine/python.exe', + 'cC:/Users/dojayama/nine/python.exe', 'C:/Development/PythonEnvs/p27/scripts/python.exe', '/Users/donjayamanne/Projects/PythonEnvs/p27/bin/python']; for (let counter = 0; counter < pythonPaths.length; counter++) { From 657e20f4a12b0ec00b77796c5381a5402ee41d3b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 6 Oct 2017 08:57:38 -0700 Subject: [PATCH 05/24] determine workspace root path --- src/test/format/extension.format.test.ts | 6 +++--- src/test/index.ts | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index e764976bcf80..530cbd0b274f 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -22,6 +22,7 @@ const pythonSettings = settings.PythonSettings.getInstance(); const ch = vscode.window.createOutputChannel('Tests'); const pythoFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting'); +const workspaceRootPath = path.join(__dirname, '..', '..', '..', 'src', 'test'); const originalUnformattedFile = path.join(pythoFilesPath, 'fileToFormat.py'); const autoPep8FileToFormat = path.join(pythoFilesPath, 'autoPep8FileToFormat.py'); @@ -39,9 +40,8 @@ suite('Formatting', () => { fs.copySync(originalUnformattedFile, file, { overwrite: true }); }); fs.ensureDirSync(path.dirname(autoPep8FileToFormat)); - const workspaceRoot = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(originalUnformattedFile)).uri.fsPath; - const yapf = execPythonFile('yapf', [originalUnformattedFile], workspaceRoot, false); - const autoPep8 = execPythonFile('autopep8', [originalUnformattedFile], workspaceRoot, false); + const yapf = execPythonFile('yapf', [originalUnformattedFile], workspaceRootPath, false); + const autoPep8 = execPythonFile('autopep8', [originalUnformattedFile], workspaceRootPath, false); await Promise.all([yapf, autoPep8]).then(formattedResults => { formattedYapf = formattedResults[0]; formattedAutoPep8 = formattedResults[1]; diff --git a/src/test/index.ts b/src/test/index.ts index af916a981db9..064f575abb67 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -18,7 +18,8 @@ let testRunner = require('vscode/lib/testrunner'); testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000 + timeout: 25000, + grep:'Format' }); initializePython(); From d492b5ab5bacf4bfd17fe42af8a3a09594cd13c8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 6 Oct 2017 15:04:03 -0700 Subject: [PATCH 06/24] revert change --- src/test/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index 064f575abb67..af916a981db9 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -18,8 +18,7 @@ let testRunner = require('vscode/lib/testrunner'); testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000, - grep:'Format' + timeout: 25000 }); initializePython(); From c0c70fbdeb263b5e50dc4674ca297713f262f83c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 6 Oct 2017 15:20:43 -0700 Subject: [PATCH 07/24] support multiple configs per workspace folder --- src/client/common/configSettings.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 2897b50eee1b..de0bbf614dd1 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -3,6 +3,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; import * as child_process from 'child_process'; +import { Uri } from 'vscode'; import { SystemVariables } from './systemVariables'; import { EventEmitter } from 'events'; const untildify = require('untildify'); @@ -133,26 +134,32 @@ export interface JupyterSettings { const IS_TEST_EXECUTION = process.env['PYTHON_DONJAYAMANNE_TEST'] === '1'; export class PythonSettings extends EventEmitter implements IPythonSettings { - private static pythonSettings: PythonSettings = new PythonSettings(); + private static pythonSettings: Map = new Map(); + private workspaceRoot?: vscode.Uri; private disposables: vscode.Disposable[] = []; - constructor() { + constructor(workspaceFolder?: Uri) { super(); - if (PythonSettings.pythonSettings) { - throw new Error('Singleton class, Use getInstance method'); - } + this.workspaceRoot = workspaceFolder ? workspaceFolder : vscode.Uri.file(__dirname); this.disposables.push(vscode.workspace.onDidChangeConfiguration(() => { this.initializeSettings(); })); this.initializeSettings(); } - public static getInstance(): PythonSettings { - return PythonSettings.pythonSettings; + public static getInstance(resource?: Uri): PythonSettings { + const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined; + const workspaceFolderUri = workspaceFolder ? workspaceFolder.uri : undefined; + const workspaceFolderKey = workspaceFolderUri ? workspaceFolderUri.fsPath : ''; + if (!PythonSettings.pythonSettings.has(workspaceFolderKey)) { + const settings = new PythonSettings(workspaceFolderUri); + PythonSettings.pythonSettings.set(workspaceFolderKey, settings); + } + return PythonSettings.pythonSettings.get(workspaceFolderKey); } private initializeSettings() { const systemVariables: SystemVariables = new SystemVariables(); - const workspaceRoot = (IS_TEST_EXECUTION || typeof vscode.workspace.rootPath !== 'string') ? __dirname : vscode.workspace.rootPath; - let pythonSettings = vscode.workspace.getConfiguration('python'); + const workspaceRoot = (IS_TEST_EXECUTION || !this.workspaceRoot) ? __dirname : this.workspaceRoot.fsPath; + const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot); this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; this.pythonPath = getAbsolutePath(this.pythonPath, IS_TEST_EXECUTION ? __dirname : workspaceRoot); this.venvPath = systemVariables.resolveAny(pythonSettings.get('venvPath'))!; From 86e36c1337bbe952493323b54245c23402c9a7d8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 6 Oct 2017 15:52:11 -0700 Subject: [PATCH 08/24] modify formatters to use resource specific settings --- src/client/common/configSettings.ts | 5 +++- src/client/extension.ts | 4 ++-- src/client/formatters/autoPep8Formatter.ts | 11 +++++---- src/client/formatters/baseFormatter.ts | 25 ++++++++++++++++---- src/client/formatters/dummyFormatter.ts | 5 ++-- src/client/formatters/yapfFormatter.ts | 16 +++++++------ src/client/providers/formatOnSaveProvider.ts | 23 +++++++++--------- src/client/providers/formatProvider.ts | 13 +++++----- src/test/format/extension.format.test.ts | 4 ++-- 9 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index de0bbf614dd1..32ef9ec060fb 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -148,7 +148,10 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { } public static getInstance(resource?: Uri): PythonSettings { const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined; - const workspaceFolderUri = workspaceFolder ? workspaceFolder.uri : undefined; + let workspaceFolderUri: Uri = workspaceFolder ? workspaceFolder.uri : undefined; + if (!workspaceFolderUri && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolderUri = vscode.workspace.workspaceFolders[0].uri; + } const workspaceFolderKey = workspaceFolderUri ? workspaceFolderUri.fsPath : ''; if (!PythonSettings.pythonSettings.has(workspaceFolderKey)) { const settings = new PythonSettings(workspaceFolderUri); diff --git a/src/client/extension.ts b/src/client/extension.ts index 4a468327b959..90f5c7d2e2ce 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -67,7 +67,7 @@ export async function activate(context: vscode.ExtensionContext) { context.subscriptions.push(...activateExecInTerminalProvider()); context.subscriptions.push(activateUpdateSparkLibraryProvider()); activateSimplePythonRefactorProvider(context, formatOutChannel); - context.subscriptions.push(activateFormatOnSaveProvider(PYTHON, settings.PythonSettings.getInstance(), formatOutChannel)); + context.subscriptions.push(activateFormatOnSaveProvider(PYTHON, formatOutChannel)); context.subscriptions.push(activateGoToObjectDefinitionProvider(context)); context.subscriptions.push(vscode.commands.registerCommand(Commands.Start_REPL, () => { @@ -114,7 +114,7 @@ export async function activate(context: vscode.ExtensionContext) { context.subscriptions.push(vscode.languages.registerSignatureHelpProvider(PYTHON, new PythonSignatureProvider(context, jediProx), '(', ',')); } if (pythonSettings.formatting.provider !== 'none') { - const formatProvider = new PythonFormattingEditProvider(context, formatOutChannel, pythonSettings); + const formatProvider = new PythonFormattingEditProvider(context, formatOutChannel); context.subscriptions.push(vscode.languages.registerDocumentFormattingEditProvider(PYTHON, formatProvider)); context.subscriptions.push(vscode.languages.registerDocumentRangeFormattingEditProvider(PYTHON, formatProvider)); } diff --git a/src/client/formatters/autoPep8Formatter.ts b/src/client/formatters/autoPep8Formatter.ts index 35b496719278..35afef4a57cf 100644 --- a/src/client/formatters/autoPep8Formatter.ts +++ b/src/client/formatters/autoPep8Formatter.ts @@ -2,17 +2,18 @@ import * as vscode from 'vscode'; import { BaseFormatter } from './baseFormatter'; -import * as settings from '../common/configSettings'; +import { PythonSettings } from '../common/configSettings'; import { Product } from '../common/installer'; export class AutoPep8Formatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { - super('autopep8', Product.autopep8, outputChannel, pythonSettings); + constructor(outputChannel: vscode.OutputChannel) { + super('autopep8', Product.autopep8, outputChannel); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { - let autopep8Path = this.pythonSettings.formatting.autopep8Path; - let autoPep8Args = Array.isArray(this.pythonSettings.formatting.autopep8Args) ? this.pythonSettings.formatting.autopep8Args : []; + const settings = PythonSettings.getInstance(document.uri); + const autopep8Path = settings.formatting.autopep8Path; + let autoPep8Args = Array.isArray(settings.formatting.autopep8Args) ? settings.formatting.autopep8Args : []; autoPep8Args = autoPep8Args.concat(['--diff']); if (range && !range.isEmpty) { autoPep8Args = autoPep8Args.concat(['--line-range', (range.start.line + 1).toString(), (range.end.line + 1).toString()]); diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index b60dc9b7dfd1..3fcce0d76661 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -4,25 +4,40 @@ import * as vscode from 'vscode'; import * as fs from 'fs'; import * as path from 'path'; import * as settings from './../common/configSettings'; +import { Uri } from 'vscode'; import { execPythonFile } from './../common/utils'; import { getTextEditsFromPatch, getTempFileWithDocumentContents } from './../common/editor'; import { isNotInstalledError } from '../common/helpers'; import { Installer, Product } from '../common/installer'; + export abstract class BaseFormatter { private installer: Installer; - constructor(public Id: string, private product: Product, protected outputChannel: vscode.OutputChannel, protected pythonSettings: settings.IPythonSettings) { + constructor(public Id: string, private product: Product, protected outputChannel: vscode.OutputChannel) { this.installer = new Installer(); } public abstract formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable; - + protected getDocumentPath(document: vscode.TextDocument, fallbackPath: string) { + if (path.basename(document.uri.fsPath) === document.uri.fsPath) { + return fallbackPath; + } + return path.dirname(document.fileName); + } + protected getWorkspaceUri(document: vscode.TextDocument) { + const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + if (workspaceFolder) { + return workspaceFolder.uri; + } + if (Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + return vscode.workspace.workspaceFolders[0].uri; + } + return vscode.Uri.file(__dirname); + } protected provideDocumentFormattingEdits(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, command: string, args: string[], cwd: string = null): Thenable { this.outputChannel.clear(); if (typeof cwd !== 'string' || cwd.length === 0) { - const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); - const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : path.dirname(document.uri.fsPath); - cwd = workspaceRootPath; + cwd = this.getWorkspaceUri(document).fsPath; } // autopep8 and yapf have the ability to read from the process input stream and return the formatted code out of the output stream diff --git a/src/client/formatters/dummyFormatter.ts b/src/client/formatters/dummyFormatter.ts index 252fdd685209..481b57c69ae6 100644 --- a/src/client/formatters/dummyFormatter.ts +++ b/src/client/formatters/dummyFormatter.ts @@ -2,12 +2,11 @@ import * as vscode from 'vscode'; import { BaseFormatter } from './baseFormatter'; -import * as settings from './../common/configSettings'; import { Product } from '../common/installer'; export class DummyFormatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { - super('none', Product.yapf, outputChannel, pythonSettings); + constructor(outputChannel: vscode.OutputChannel) { + super('none', Product.yapf, outputChannel); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { diff --git a/src/client/formatters/yapfFormatter.ts b/src/client/formatters/yapfFormatter.ts index f9474ea841f9..ff8e40e21066 100644 --- a/src/client/formatters/yapfFormatter.ts +++ b/src/client/formatters/yapfFormatter.ts @@ -1,25 +1,27 @@ 'use strict'; import * as vscode from 'vscode'; +import * as path from 'path'; import { BaseFormatter } from './baseFormatter'; -import * as settings from './../common/configSettings'; +import { PythonSettings } from '../common/configSettings'; import { Product } from '../common/installer'; -import * as path from 'path'; export class YapfFormatter extends BaseFormatter { - constructor(outputChannel: vscode.OutputChannel, pythonSettings: settings.IPythonSettings) { - super('yapf', Product.yapf, outputChannel, pythonSettings); + constructor(outputChannel: vscode.OutputChannel) { + super('yapf', Product.yapf, outputChannel); } public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { - let yapfPath = this.pythonSettings.formatting.yapfPath; - let yapfArgs = Array.isArray(this.pythonSettings.formatting.yapfArgs) ? this.pythonSettings.formatting.yapfArgs : []; + const settings = PythonSettings.getInstance(document.uri); + const yapfPath = settings.formatting.yapfPath; + let yapfArgs = Array.isArray(settings.formatting.yapfArgs) ? settings.formatting.yapfArgs : []; yapfArgs = yapfArgs.concat(['--diff']); if (range && !range.isEmpty) { yapfArgs = yapfArgs.concat(['--lines', `${range.start.line + 1}-${range.end.line + 1}`]); } // Yapf starts looking for config file starting from the file path - let cwd = path.dirname(document.fileName); + const fallbarFolder = this.getWorkspaceUri(document).fsPath; + const cwd = this.getDocumentPath(document, fallbarFolder); return super.provideDocumentFormattingEdits(document, options, token, yapfPath, yapfArgs, cwd); } } diff --git a/src/client/providers/formatOnSaveProvider.ts b/src/client/providers/formatOnSaveProvider.ts index b3a2ec3753bf..1b550f0d6cc4 100644 --- a/src/client/providers/formatOnSaveProvider.ts +++ b/src/client/providers/formatOnSaveProvider.ts @@ -7,15 +7,13 @@ import { BaseFormatter } from "./../formatters/baseFormatter"; import { YapfFormatter } from "./../formatters/yapfFormatter"; import { AutoPep8Formatter } from "./../formatters/autoPep8Formatter"; import { DummyFormatter } from "./../formatters/dummyFormatter"; -import * as settings from "./../common/configSettings"; +import { PythonSettings } from "./../common/configSettings"; -export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilter, settings: settings.IPythonSettings, outputChannel: vscode.OutputChannel): vscode.Disposable { - let formatters = new Map(); - let pythonSettings = settings; - - let yapfFormatter = new YapfFormatter(outputChannel, settings); - let autoPep8 = new AutoPep8Formatter(outputChannel, settings); - const dummyFormatter = new DummyFormatter(outputChannel, settings); +export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilter, outputChannel: vscode.OutputChannel): vscode.Disposable { + const formatters = new Map(); + const yapfFormatter = new YapfFormatter(outputChannel); + const autoPep8 = new AutoPep8Formatter(outputChannel); + const dummyFormatter = new DummyFormatter(outputChannel); formatters.set(yapfFormatter.Id, yapfFormatter); formatters.set(autoPep8.Id, autoPep8); @@ -26,11 +24,12 @@ export function activateFormatOnSaveProvider(languageFilter: vscode.DocumentFilt if (document.languageId !== languageFilter.language) { return; } - let textEditor = vscode.window.activeTextEditor; - let editorConfig = vscode.workspace.getConfiguration('editor'); + const textEditor = vscode.window.activeTextEditor; + const editorConfig = vscode.workspace.getConfiguration('editor'); const globalEditorFormatOnSave = editorConfig && editorConfig.has('formatOnSave') && editorConfig.get('formatOnSave') === true; - if ((pythonSettings.formatting.formatOnSave || globalEditorFormatOnSave) && textEditor.document === document) { - let formatter = formatters.get(pythonSettings.formatting.provider); + const settings = PythonSettings.getInstance(document.uri); + if ((settings.formatting.formatOnSave || globalEditorFormatOnSave) && textEditor.document === document) { + const formatter = formatters.get(settings.formatting.provider); e.waitUntil(formatter.formatDocument(document, null, null)); } }, null, null); diff --git a/src/client/providers/formatProvider.ts b/src/client/providers/formatProvider.ts index 5df3ab9e2cca..58844bca269c 100644 --- a/src/client/providers/formatProvider.ts +++ b/src/client/providers/formatProvider.ts @@ -5,15 +5,15 @@ import { BaseFormatter } from './../formatters/baseFormatter'; import { YapfFormatter } from './../formatters/yapfFormatter'; import { AutoPep8Formatter } from './../formatters/autoPep8Formatter'; import { DummyFormatter } from './../formatters/dummyFormatter'; -import * as settings from './../common/configSettings'; +import { PythonSettings } from './../common/configSettings'; export class PythonFormattingEditProvider implements vscode.DocumentFormattingEditProvider, vscode.DocumentRangeFormattingEditProvider { private formatters = new Map(); - public constructor(context: vscode.ExtensionContext, outputChannel: vscode.OutputChannel, private settings: settings.IPythonSettings) { - let yapfFormatter = new YapfFormatter(outputChannel, settings); - let autoPep8 = new AutoPep8Formatter(outputChannel, settings); - let dummy = new DummyFormatter(outputChannel, settings); + public constructor(context: vscode.ExtensionContext, outputChannel: vscode.OutputChannel) { + const yapfFormatter = new YapfFormatter(outputChannel); + const autoPep8 = new AutoPep8Formatter(outputChannel); + const dummy = new DummyFormatter(outputChannel); this.formatters.set(yapfFormatter.Id, yapfFormatter); this.formatters.set(autoPep8.Id, autoPep8); this.formatters.set(dummy.Id, dummy); @@ -24,7 +24,8 @@ export class PythonFormattingEditProvider implements vscode.DocumentFormattingEd } public provideDocumentRangeFormattingEdits(document: vscode.TextDocument, range: vscode.Range, options: vscode.FormattingOptions, token: vscode.CancellationToken): Thenable { - let formatter = this.formatters.get(this.settings.formatting.provider); + const settings = PythonSettings.getInstance(document.uri); + const formatter = this.formatters.get(settings.formatting.provider); return formatter.formatDocument(document, options, token, range); } diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index 530cbd0b274f..986d64e976a9 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -78,11 +78,11 @@ suite('Formatting', () => { }); } test('AutoPep8', done => { - testFormatting(new AutoPep8Formatter(ch, pythonSettings), formattedAutoPep8, autoPep8FileToFormat).then(done, done); + testFormatting(new AutoPep8Formatter(ch), formattedAutoPep8, autoPep8FileToFormat).then(done, done); }); test('Yapf', done => { - testFormatting(new YapfFormatter(ch, pythonSettings), formattedYapf, yapfFileToFormat).then(done, done); + testFormatting(new YapfFormatter(ch), formattedYapf, yapfFileToFormat).then(done, done); }); function testAutoFormatting(formatter: string, formattedContents: string, fileToFormat: string): PromiseLike { From c5cd8a9fc53013a0c79d7cccbefcae8ebba4747f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 13:18:53 -0700 Subject: [PATCH 09/24] modified installer to pass resource for workspace resolution --- src/client/common/installer.ts | 168 +++++++++++------- src/client/formatters/baseFormatter.ts | 6 +- src/client/linters/baseLinter.ts | 8 +- .../linters/errorHandlers/invalidArgs.ts | 6 +- src/client/linters/errorHandlers/main.ts | 4 +- .../linters/errorHandlers/notInstalled.ts | 5 +- src/client/linters/errorHandlers/standard.ts | 15 +- src/client/linters/prospector.ts | 6 +- src/client/linters/pydocstyle.ts | 2 +- src/client/providers/renameProvider.ts | 2 +- .../providers/simpleRefactorProvider.ts | 2 +- .../unittests/common/baseTestManager.ts | 2 +- src/client/workspaceSymbols/main.ts | 33 ++-- src/test/autocomplete/base.test.ts | 2 +- 14 files changed, 144 insertions(+), 117 deletions(-) diff --git a/src/client/common/installer.ts b/src/client/common/installer.ts index 1b3991047944..6d7af57228a5 100644 --- a/src/client/common/installer.ts +++ b/src/client/common/installer.ts @@ -1,9 +1,9 @@ import * as vscode from 'vscode'; import * as settings from './configSettings'; import * as os from 'os'; +import { commands, ConfigurationTarget, Disposable, OutputChannel, Terminal, Uri, window, workspace } from 'vscode'; import { isNotInstalledError } from './helpers'; import { execPythonFile, getFullyQualifiedPythonInterpreterPath } from './utils'; -import { Documentation } from './constants'; export enum Product { pytest, @@ -111,6 +111,9 @@ SettingToDisableProduct.set(Product.pydocstyle, 'linting.pydocstyleEnabled'); SettingToDisableProduct.set(Product.pylint, 'linting.pylintEnabled'); SettingToDisableProduct.set(Product.pytest, 'unitTest.pyTestEnabled'); +const ProductInstallationPrompt = new Map(); +ProductInstallationPrompt.set(Product.ctags, 'Install CTags to enable Python workspace symbols'); + enum ProductType { Linter, Formatter, @@ -142,6 +145,11 @@ ProductTypes.set(Product.autopep8, ProductType.Formatter); ProductTypes.set(Product.yapf, ProductType.Formatter); ProductTypes.set(Product.rope, ProductType.RefactoringLibrary); +export enum InstallerResponse { + Installed, + Disabled, + Ignore +} export class Installer implements vscode.Disposable { private static terminal: vscode.Terminal | undefined | null; private disposables: vscode.Disposable[] = []; @@ -155,12 +163,14 @@ export class Installer implements vscode.Disposable { public dispose() { this.disposables.forEach(d => d.dispose()); } - public shouldDisplayPrompt(product: Product) { + private shouldDisplayPrompt(product: Product) { const productName = ProductNames.get(product)!; - return settings.PythonSettings.getInstance().disablePromptForFeatures.indexOf(productName) === -1; + const pythonConfig = workspace.getConfiguration('python'); + const disablePromptForFeatures = pythonConfig.get('disablePromptForFeatures', [] as string[]); + return disablePromptForFeatures.indexOf(productName) === -1; } - async promptToInstall(product: Product) { + public async promptToInstall(product: Product, resource?: Uri): Promise { const productType = ProductTypes.get(product)!; const productTypeName = ProductTypeNames.get(productType); const productName = ProductNames.get(product)!; @@ -173,10 +183,10 @@ export class Installer implements vscode.Disposable { else { console.warn(message); } - return; + return InstallerResponse.Ignore; } - const installOption = 'Install ' + productName; + const installOption = ProductInstallationPrompt.has(product) ? ProductInstallationPrompt.get(product) : 'Install ' + productName; const disableOption = 'Disable ' + productTypeName; const dontShowAgain = `Don't show this prompt again`; const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8'; @@ -189,46 +199,50 @@ export class Installer implements vscode.Disposable { if (SettingToDisableProduct.has(product)) { options.push(...[disableOption, dontShowAgain]); } - return vscode.window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options).then(item => { - switch (item) { - case installOption: { - return this.install(product); - } - case disableOption: { - if (Linters.indexOf(product) >= 0) { - return disableLinter(product); - } - else { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const settingToDisable = SettingToDisableProduct.get(product)!; - return pythonConfig.update(settingToDisable, false); - } - } - case useOtherFormatter: { - const pythonConfig = vscode.workspace.getConfiguration('python'); - return pythonConfig.update('formatting.provider', alternateFormatter); - } - case dontShowAgain: { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const features = pythonConfig.get('disablePromptForFeatures', [] as string[]); - features.push(productName); - return pythonConfig.update('disablePromptForFeatures', features, true); + const item = await window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options); + switch (item) { + case installOption: { + return this.install(product, resource); + } + case disableOption: { + if (Linters.indexOf(product) >= 0) { + return this.disableLinter(product, resource).then(() => InstallerResponse.Disabled); } - case 'Help': { - return Promise.resolve(); + else { + const settingToDisable = SettingToDisableProduct.get(product)!; + return this.updateSetting(settingToDisable, false, resource).then(() => InstallerResponse.Disabled); } } - }); + case useOtherFormatter: { + return this.updateSetting('formatting.provider', alternateFormatter, resource) + .then(() => InstallerResponse.Installed); + } + case dontShowAgain: { + const pythonConfig = workspace.getConfiguration('python'); + const features = pythonConfig.get('disablePromptForFeatures', [] as string[]); + features.push(productName); + return pythonConfig.update('disablePromptForFeatures', features, true).then(() => InstallerResponse.Ignore); + } + } } - - install(product: Product): Promise { + public async install(product: Product, resource?: Uri): Promise { if (!this.outputChannel && !Installer.terminal) { - Installer.terminal = vscode.window.createTerminal('Python Installer'); + Installer.terminal = window.createTerminal('Python Installer'); } - if (product === Product.ctags && os.platform() === 'win32') { - vscode.commands.executeCommand('python.displayHelp', Documentation.Workspace.InstallOnWindows); - return Promise.resolve(); + if (product === Product.ctags && settings.IS_WINDOWS) { + if (this.outputChannel) { + this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols'); + this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.'); + this.outputChannel.appendLine('Option 1: Extract ctags.exe from the downloaded zip to any folder within your PATH so that Visual Studio Code can run it.'); + this.outputChannel.appendLine('Option 2: Extract to any folder and add the path to this folder to the command setting.'); + this.outputChannel.appendLine('Option 3: Extract to any folder and define that path in the python.workspaceSymbols.ctagsPath setting of your user settings file (settings.json).'); + this.outputChannel.show(); + } + else { + window.showInformationMessage('Install Universal Ctags and set it in your path or define the path in your python.workspaceSymbols.ctagsPath settings'); + } + return InstallerResponse.Ignore; } let installArgs = ProductInstallScripts.get(product)!; @@ -241,19 +255,19 @@ export class Installer implements vscode.Disposable { installArgs.splice(2, 0, '--proxy'); } } + let installationPromise: Promise; if (this.outputChannel && installArgs[0] === '-m') { // Errors are just displayed to the user this.outputChannel.show(); - return execPythonFile(settings.PythonSettings.getInstance().pythonPath, installArgs, vscode.workspace.rootPath!, true, (data) => { - this.outputChannel!.append(data); - }); + installationPromise = execPythonFile(settings.PythonSettings.getInstance(resource).pythonPath, + installArgs, getCwdForInstallScript(resource), true, (data) => { this.outputChannel!.append(data); }); } else { // When using terminal get the fully qualitified path // Cuz people may launch vs code from terminal when they have activated the appropriate virtual env // Problem is terminal doesn't use the currently activated virtual env // Must have something to do with the process being launched in the terminal - return getFullyQualifiedPythonInterpreterPath() + installationPromise = getFullyQualifiedPythonInterpreterPath() .then(pythonPath => { let installScript = installArgs.join(' '); @@ -269,42 +283,68 @@ export class Installer implements vscode.Disposable { Installer.terminal!.show(false); }); } + + return installationPromise + .then(() => this.isInstalled(product)) + .then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore); } - isInstalled(product: Product): Promise { - return isProductInstalled(product); + public isInstalled(product: Product, resource?: Uri): Promise { + return isProductInstalled(product, resource); } - uninstall(product: Product): Promise { - return uninstallproduct(product); + public uninstall(product: Product, resource?: Uri): Promise { + return uninstallproduct(product, resource); + } + public disableLinter(product: Product, resource: Uri) { + if (resource && !workspace.getWorkspaceFolder(resource)) { + const settingToDisable = SettingToDisableProduct.get(product)!; + const pythonConfig = workspace.getConfiguration('python', resource); + return pythonConfig.update(settingToDisable, false, ConfigurationTarget.Workspace); + } + else { + const pythonConfig = workspace.getConfiguration('python'); + return pythonConfig.update('linting.enabledWithoutWorkspace', false, true); + } + } + private updateSetting(setting: string, value: any, resource?: Uri) { + if (resource && !workspace.getWorkspaceFolder(resource)) { + const pythonConfig = workspace.getConfiguration('python', resource); + return pythonConfig.update(setting, value, ConfigurationTarget.Workspace); + } + else { + const pythonConfig = workspace.getConfiguration('python'); + return pythonConfig.update(setting, value, true); + } } } -export function disableLinter(product: Product, global?: boolean) { - const pythonConfig = vscode.workspace.getConfiguration('python'); - const settingToDisable = SettingToDisableProduct.get(product)!; - if (vscode.workspace.rootPath) { - return pythonConfig.update(settingToDisable, false, global); +function getCwdForInstallScript(resource?: Uri) { + const workspaceFolder = workspace.getWorkspaceFolder(resource); + if (workspaceFolder) { + return workspaceFolder.uri.fsPath; } - else { - return pythonConfig.update('linting.enabledWithoutWorkspace', false, true); + if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) { + return workspace.workspaceFolders[0].uri.fsPath; } + return __dirname; } -async function isProductInstalled(product: Product): Promise { +async function isProductInstalled(product: Product, resource?: Uri): Promise { if (!ProductExecutableAndArgs.has(product)) { return; } const prodExec = ProductExecutableAndArgs.get(product)!; - return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), vscode.workspace.rootPath!, false) - .then(() => { - return true; - }).catch(reason => { - return !isNotInstalledError(reason); - }); + const cwd = getCwdForInstallScript(resource); + return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), cwd, false) + .then(() => true) + .catch(reason => !isNotInstalledError(reason)); } -function uninstallproduct(product: Product): Promise { +function uninstallproduct(product: Product, resource?: Uri): Promise { + if (!ProductUninstallScripts.has(product)) { + return Promise.resolve(); + } const uninstallArgs = ProductUninstallScripts.get(product)!; - return execPythonFile('python', uninstallArgs, vscode.workspace.rootPath!, false); -} + return execPythonFile('python', uninstallArgs, getCwdForInstallScript(resource), false); +} \ No newline at end of file diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 3fcce0d76661..0beaa02e9291 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -61,14 +61,14 @@ export abstract class BaseFormatter { } return getTextEditsFromPatch(document.getText(), data[1]); }).catch(error => { - this.handleError(this.Id, command, error); + this.handleError(this.Id, command, error, document.uri); return []; }); vscode.window.setStatusBarMessage(`Formatting with ${this.Id}`, promise); return promise; } - protected handleError(expectedFileName: string, fileName: string, error: Error) { + protected handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { let customError = `Formatting with ${this.Id} failed.`; if (isNotInstalledError(error)) { @@ -84,7 +84,7 @@ export abstract class BaseFormatter { } else { customError += `\nYou could either install the '${this.Id}' formatter, turn it off or use another formatter.`; - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, resource); } } diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index b4cd6ca154ed..edb02ed2eb79 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -1,7 +1,7 @@ 'use strict'; import { execPythonFile } from './../common/utils'; import * as settings from './../common/configSettings'; -import { OutputChannel } from 'vscode'; +import { OutputChannel, Uri } from 'vscode'; import { Installer, Product } from '../common/installer'; import * as vscode from 'vscode'; import { ErrorHandler } from './errorHandlers/main'; @@ -135,12 +135,12 @@ export abstract class BaseLinter { let outputLines = data.split(/\r?\n/g); return this.parseLines(outputLines, regEx); }).catch(error => { - this.handleError(this.Id, command, error); + this.handleError(this.Id, command, error, document.uri); return []; }); } - protected handleError(expectedFileName: string, fileName: string, error: Error) { - this._errorHandler.handleError(expectedFileName, fileName, error); + protected handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { + this._errorHandler.handleError(expectedFileName, fileName, error, resource); } } diff --git a/src/client/linters/errorHandlers/invalidArgs.ts b/src/client/linters/errorHandlers/invalidArgs.ts index 514b2b82e523..b2d4b155cadb 100644 --- a/src/client/linters/errorHandlers/invalidArgs.ts +++ b/src/client/linters/errorHandlers/invalidArgs.ts @@ -1,6 +1,6 @@ 'use strict'; import { isNotInstalledError } from '../../common/helpers'; -import * as vscode from 'vscode'; +import { Uri, window } from 'vscode'; import { StandardErrorHandler } from './standard'; export class InvalidArgumentsErrorHandler extends StandardErrorHandler { @@ -13,14 +13,14 @@ export class InvalidArgumentsErrorHandler extends StandardErrorHandler { private displayInvalidArgsError() { // Ok if we have a space after the file name, this means we have some arguments defined and this isn't supported - vscode.window.showErrorMessage(`Unsupported configuration for '${this.id}'`, 'View Errors').then(item => { + window.showErrorMessage(`Unsupported configuration for '${this.id}'`, 'View Errors').then(item => { if (item === 'View Errors') { this.outputChannel.show(); } }); } - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (!isNotInstalledError(error)) { return false; } diff --git a/src/client/linters/errorHandlers/main.ts b/src/client/linters/errorHandlers/main.ts index 9503e01e4a69..cc9120627929 100644 --- a/src/client/linters/errorHandlers/main.ts +++ b/src/client/linters/errorHandlers/main.ts @@ -1,5 +1,5 @@ 'use strict'; -import { OutputChannel } from 'vscode'; +import { OutputChannel, Uri } from 'vscode'; import { Installer, Product } from '../../common/installer'; import { InvalidArgumentsErrorHandler } from './invalidArgs'; import { StandardErrorHandler } from './standard'; @@ -15,7 +15,7 @@ export class ErrorHandler { ]; } - public handleError(expectedFileName: string, fileName: string, error: Error) { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri) { this._errorHandlers.some(handler => handler.handleError(expectedFileName, fileName, error)); } } \ No newline at end of file diff --git a/src/client/linters/errorHandlers/notInstalled.ts b/src/client/linters/errorHandlers/notInstalled.ts index bc7c676a9390..f875997b184f 100644 --- a/src/client/linters/errorHandlers/notInstalled.ts +++ b/src/client/linters/errorHandlers/notInstalled.ts @@ -1,14 +1,15 @@ 'use strict'; +import { Uri } from 'vscode'; import { isNotInstalledError } from '../../common/helpers'; import { StandardErrorHandler } from './standard'; export class NotInstalledErrorHandler extends StandardErrorHandler { - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (!isNotInstalledError(error)) { return false; } - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, resource); const customError = `Linting with ${this.id} failed.\nYou could either install the '${this.id}' linter or turn it off in setings.json via "python.linting.${this.id}Enabled = false".`; this.outputChannel.appendLine(`\n${customError}\n${error + ''}`); return true; diff --git a/src/client/linters/errorHandlers/standard.ts b/src/client/linters/errorHandlers/standard.ts index 114d99e5c79b..e9f665a0d09c 100644 --- a/src/client/linters/errorHandlers/standard.ts +++ b/src/client/linters/errorHandlers/standard.ts @@ -1,18 +1,17 @@ 'use strict'; -import { OutputChannel } from 'vscode'; -import { Installer, Product, disableLinter } from '../../common/installer'; -import * as vscode from 'vscode'; +import { OutputChannel, Uri, window } from 'vscode'; +import { Installer, Product } from '../../common/installer'; export class StandardErrorHandler { constructor(protected id: string, protected product: Product, protected installer: Installer, protected outputChannel: OutputChannel) { } - private displayLinterError() { + private displayLinterError(resource?: Uri) { const message = `There was an error in running the linter '${this.id}'`; - vscode.window.showErrorMessage(message, 'Disable linter', 'View Errors').then(item => { + window.showErrorMessage(message, 'Disable linter', 'View Errors').then(item => { switch (item) { case 'Disable linter': { - disableLinter(this.product); + this.installer.disableLinter(this.product, resource); break; } case 'View Errors': { @@ -23,7 +22,7 @@ export class StandardErrorHandler { }); } - public handleError(expectedFileName: string, fileName: string, error: Error): boolean { + public handleError(expectedFileName: string, fileName: string, error: Error, resource?: Uri): boolean { if (typeof error === 'string' && (error as string).indexOf("OSError: [Errno 2] No such file or directory: '/") > 0) { return false; } @@ -31,7 +30,7 @@ export class StandardErrorHandler { console.error(error); this.outputChannel.appendLine(`Linting with ${this.id} failed.\n${error + ''}`); - this.displayLinterError(); + this.displayLinterError(resource); return true; } } diff --git a/src/client/linters/prospector.ts b/src/client/linters/prospector.ts index 42c14dfd76e2..6dc2ce4dcfb8 100644 --- a/src/client/linters/prospector.ts +++ b/src/client/linters/prospector.ts @@ -39,8 +39,8 @@ export class Linter extends baseLinter.BaseLinter { let prospectorPath = this.pythonSettings.linting.prospectorPath; let outputChannel = this.outputChannel; let prospectorArgs = Array.isArray(this.pythonSettings.linting.prospectorArgs) ? this.pythonSettings.linting.prospectorArgs : []; - - if (prospectorArgs.length === 0 && ProductExecutableAndArgs.has(Product.prospector) && prospectorPath.toLocaleLowerCase() === 'prospector'){ + + if (prospectorArgs.length === 0 && ProductExecutableAndArgs.has(Product.prospector) && prospectorPath.toLocaleLowerCase() === 'prospector') { prospectorPath = ProductExecutableAndArgs.get(Product.prospector).executable; prospectorArgs = ProductExecutableAndArgs.get(Product.prospector).args; } @@ -73,7 +73,7 @@ export class Linter extends baseLinter.BaseLinter { resolve(diagnostics); }).catch(error => { - this.handleError(this.Id, prospectorPath, error); + this.handleError(this.Id, prospectorPath, error, document.uri); resolve([]); }); }); diff --git a/src/client/linters/pydocstyle.ts b/src/client/linters/pydocstyle.ts index b361bf1640ec..1c5cc9b374b2 100644 --- a/src/client/linters/pydocstyle.ts +++ b/src/client/linters/pydocstyle.ts @@ -102,7 +102,7 @@ export class Linter extends baseLinter.BaseLinter { }); resolve(diagnostics); }, error => { - this.handleError(this.Id, commandLine, error); + this.handleError(this.Id, commandLine, error, document.uri); resolve([]); }); }); diff --git a/src/client/providers/renameProvider.ts b/src/client/providers/renameProvider.ts index 9afcb4db6c47..50205bfc75dc 100644 --- a/src/client/providers/renameProvider.ts +++ b/src/client/providers/renameProvider.ts @@ -48,7 +48,7 @@ export class PythonRenameProvider implements vscode.RenameProvider { return workspaceEdit; }).catch(reason => { if (reason === 'Not installed') { - this.installer.promptToInstall(Product.rope); + this.installer.promptToInstall(Product.rope, document.uri); return Promise.reject(''); } else { diff --git a/src/client/providers/simpleRefactorProvider.ts b/src/client/providers/simpleRefactorProvider.ts index 99e62ac55b5a..080db9518de6 100644 --- a/src/client/providers/simpleRefactorProvider.ts +++ b/src/client/providers/simpleRefactorProvider.ts @@ -127,7 +127,7 @@ function extractName(extensionDir: string, textEditor: vscode.TextEditor, range: } }).catch(error => { if (error === 'Not installed') { - installer.promptToInstall(Product.rope); + installer.promptToInstall(Product.rope, textEditor.document.uri); return Promise.reject(''); } let errorMessage = error + ''; diff --git a/src/client/unittests/common/baseTestManager.ts b/src/client/unittests/common/baseTestManager.ts index e6c31d31a0be..1dc38f7259b2 100644 --- a/src/client/unittests/common/baseTestManager.ts +++ b/src/client/unittests/common/baseTestManager.ts @@ -90,7 +90,7 @@ export abstract class BaseTestManager { return tests; }).catch(reason => { if (isNotInstalledError(reason) && !quietMode) { - this.installer.promptToInstall(this.product); + this.installer.promptToInstall(this.product, vscode.Uri.file(this.rootDirectory)); } this.tests = null; diff --git a/src/client/workspaceSymbols/main.ts b/src/client/workspaceSymbols/main.ts index 835c7cec545c..5c0206290d0e 100644 --- a/src/client/workspaceSymbols/main.ts +++ b/src/client/workspaceSymbols/main.ts @@ -1,6 +1,6 @@ import * as vscode from 'vscode'; import { Generator } from './generator'; -import { Product, Installer } from '../common/installer'; +import { Installer, InstallerResponse, Product } from '../common/installer'; import { PythonSettings } from '../common/configSettings'; import { fsExistsAsync } from '../common/utils'; import { isNotInstalledError } from '../common/helpers'; @@ -52,11 +52,6 @@ export class WorkspaceSymbols implements vscode.Disposable { dispose() { this.disposables.forEach(d => d.dispose()); } - disableDocumentLanguageProvider(): Thenable { - const pythonConfig = vscode.workspace.getConfiguration('python'); - return pythonConfig.update('python.workspaceSymbols.enabled', false); - - } buildWorkspaceSymbols(rebuild: boolean = true, token?: vscode.CancellationToken): Promise { if (!pythonSettings.workspaceSymbols.enabled || (token && token.isCancellationRequested)) { return Promise.resolve([]); @@ -81,23 +76,15 @@ export class WorkspaceSymbols implements vscode.Disposable { if (!token || token.isCancellationRequested) { return; } - return new Promise((resolve, reject) => { - vscode.window.showErrorMessage('CTags needs to be installed to get support for Python workspace symbols', - 'Install', `Don't ask again`).then(item => { - switch (item) { - case 'Install': { - this.installer.install(Product.ctags).then(() => { - return this.buildWorkspaceSymbols(rebuild, token); - }).catch(reason => reject(reason)); - break; - } - case `Don't ask again`: { - this.disableDocumentLanguageProvider().then(() => resolve(), reason => reject(reason)); - break; - } - } - }); - }); + return this.installer.promptToInstall(Product.ctags) + .then(result => { + if (!token || token.isCancellationRequested) { + return; + } + if (result === InstallerResponse.Installed) { + return this.buildWorkspaceSymbols(rebuild, token); + } + }); }); }); } diff --git a/src/test/autocomplete/base.test.ts b/src/test/autocomplete/base.test.ts index 609bed153456..83123b308f91 100644 --- a/src/test/autocomplete/base.test.ts +++ b/src/test/autocomplete/base.test.ts @@ -114,7 +114,7 @@ suite('Autocomplete', () => { const position = new vscode.Position(10, 9); const list = await vscode.commands.executeCommand('vscode.executeCompletionItemProvider', textDocument.uri, position); assert.notEqual(list.items.filter(item => item.label === 'sleep').length, 0, 'sleep not found'); - assert.notEqual(list.items.filter(item => item.documentation.startsWith("Delay execution for a given number of seconds. The argument may be")).length, 0, 'Documentation incorrect'); + assert.notEqual(list.items.filter(item => item.documentation.toString().startsWith("Delay execution for a given number of seconds. The argument may be")).length, 0, 'Documentation incorrect'); }); test('For custom class', done => { From 663b3455329e9c89ac9f614a08b31b818a4385b8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 13:54:07 -0700 Subject: [PATCH 10/24] null test in installer --- src/client/common/installer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/installer.ts b/src/client/common/installer.ts index 6d7af57228a5..8710273ee857 100644 --- a/src/client/common/installer.ts +++ b/src/client/common/installer.ts @@ -320,7 +320,7 @@ export class Installer implements vscode.Disposable { } function getCwdForInstallScript(resource?: Uri) { - const workspaceFolder = workspace.getWorkspaceFolder(resource); + const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined; if (workspaceFolder) { return workspaceFolder.uri.fsPath; } From bcca381e9e13085d47ac27e211ca6c1b3ae8b53c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 14:10:02 -0700 Subject: [PATCH 11/24] canges to config settings to support multiroot workspace --- src/client/common/configSettings.ts | 2 +- src/client/common/systemVariables.ts | 5 ++--- src/test/common/configSettings.test.ts | 4 +++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 32ef9ec060fb..a7d013a1d68f 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -160,8 +160,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { return PythonSettings.pythonSettings.get(workspaceFolderKey); } private initializeSettings() { - const systemVariables: SystemVariables = new SystemVariables(); const workspaceRoot = (IS_TEST_EXECUTION || !this.workspaceRoot) ? __dirname : this.workspaceRoot.fsPath; + const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined); const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot); this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; this.pythonPath = getAbsolutePath(this.pythonPath, IS_TEST_EXECUTION ? __dirname : workspaceRoot); diff --git a/src/client/common/systemVariables.ts b/src/client/common/systemVariables.ts index f0df0218c2d9..e4adcaf67862 100644 --- a/src/client/common/systemVariables.ts +++ b/src/client/common/systemVariables.ts @@ -133,11 +133,10 @@ export abstract class AbstractSystemVariables implements ISystemVariables { export class SystemVariables extends AbstractSystemVariables { private _workspaceRoot: string; private _workspaceRootFolderName: string; - private _execPath: string; - constructor() { + constructor(workspaceRoot?: string) { super(); - this._workspaceRoot = typeof vscode.workspace.rootPath === 'string' ? vscode.workspace.rootPath : __dirname; + this._workspaceRoot = typeof workspaceRoot === 'string' ? vscode.workspace.rootPath : __dirname; this._workspaceRootFolderName = Path.basename(this._workspaceRoot); Object.keys(process.env).forEach(key => { this[`env:${key}`] = this[`env.${key}`] = process.env[key]; diff --git a/src/test/common/configSettings.test.ts b/src/test/common/configSettings.test.ts index 8382b8f4e1c9..07cfe597033a 100644 --- a/src/test/common/configSettings.test.ts +++ b/src/test/common/configSettings.test.ts @@ -9,11 +9,13 @@ import * as assert from 'assert'; // You can import and use all API from the 'vscode' module // as well as import your extension to test it import * as vscode from 'vscode'; +import * as path from 'path'; import { initialize, IS_TRAVIS } from './../initialize'; import { PythonSettings } from '../../client/common/configSettings'; import { SystemVariables } from '../../client/common/systemVariables'; const pythonSettings = PythonSettings.getInstance(); +const workspaceRoot = path.join(__dirname, '..', '..', '..', 'src', 'test'); // Defines a Mocha test suite to group tests of similar kind together suite('Configuration Settings', () => { @@ -21,7 +23,7 @@ suite('Configuration Settings', () => { if (!IS_TRAVIS) { test('Check Values', done => { - const systemVariables: SystemVariables = new SystemVariables(); + const systemVariables: SystemVariables = new SystemVariables(workspaceRoot); const pythonConfig = vscode.workspace.getConfiguration('python'); Object.keys(pythonSettings).forEach(key => { let settingValue = pythonConfig.get(key, 'Not a config'); From e3279cbcc197c3b4a22207cb6b9f5fc387f28e35 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 14:13:08 -0700 Subject: [PATCH 12/24] changes to code refactoring to support workspace symbols --- src/client/common/editor.ts | 4 ++-- src/client/providers/renameProvider.ts | 15 ++++++++----- .../providers/simpleRefactorProvider.ts | 22 ++++++++++++++----- src/client/refactor/proxy.ts | 2 +- src/test/index.ts | 3 ++- .../extension.refactor.extract.method.test.ts | 2 +- .../extension.refactor.extract.var.test.ts | 2 +- 7 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index 9d23d38ff678..7da7a2657e8b 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -75,7 +75,7 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[] return textEdits; } -export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit { +export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?:string): WorkspaceEdit { const workspaceEdit = new WorkspaceEdit(); filePatches.forEach(patch => { const indexOfAtAt = patch.indexOf('@@'); @@ -101,7 +101,7 @@ export function getWorkspaceEditsFromPatch(filePatches: string[]): WorkspaceEdit } let fileName = fileNameLines[0].substring(fileNameLines[0].indexOf(' a') + 3).trim(); - fileName = path.isAbsolute(fileName) ? fileName : path.resolve(vscode.workspace.rootPath, fileName); + fileName = workspaceRoot && !path.isAbsolute(fileName) ? path.resolve(workspaceRoot, fileName) : fileName; if (!fs.existsSync(fileName)) { return; } diff --git a/src/client/providers/renameProvider.ts b/src/client/providers/renameProvider.ts index 50205bfc75dc..e65b915b856b 100644 --- a/src/client/providers/renameProvider.ts +++ b/src/client/providers/renameProvider.ts @@ -7,7 +7,6 @@ import * as path from 'path'; import { PythonSettings } from '../common/configSettings'; import { Installer, Product } from '../common/installer'; -const pythonSettings = PythonSettings.getInstance(); const EXTENSION_DIR = path.join(__dirname, '..', '..', '..'); interface RenameResponse { results: [{ diff: string }]; @@ -41,11 +40,17 @@ export class PythonRenameProvider implements vscode.RenameProvider { return; } - let proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, vscode.workspace.rootPath); + let workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); + + let proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, workspaceRoot); return proxy.rename(document, newName, document.uri.fsPath, range).then(response => { - //return response.results[0].diff; - const workspaceEdit = getWorkspaceEditsFromPatch(response.results.map(fileChanges => fileChanges.diff)); - return workspaceEdit; + const fileDiffs = response.results.map(fileChanges => fileChanges.diff); + return getWorkspaceEditsFromPatch(fileDiffs, workspaceRoot); }).catch(reason => { if (reason === 'Not installed') { this.installer.promptToInstall(Product.rope, document.uri); diff --git a/src/client/providers/simpleRefactorProvider.ts b/src/client/providers/simpleRefactorProvider.ts index 080db9518de6..35f765165eb1 100644 --- a/src/client/providers/simpleRefactorProvider.ts +++ b/src/client/providers/simpleRefactorProvider.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import { RefactorProxy } from '../refactor/proxy'; import { getTextEditsFromPatch } from '../common/editor'; -import { PythonSettings, IPythonSettings } from '../common/configSettings'; +import { PythonSettings } from '../common/configSettings'; import { Installer, Product } from '../common/installer'; interface RenameResponse { @@ -34,8 +34,14 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo // Exported for unit testing export function extractVariable(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, - outputChannel: vscode.OutputChannel, workspaceRoot: string = vscode.workspace.rootPath, - pythonSettings: IPythonSettings = PythonSettings.getInstance()): Promise { + outputChannel: vscode.OutputChannel): Promise { + + let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { let newName = 'newvariable' + new Date().getMilliseconds().toString(); @@ -50,8 +56,14 @@ export function extractVariable(extensionDir: string, textEditor: vscode.TextEdi // Exported for unit testing export function extractMethod(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range, - outputChannel: vscode.OutputChannel, workspaceRoot: string = vscode.workspace.rootPath, - pythonSettings: IPythonSettings = PythonSettings.getInstance()): Promise { + outputChannel: vscode.OutputChannel): Promise { + + let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri); + if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { + workspaceFolder = vscode.workspace.workspaceFolders[0]; + } + const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname; + const pythonSettings = PythonSettings.getInstance(workspaceFolder ? workspaceFolder.uri : undefined); return validateDocumentForRefactor(textEditor).then(() => { let newName = 'newmethod' + new Date().getMilliseconds().toString(); diff --git a/src/client/refactor/proxy.ts b/src/client/refactor/proxy.ts index 41a717036754..aada24c2323d 100644 --- a/src/client/refactor/proxy.ts +++ b/src/client/refactor/proxy.ts @@ -17,7 +17,7 @@ export class RefactorProxy extends vscode.Disposable { private _commandResolve: (value?: any | PromiseLike) => void; private _commandReject: (reason?: any) => void; private _initializeReject: (reason?: any) => void; - constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string = vscode.workspace.rootPath) { + constructor(extensionDir: string, private pythonSettings: IPythonSettings, private workspaceRoot: string) { super(() => { }); this._extensionDir = extensionDir; } diff --git a/src/test/index.ts b/src/test/index.ts index af916a981db9..28acd887d255 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -18,7 +18,8 @@ let testRunner = require('vscode/lib/testrunner'); testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000 + timeout: 25000, + grep: 'Configuration Settings' }); initializePython(); diff --git a/src/test/refactor/extension.refactor.extract.method.test.ts b/src/test/refactor/extension.refactor.extract.method.test.ts index f3cd520f608c..021bb90532ff 100644 --- a/src/test/refactor/extension.refactor.extract.method.test.ts +++ b/src/test/refactor/extension.refactor.extract.method.test.ts @@ -119,7 +119,7 @@ suite('Method Extraction', () => { textEditor = editor; return; }).then(() => { - return extractMethod(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch, path.dirname(refactorTargetFile), pythonSettings).then(() => { + return extractMethod(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch).then(() => { if (shouldError) { ignoreErrorHandling = true; assert.fail('No error', 'Error', 'Extraction should fail with an error', ''); diff --git a/src/test/refactor/extension.refactor.extract.var.test.ts b/src/test/refactor/extension.refactor.extract.var.test.ts index 7428a6b1d43d..1e2a896a4e56 100644 --- a/src/test/refactor/extension.refactor.extract.var.test.ts +++ b/src/test/refactor/extension.refactor.extract.var.test.ts @@ -117,7 +117,7 @@ suite('Variable Extraction', () => { textEditor = editor; return; }).then(() => { - return extractVariable(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch, path.dirname(refactorTargetFile), pythonSettings).then(() => { + return extractVariable(EXTENSION_DIR, textEditor, rangeOfTextToExtract, ch).then(() => { if (shouldError) { ignoreErrorHandling = true; assert.fail('No error', 'Error', 'Extraction should fail with an error', ''); From 965eae9272a03a280f3f2ce066c6f7f46cea9dc8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 14:16:34 -0700 Subject: [PATCH 13/24] oops --- src/test/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index 28acd887d255..af916a981db9 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -18,8 +18,7 @@ let testRunner = require('vscode/lib/testrunner'); testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000, - grep: 'Configuration Settings' + timeout: 25000 }); initializePython(); From 234ff785a35268818078ff2acbfd334474edf363 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 9 Oct 2017 15:16:24 -0700 Subject: [PATCH 14/24] modified to settings are resolved using document uri --- src/client/linters/baseLinter.ts | 16 ++++++----- src/client/linters/flake8.ts | 5 +--- src/client/linters/mypy.ts | 5 +--- src/client/linters/pep8Linter.ts | 11 +++----- src/client/linters/prospector.ts | 5 +--- src/client/linters/pydocstyle.ts | 5 +--- src/client/linters/pylama.ts | 5 +--- src/client/linters/pylint.ts | 5 +--- src/client/providers/lintProvider.ts | 40 ++++++++++------------------ src/test/linters/lint.test.ts | 29 +++++++++++++++----- 10 files changed, 57 insertions(+), 69 deletions(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index b4cd6ca154ed..9e1dcd8a54f5 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -1,6 +1,6 @@ 'use strict'; +import { IPythonSettings, PythonSettings } from '../common/configSettings'; import { execPythonFile } from './../common/utils'; -import * as settings from './../common/configSettings'; import { OutputChannel } from 'vscode'; import { Installer, Product } from '../common/installer'; import * as vscode from 'vscode'; @@ -49,9 +49,12 @@ export function matchNamedRegEx(data, regex): IRegexGroup { export abstract class BaseLinter { public Id: string; - protected pythonSettings: settings.IPythonSettings; protected _columnOffset = 0; private _errorHandler: ErrorHandler; + private _pythonSettings: IPythonSettings; + protected get pythonSettings(): IPythonSettings { + return this._pythonSettings; + } protected getWorkspaceRootPath(document: vscode.TextDocument): string { const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined; @@ -59,12 +62,13 @@ export abstract class BaseLinter { } constructor(id: string, public product: Product, protected outputChannel: OutputChannel) { this.Id = id; - this.pythonSettings = settings.PythonSettings.getInstance(); this._errorHandler = new ErrorHandler(this.Id, product, new Installer(), this.outputChannel); } - public abstract isEnabled(): Boolean; - public abstract runLinter(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise; - + public lint(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise { + this._pythonSettings = PythonSettings.getInstance(document.uri); + return this.runLinter(document, cancellation); + } + protected abstract runLinter(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise; protected parseMessagesSeverity(error: string, categorySeverity: any): LintMessageSeverity { if (categorySeverity[error]) { let severityName = categorySeverity[error]; diff --git a/src/client/linters/flake8.ts b/src/client/linters/flake8.ts index b141b3a3a86c..00bca18c28d4 100644 --- a/src/client/linters/flake8.ts +++ b/src/client/linters/flake8.ts @@ -12,10 +12,7 @@ export class Linter extends baseLinter.BaseLinter { super('flake8', Product.flake8, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.flake8Enabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.flake8Enabled) { return Promise.resolve([]); } diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts index 407c218bc08f..8696d4db5a78 100644 --- a/src/client/linters/mypy.ts +++ b/src/client/linters/mypy.ts @@ -12,10 +12,7 @@ export class Linter extends baseLinter.BaseLinter { super('mypy', Product.mypy, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.mypyEnabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.mypyEnabled) { return Promise.resolve([]); } diff --git a/src/client/linters/pep8Linter.ts b/src/client/linters/pep8Linter.ts index bb42f3c30834..18f77e4bce54 100644 --- a/src/client/linters/pep8Linter.ts +++ b/src/client/linters/pep8Linter.ts @@ -7,23 +7,20 @@ import { TextDocument, CancellationToken } from 'vscode'; export class Linter extends baseLinter.BaseLinter { _columnOffset = 1; - + constructor(outputChannel: OutputChannel) { super('pep8', Product.pep8, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.pep8Enabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.pep8Enabled) { return Promise.resolve([]); } let pep8Path = this.pythonSettings.linting.pep8Path; let pep8Args = Array.isArray(this.pythonSettings.linting.pep8Args) ? this.pythonSettings.linting.pep8Args : []; - - if (pep8Args.length === 0 && ProductExecutableAndArgs.has(Product.pep8) && pep8Path.toLocaleLowerCase() === 'pep8'){ + + if (pep8Args.length === 0 && ProductExecutableAndArgs.has(Product.pep8) && pep8Path.toLocaleLowerCase() === 'pep8') { pep8Path = ProductExecutableAndArgs.get(Product.pep8).executable; pep8Args = ProductExecutableAndArgs.get(Product.pep8).args; } diff --git a/src/client/linters/prospector.ts b/src/client/linters/prospector.ts index 42c14dfd76e2..e02a10f210be 100644 --- a/src/client/linters/prospector.ts +++ b/src/client/linters/prospector.ts @@ -28,10 +28,7 @@ export class Linter extends baseLinter.BaseLinter { super('prospector', Product.prospector, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.prospectorEnabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.prospectorEnabled) { return Promise.resolve([]); } diff --git a/src/client/linters/pydocstyle.ts b/src/client/linters/pydocstyle.ts index b361bf1640ec..e58fa4edd5a5 100644 --- a/src/client/linters/pydocstyle.ts +++ b/src/client/linters/pydocstyle.ts @@ -13,10 +13,7 @@ export class Linter extends baseLinter.BaseLinter { super('pydocstyle', Product.pydocstyle, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.pydocstyleEnabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.pydocstyleEnabled) { return Promise.resolve([]); } diff --git a/src/client/linters/pylama.ts b/src/client/linters/pylama.ts index 718ec652adf4..f0251ee728ea 100644 --- a/src/client/linters/pylama.ts +++ b/src/client/linters/pylama.ts @@ -14,10 +14,7 @@ export class Linter extends baseLinter.BaseLinter { super('pylama', Product.pylama, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.pylamaEnabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.pylamaEnabled) { return Promise.resolve([]); } diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index d485f7764ba1..5649d079f008 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -10,10 +10,7 @@ export class Linter extends baseLinter.BaseLinter { super('pylint', Product.pylint, outputChannel); } - public isEnabled(): Boolean { - return this.pythonSettings.linting.pylintEnabled; - } - public runLinter(document: TextDocument, cancellation: CancellationToken): Promise { + protected runLinter(document: TextDocument, cancellation: CancellationToken): Promise { if (!this.pythonSettings.linting.pylintEnabled) { return Promise.resolve([]); } diff --git a/src/client/providers/lintProvider.ts b/src/client/providers/lintProvider.ts index 165055bfccc8..b9b5b92107a7 100644 --- a/src/client/providers/lintProvider.ts +++ b/src/client/providers/lintProvider.ts @@ -10,7 +10,7 @@ import * as pylama from './../linters/pylama'; import * as flake8 from './../linters/flake8'; import * as pydocstyle from './../linters/pydocstyle'; import * as mypy from './../linters/mypy'; -import * as settings from '../common/configSettings'; +import { PythonSettings } from '../common/configSettings'; import * as fs from 'fs'; import { LinterErrors } from '../common/constants'; const Minimatch = require("minimatch").Minimatch; @@ -37,37 +37,23 @@ interface DocumentHasJupyterCodeCells { (doc: vscode.TextDocument, token: vscode.CancellationToken): Promise; } export class LintProvider extends vscode.Disposable { - private settings: settings.IPythonSettings; private diagnosticCollection: vscode.DiagnosticCollection; private linters: linter.BaseLinter[] = []; private pendingLintings = new Map(); private outputChannel: vscode.OutputChannel; private context: vscode.ExtensionContext; private disposables: vscode.Disposable[]; - private ignoreMinmatches: { match: (fname: string) => boolean }[]; public constructor(context: vscode.ExtensionContext, outputChannel: vscode.OutputChannel, public documentHasJupyterCodeCells: DocumentHasJupyterCodeCells) { super(() => { }); this.outputChannel = outputChannel; this.context = context; - this.settings = settings.PythonSettings.getInstance(); this.disposables = []; - this.ignoreMinmatches = []; this.initialize(); - - this.disposables.push(vscode.workspace.onDidChangeConfiguration(this.onConfigChanged.bind(this))); } dispose() { this.disposables.forEach(d => d.dispose()); } - private onConfigChanged() { - this.initializeGlobs(); - } - private initializeGlobs() { - this.ignoreMinmatches = settings.PythonSettings.getInstance().linting.ignorePatterns.map(pattern => { - return new Minimatch(pattern); - }); - } private isDocumentOpen(uri: vscode.Uri): boolean { return vscode.window.visibleTextEditors.some(editor => editor.document && editor.document.uri.fsPath === uri.fsPath); } @@ -84,7 +70,8 @@ export class LintProvider extends vscode.Disposable { this.linters.push(new mypy.Linter(this.outputChannel)); let disposable = vscode.workspace.onDidSaveTextDocument((e) => { - if (e.languageId !== 'python' || !this.settings.linting.enabled || !this.settings.linting.lintOnSave) { + const settings = PythonSettings.getInstance(e.uri); + if (e.languageId !== 'python' || !settings.linting.enabled || !settings.linting.lintOnSave) { return; } this.lintDocument(e, 100); @@ -92,7 +79,8 @@ export class LintProvider extends vscode.Disposable { this.context.subscriptions.push(disposable); vscode.workspace.onDidOpenTextDocument((e) => { - if (e.languageId !== 'python' || !this.settings.linting.enabled) { + const settings = PythonSettings.getInstance(e.uri); + if (e.languageId !== 'python' || !settings.linting.enabled) { return; } // Exclude files opened by vscode when showing a diff view @@ -116,7 +104,6 @@ export class LintProvider extends vscode.Disposable { } }); this.context.subscriptions.push(disposable); - this.initializeGlobs(); } private lastTimeout: number; @@ -138,7 +125,12 @@ export class LintProvider extends vscode.Disposable { const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); const workspaceRootPath = (workspaceFolder && typeof workspaceFolder.uri.fsPath === 'string') ? workspaceFolder.uri.fsPath : undefined; const relativeFileName = typeof workspaceRootPath === 'string' ? path.relative(workspaceRootPath, document.fileName) : document.fileName; - if (this.ignoreMinmatches.some(matcher => matcher.match(document.fileName) || matcher.match(relativeFileName))) { + const settings = PythonSettings.getInstance(document.uri); + const ignoreMinmatches = settings.linting.ignorePatterns.map(pattern => { + return new Minimatch(pattern); + }); + + if (ignoreMinmatches.some(matcher => matcher.match(document.fileName) || matcher.match(relativeFileName))) { return; } if (this.pendingLintings.has(document.uri.fsPath)) { @@ -156,14 +148,10 @@ export class LintProvider extends vscode.Disposable { this.pendingLintings.set(document.uri.fsPath, cancelToken); this.outputChannel.clear(); let promises: Promise[] = this.linters.map(linter => { - if (typeof workspaceRootPath !== 'string' && !this.settings.linting.enabledWithoutWorkspace) { - return Promise.resolve([]); - } - if (!linter.isEnabled()) { + if (typeof workspaceRootPath !== 'string' && !settings.linting.enabledWithoutWorkspace) { return Promise.resolve([]); } - // turn off telemetry for linters (at least for now) - return linter.runLinter(document, cancelToken.token); + return linter.lint(document, cancelToken.token); }); this.documentHasJupyterCodeCells(document, cancelToken.token).then(hasJupyterCodeCells => { // linters will resolve asynchronously - keep a track of all @@ -189,7 +177,7 @@ export class LintProvider extends vscode.Disposable { }); // Limit the number of messages to the max value - diagnostics = diagnostics.filter((value, index) => index <= this.settings.linting.maxNumberOfProblems); + diagnostics = diagnostics.filter((value, index) => index <= settings.linting.maxNumberOfProblems); if (!this.isDocumentOpen(document.uri)) { diagnostics = []; diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index 45657a478476..bf96de28cfc3 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -141,14 +141,31 @@ suite('Linting', () => { pythonSettings.linting.pydocstyleEnabled = true; }); suiteTeardown(() => closeActiveWindows()); - teardown(() => closeActiveWindows()); + teardown(() => { + closeActiveWindows(); + pythonSettings.linting.lintOnSave = false; + pythonSettings.linting.lintOnTextChange = false; + pythonSettings.linting.enabled = true; + pythonSettings.linting.pylintEnabled = true; + pythonSettings.linting.flake8Enabled = true; + pythonSettings.linting.pep8Enabled = true; + pythonSettings.linting.prospectorEnabled = true; + pythonSettings.linting.pydocstyleEnabled = true; + }); - function testEnablingDisablingOfLinter(linter: baseLinter.BaseLinter, propertyName: string) { + async function testEnablingDisablingOfLinter(linter: baseLinter.BaseLinter, propertyName: string) { pythonSettings.linting[propertyName] = true; - assert.equal(true, linter.isEnabled()); - pythonSettings.linting[propertyName] = false; - assert.equal(false, linter.isEnabled()); + let cancelToken = new vscode.CancellationTokenSource(); + disableAllButThisLinter(linter.product); + const document = await vscode.workspace.openTextDocument(fileToLint); + const editor = await vscode.window.showTextDocument(document); + try { + const messages = await linter.lint(editor.document, cancelToken.token); + assert.equal(messages.length, 0, 'Errors returned even when linter is disabled'); + } catch (error) { + assert.fail(error, null, 'Linter error'); + } } test('Enable and Disable Pylint', () => { let ch = new MockOutputChannel('Lint'); @@ -187,7 +204,7 @@ suite('Linting', () => { return vscode.workspace.openTextDocument(pythonFile) .then(document => vscode.window.showTextDocument(document)) .then(editor => { - return linter.runLinter(editor.document, cancelToken.token); + return linter.lint(editor.document, cancelToken.token); }) .then(messages => { // Different versions of python return different errors, From da4e2ab8eef428907dda2cd4b352b8eb2bf71340 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 10 Oct 2017 17:35:55 -0700 Subject: [PATCH 15/24] unit tests for multi root support --- .vscode/launch.json | 17 ++ .vscode/settings.json | 3 +- package.json | 4 +- src/client/common/configSettings.ts | 12 +- src/client/common/logger.ts | 4 +- .../common/configSettings.multiroot.test.ts | 165 ++++++++++++++++++ src/test/index.ts | 9 +- src/test/initialize.ts | 4 + src/test/linters/lint.multiroot.test.ts | 80 +++++++++ src/test/multiRootTest.ts | 8 + .../multiRootWkspc/disableLinters/file.py | 87 +++++++++ src/test/multiRootWkspc/multi.code-workspace | 22 +++ src/test/multiRootWkspc/parent/child/file.py | 87 +++++++++ .../workspace1/.vscode/settings.json | 5 + src/test/multiRootWkspc/workspace1/file.py | 87 +++++++++ 15 files changed, 583 insertions(+), 11 deletions(-) create mode 100644 src/test/common/configSettings.multiroot.test.ts create mode 100644 src/test/linters/lint.multiroot.test.ts create mode 100644 src/test/multiRootTest.ts create mode 100644 src/test/multiRootWkspc/disableLinters/file.py create mode 100644 src/test/multiRootWkspc/multi.code-workspace create mode 100644 src/test/multiRootWkspc/parent/child/file.py create mode 100644 src/test/multiRootWkspc/workspace1/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace1/file.py diff --git a/.vscode/launch.json b/.vscode/launch.json index 284cb90bbaa5..6d4317be09b1 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -67,6 +67,23 @@ ], "preLaunchTask": "npm" }, + { + "name": "Launch Multiroot Tests", + "type": "extensionHost", + "request": "launch", + "runtimeExecutable": "${execPath}", + "args": [ + "${workspaceRoot}/src/test/multiRootWkspc/multi.code-workspace", + "--extensionDevelopmentPath=${workspaceRoot}", + "--extensionTestsPath=${workspaceRoot}/out/test" + ], + "stopOnEntry": false, + "sourceMaps": true, + "outFiles": [ + "${workspaceRoot}/out/**/*.js" + ], + "preLaunchTask": "npm" + }, { "name": "Python", "type": "python", diff --git a/.vscode/settings.json b/.vscode/settings.json index 2dc22387c78a..a09d3f11e14e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -4,7 +4,8 @@ "out": false, // set this to true to hide the "out" folder with the compiled JS files "**/*.pyc": true, "**/__pycache__": true, - "node_modules": true + "node_modules": true, + ".vscode-test":true }, "search.exclude": { "out": true // set this to false to include "out" folder in search results diff --git a/package.json b/package.json index e19878709162..cb764423755b 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.15.0" + "vscode": "^1.17.0" }, "recommendations": [ "donjayamanne.jupyter" @@ -1594,7 +1594,7 @@ "vscode:prepublish": "tsc -p ./ && webpack", "compile": "webpack && tsc -watch -p ./", "postinstall": "node ./node_modules/vscode/bin/install", - "test": "node ./node_modules/vscode/bin/test" + "test": "node ./node_modules/vscode/bin/test && node ./out/test/multiRootTest.js" }, "dependencies": { "anser": "^1.1.0", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index a7d013a1d68f..97222940b071 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -146,6 +146,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { this.initializeSettings(); } + public static dispose() { + if (!IS_TEST_EXECUTION) { + throw new Error('Dispose can only be called from unit tests'); + } + PythonSettings.pythonSettings.clear(); + } public static getInstance(resource?: Uri): PythonSettings { const workspaceFolder = resource ? vscode.workspace.getWorkspaceFolder(resource) : undefined; let workspaceFolderUri: Uri = workspaceFolder ? workspaceFolder.uri : undefined; @@ -160,15 +166,15 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { return PythonSettings.pythonSettings.get(workspaceFolderKey); } private initializeSettings() { - const workspaceRoot = (IS_TEST_EXECUTION || !this.workspaceRoot) ? __dirname : this.workspaceRoot.fsPath; + const workspaceRoot = this.workspaceRoot.fsPath; const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined); const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot); this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; - this.pythonPath = getAbsolutePath(this.pythonPath, IS_TEST_EXECUTION ? __dirname : workspaceRoot); + this.pythonPath = getAbsolutePath(this.pythonPath, workspaceRoot); this.venvPath = systemVariables.resolveAny(pythonSettings.get('venvPath'))!; this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; if (typeof this.jediPath === 'string' && this.jediPath.length > 0) { - this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), IS_TEST_EXECUTION ? __dirname : workspaceRoot); + this.jediPath = getAbsolutePath(systemVariables.resolveAny(this.jediPath), workspaceRoot); } else { this.jediPath = ''; diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts index ac372199e258..173da70d78ab 100644 --- a/src/client/common/logger.ts +++ b/src/client/common/logger.ts @@ -22,7 +22,9 @@ class Logger { Logger.writeLine(category, message); } static writeLine(category: string = "log", line: any) { - console[category](line); + if (process.env['PYTHON_DONJAYAMANNE_TEST'] !== '1') { + console[category](line); + } if (outChannel) { outChannel.appendLine(line); } diff --git a/src/test/common/configSettings.multiroot.test.ts b/src/test/common/configSettings.multiroot.test.ts new file mode 100644 index 000000000000..b40b92ec6308 --- /dev/null +++ b/src/test/common/configSettings.multiroot.test.ts @@ -0,0 +1,165 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import { ConfigurationTarget, Uri, workspace } from 'vscode'; +import { initialize, closeActiveWindows } from '../initialize'; +import { PythonSettings } from '../../client/common/configSettings'; + +const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); + +suite('Multiroot Config Settings', () => { + suiteSetup(async () => { + await initialize(); + await resetSettings(); + }); + suiteTeardown(() => closeActiveWindows()); + teardown(async () => { + await resetSettings(); + // await closeActiveWindows(); + }); + + async function resetSettings() { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + let value = settings.inspect('pythonPath'); + if (value.workspaceValue && value.workspaceValue !== 'python') { + await settings.update('pythonPath', undefined, ConfigurationTarget.Workspace); + } + if (value.workspaceFolderValue && value.workspaceFolderValue !== 'python') { + await settings.update('pythonPath', undefined, ConfigurationTarget.WorkspaceFolder); + } + PythonSettings.dispose(); + } + async function enableDisableSetting(resource: Uri, configTarget: ConfigurationTarget, setting: string, value: boolean) { + let settings = workspace.getConfiguration('python.linting', resource); + await settings.update(setting, value, configTarget); + settings = workspace.getConfiguration('python.linting', resource); + return settings.get(setting); + } + + async function testLinterSetting(resource: Uri, configTarget: ConfigurationTarget, setting: string, value: boolean) { + const valueInSetting = await enableDisableSetting(resource, configTarget, setting, value); + PythonSettings.dispose(); + const cfgSetting = PythonSettings.getInstance(resource); + assert.equal(valueInSetting, cfgSetting.linting[setting], `Both settings ${setting} should be ${value} for ${resource.fsPath}`); + } + + test('Workspace folder should inherit Python Path from workspace root', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + + settings = workspace.getConfiguration('python', workspaceUri); + assert.equal(settings.get('pythonPath'), pythonPath, 'Python path not set in workspace root'); + + const cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.pythonPath, pythonPath, 'Python Path not inherited from workspace'); + }); + + test('Workspace folder should not inherit Python Path from workspace root', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + let settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + await settings.update('pythonPath', 'privatePythonPath', ConfigurationTarget.WorkspaceFolder); + + const cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.pythonPath, 'privatePythonPath', 'Python Path for workspace folder is incorrect'); + }); + + + test('Workspace folder should inherit Python Path from workspace root when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + const settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + + const document = await workspace.openTextDocument(fileToOpen); + const cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.pythonPath, pythonPath, 'Python Path not inherited from workspace'); + }); + + test('Workspace folder should not inherit Python Path from workspace root when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + const settings = workspace.getConfiguration('python', workspaceUri); + const pythonPath = `x${new Date().getTime()}`; + await settings.update('pythonPath', pythonPath, ConfigurationTarget.Workspace); + await settings.update('pythonPath', 'privatePythonPath', ConfigurationTarget.WorkspaceFolder); + + const document = await workspace.openTextDocument(fileToOpen); + const cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.pythonPath, 'privatePythonPath', 'Python Path for workspace folder is incorrect'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + }); + + test('Enabling/Disabling Pylint in root and workspace should be reflected in config settings', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const workspaceFolder = Uri.file(path.join(multirootPath, 'workspace1')); + + await testLinterSetting(workspaceFolder, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + + let cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.linting.pylintEnabled, false, 'Workspace folder pylint setting is true when it should not be'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceFolder, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + + cfgSetting = PythonSettings.getInstance(workspaceUri); + assert.equal(cfgSetting.linting.pylintEnabled, true, 'Workspace folder pylint setting is false when it should not be'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, true, 'Pylint should be enabled in workspace'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, false, 'Pylint should not be enabled in workspace'); + }); + + test('Enabling/Disabling Pylint in root should be reflected in config settings when opening a document', async () => { + const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); + const fileToOpen = path.join(multirootPath, 'workspace1', 'file.py'); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, true, 'Pylint should be enabled in workspace'); + PythonSettings.dispose(); + + await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(cfg.linting.pylintEnabled, false, 'Pylint should not be enabled in workspace'); + }); + + +}); diff --git a/src/test/index.ts b/src/test/index.ts index af916a981db9..af06fc19b486 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -1,4 +1,4 @@ -import { initializePython } from './initialize'; +import { initializePython, isMultitrootTest } from './initialize'; // // PLEASE DO NOT MODIFY / DELETE UNLESS YOU KNOW WHAT YOU ARE DOING // @@ -11,14 +11,15 @@ import { initializePython } from './initialize'; // to report the results back to the caller. When the tests are finished, return // a possible error to the callback or null if none. -let testRunner = require('vscode/lib/testrunner'); - +const testRunner = require('vscode/lib/testrunner'); +const grep = isMultitrootTest() ? 'Multiroot' : undefined; // You can directly control Mocha options by uncommenting the following lines // See https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically#set-options for more info testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results - timeout: 25000 + timeout: 25000, + grep }); initializePython(); diff --git a/src/test/initialize.ts b/src/test/initialize.ts index 1dcd6e247fd4..0ed5ca288c13 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -89,3 +89,7 @@ export function initializePython() { const pythonConfig = vscode.workspace.getConfiguration('python'); pythonConfig.update('pythonPath', PYTHON_PATH); } + +export function isMultitrootTest() { + return Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0; +} diff --git a/src/test/linters/lint.multiroot.test.ts b/src/test/linters/lint.multiroot.test.ts new file mode 100644 index 000000000000..a53d468492a7 --- /dev/null +++ b/src/test/linters/lint.multiroot.test.ts @@ -0,0 +1,80 @@ +// import * as assert from 'assert'; +// import * as path from 'path'; +// import * as baseLinter from '../../client/linters/baseLinter'; +// import * as pyLint from '../../client/linters/pylint'; +// import * as flake8 from '../../client/linters/flake8'; +// import { PythonSettings } from '../../client/common/configSettings'; +// import { CancellationTokenSource, ConfigurationTarget, Uri, window, workspace } from 'vscode'; +// import { initialize, closeActiveWindows } from '../initialize'; +// import { MockOutputChannel } from '../mockClasses'; + +// const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); + +// suite('Multiroot Linting', () => { +// suiteSetup(async () => { +// await initialize(); +// PythonSettings.dispose(); +// }); +// suiteTeardown(() => closeActiveWindows()); +// teardown(async () => { +// await closeActiveWindows(); +// PythonSettings.dispose(); +// }); + +// async function testLinterInWorkspaceFolder(linter: baseLinter.BaseLinter, workspaceFolderRelativePath: string, mustHaveErrors: boolean) { +// const fileToLint = path.join(multirootPath, workspaceFolderRelativePath, 'file.py'); +// const cancelToken = new CancellationTokenSource(); +// const document = await workspace.openTextDocument(fileToLint); +// const editor = await window.showTextDocument(document); +// const messages = await linter.lint(editor.document, cancelToken.token); +// const errorMessage = mustHaveErrors ? 'No errors returned by linter' : 'Errors returned by linter'; +// assert.equal(messages.length > 0, mustHaveErrors, errorMessage); +// } +// async function enableDisableSetting(workspaceFolder, configTarget: ConfigurationTarget, setting: string, value: boolean) { +// const folderUri = Uri.file(workspaceFolder); +// const settings = workspace.getConfiguration('python.linting', folderUri); +// await settings.update(setting, value, configTarget); +// } + +// test('Enabling Pylint in root and also in Workspace, should return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); +// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); +// }); + +// test('Enabling Pylint in root and disabling in Workspace, should not return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); +// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', false); +// }); + +// test('Disabling Pylint in root and enabling in Workspace, should return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', false); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); +// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); +// }); + +// test('Enabling Flake8 in root and also in Workspace, should return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); +// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); +// }); + +// test('Enabling Flake8 in root and disabling in Workspace, should not return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', false); +// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', false); +// }); + +// test('Disabling Flake8 in root and enabling in Workspace, should return errors', async () => { +// let ch = new MockOutputChannel('Lint'); +// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', false); +// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); +// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); +// }); +// }); diff --git a/src/test/multiRootTest.ts b/src/test/multiRootTest.ts new file mode 100644 index 000000000000..b7ad307ef20c --- /dev/null +++ b/src/test/multiRootTest.ts @@ -0,0 +1,8 @@ +import * as path from 'path'; + +process.env.CODE_TESTS_WORKSPACE = path.join(__dirname, '..', '..', 'src', 'test', 'multiRootWkspc', 'multi.code-workspace'); + +function start() { + require('../../node_modules/vscode/bin/test'); +} +start(); diff --git a/src/test/multiRootWkspc/disableLinters/file.py b/src/test/multiRootWkspc/disableLinters/file.py new file mode 100644 index 000000000000..047ba0dc679e --- /dev/null +++ b/src/test/multiRootWkspc/disableLinters/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print self + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print self\ + + "foo" + + def meth3(self): + """test one line disabling""" + # no error + print self.bla # pylint: disable=no-member + # error + print self.blop + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + print self.blop + # pylint: enable=no-member + # error + print self.blip + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + if self.blop: + # pylint: enable=no-member + # error + print self.blip + else: + # no error + print self.blip + # no error + print self.blip + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + try: + # pylint: enable=no-member + # error + print self.blip + except UndefinedName: # pylint: disable=undefined-variable + # no error + print self.blip + # no error + print self.blip + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print self.blip + else: + # error + print self.blip + # error + print self.blip + + + def meth8(self): + """test late disabling""" + # error + print self.blip + # pylint: disable=no-member + # no error + print self.bla + print self.blop \ No newline at end of file diff --git a/src/test/multiRootWkspc/multi.code-workspace b/src/test/multiRootWkspc/multi.code-workspace new file mode 100644 index 000000000000..bd948abca8a3 --- /dev/null +++ b/src/test/multiRootWkspc/multi.code-workspace @@ -0,0 +1,22 @@ +{ + "folders": [ + { + "path": "workspace1" + }, + { + "path": "parent\\child" + }, + { + "path": "disableLinters" + } + ], + "settings": { + "python.linting.flake8Enabled": false, + "python.linting.mypyEnabled": true, + "python.linting.pydocstyleEnabled": true, + "python.linting.pylamaEnabled": true, + "python.linting.pylintEnabled": true, + "python.linting.pep8Enabled": true, + "python.linting.prospectorEnabled": true + } +} \ No newline at end of file diff --git a/src/test/multiRootWkspc/parent/child/file.py b/src/test/multiRootWkspc/parent/child/file.py new file mode 100644 index 000000000000..047ba0dc679e --- /dev/null +++ b/src/test/multiRootWkspc/parent/child/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print self + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print self\ + + "foo" + + def meth3(self): + """test one line disabling""" + # no error + print self.bla # pylint: disable=no-member + # error + print self.blop + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + print self.blop + # pylint: enable=no-member + # error + print self.blip + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + if self.blop: + # pylint: enable=no-member + # error + print self.blip + else: + # no error + print self.blip + # no error + print self.blip + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + try: + # pylint: enable=no-member + # error + print self.blip + except UndefinedName: # pylint: disable=undefined-variable + # no error + print self.blip + # no error + print self.blip + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print self.blip + else: + # error + print self.blip + # error + print self.blip + + + def meth8(self): + """test late disabling""" + # error + print self.blip + # pylint: disable=no-member + # no error + print self.bla + print self.blop \ No newline at end of file diff --git a/src/test/multiRootWkspc/workspace1/.vscode/settings.json b/src/test/multiRootWkspc/workspace1/.vscode/settings.json new file mode 100644 index 000000000000..6155e911580c --- /dev/null +++ b/src/test/multiRootWkspc/workspace1/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "python.linting.pylintEnabled": false, + "python.linting.enabled": false, + "python.linting.flake8Enabled": true +} \ No newline at end of file diff --git a/src/test/multiRootWkspc/workspace1/file.py b/src/test/multiRootWkspc/workspace1/file.py new file mode 100644 index 000000000000..047ba0dc679e --- /dev/null +++ b/src/test/multiRootWkspc/workspace1/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print self + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print self\ + + "foo" + + def meth3(self): + """test one line disabling""" + # no error + print self.bla # pylint: disable=no-member + # error + print self.blop + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + print self.blop + # pylint: enable=no-member + # error + print self.blip + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + if self.blop: + # pylint: enable=no-member + # error + print self.blip + else: + # no error + print self.blip + # no error + print self.blip + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + try: + # pylint: enable=no-member + # error + print self.blip + except UndefinedName: # pylint: disable=undefined-variable + # no error + print self.blip + # no error + print self.blip + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print self.blip + else: + # error + print self.blip + # error + print self.blip + + + def meth8(self): + """test late disabling""" + # error + print self.blip + # pylint: disable=no-member + # no error + print self.bla + print self.blop \ No newline at end of file From b575dcc403e62d8cd4fe87f8da7eae23f6ad552d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 10:48:35 -0700 Subject: [PATCH 16/24] fix unittests for multiroot --- src/test/index.ts | 6 ++++-- src/test/initialize.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index af06fc19b486..e5fe465f9647 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -12,14 +12,16 @@ import { initializePython, isMultitrootTest } from './initialize'; // a possible error to the callback or null if none. const testRunner = require('vscode/lib/testrunner'); -const grep = isMultitrootTest() ? 'Multiroot' : undefined; +const invert = isMultitrootTest() ? undefined : 'invert'; + // You can directly control Mocha options by uncommenting the following lines // See https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically#set-options for more info testRunner.configure({ ui: 'tdd', // the TDD UI is being used in extension.test.ts (suite, test, etc.) useColors: true, // colored output from test results timeout: 25000, - grep + grep : 'Multiroot', + invert }); initializePython(); diff --git a/src/test/initialize.ts b/src/test/initialize.ts index 0ed5ca288c13..97b22674894f 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -91,5 +91,5 @@ export function initializePython() { } export function isMultitrootTest() { - return Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0; + return Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 1; } From 7457c8920937d1ec0444c35bc629d8134cfb8ea9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 10:49:26 -0700 Subject: [PATCH 17/24] exclude files --- .gitignore | 1 + .vscode/settings.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 58ce4158ddbe..f2183e471414 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ src/test/.vscode/** *.noseids .vscode-test __pycache__ +npm-debug.log \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json index a09d3f11e14e..1b512d4d5c07 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,7 +1,7 @@ // Place your settings in this file to overwrite default and user settings. { "files.exclude": { - "out": false, // set this to true to hide the "out" folder with the compiled JS files + "out": true, // set this to true to hide the "out" folder with the compiled JS files "**/*.pyc": true, "**/__pycache__": true, "node_modules": true, From 54741c13d412ad722c6a454800a017e07090be67 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 14:25:21 -0700 Subject: [PATCH 18/24] add new line --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f2183e471414..cb3999278277 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,4 @@ src/test/.vscode/** *.noseids .vscode-test __pycache__ -npm-debug.log \ No newline at end of file +npm-debug.log From ef306b001f9846747725671a08da95e46f8e69d3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 5 Oct 2017 15:47:14 -0700 Subject: [PATCH 19/24] config changes for multiroot workspace From 23ab7518d50db78bac50cf99b516278a988125c7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 14:59:12 -0700 Subject: [PATCH 20/24] new lines and enabled multi root linter tests --- src/test/linters/lint.multiroot.test.ts | 142 +++++++++--------- .../multiRootWkspc/disableLinters/file.py | 2 +- src/test/multiRootWkspc/multi.code-workspace | 4 +- src/test/multiRootWkspc/parent/child/file.py | 2 +- .../workspace1/.vscode/settings.json | 4 +- src/test/multiRootWkspc/workspace1/file.py | 2 +- 6 files changed, 78 insertions(+), 78 deletions(-) diff --git a/src/test/linters/lint.multiroot.test.ts b/src/test/linters/lint.multiroot.test.ts index a53d468492a7..e038ac9a1339 100644 --- a/src/test/linters/lint.multiroot.test.ts +++ b/src/test/linters/lint.multiroot.test.ts @@ -1,80 +1,80 @@ -// import * as assert from 'assert'; -// import * as path from 'path'; -// import * as baseLinter from '../../client/linters/baseLinter'; -// import * as pyLint from '../../client/linters/pylint'; -// import * as flake8 from '../../client/linters/flake8'; -// import { PythonSettings } from '../../client/common/configSettings'; -// import { CancellationTokenSource, ConfigurationTarget, Uri, window, workspace } from 'vscode'; -// import { initialize, closeActiveWindows } from '../initialize'; -// import { MockOutputChannel } from '../mockClasses'; +import * as assert from 'assert'; +import * as path from 'path'; +import * as baseLinter from '../../client/linters/baseLinter'; +import * as pyLint from '../../client/linters/pylint'; +import * as flake8 from '../../client/linters/flake8'; +import { PythonSettings } from '../../client/common/configSettings'; +import { CancellationTokenSource, ConfigurationTarget, Uri, window, workspace } from 'vscode'; +import { initialize, closeActiveWindows } from '../initialize'; +import { MockOutputChannel } from '../mockClasses'; -// const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); +const multirootPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'multiRootWkspc'); -// suite('Multiroot Linting', () => { -// suiteSetup(async () => { -// await initialize(); -// PythonSettings.dispose(); -// }); -// suiteTeardown(() => closeActiveWindows()); -// teardown(async () => { -// await closeActiveWindows(); -// PythonSettings.dispose(); -// }); +suite('Multiroot Linting', () => { + suiteSetup(async () => { + await initialize(); + PythonSettings.dispose(); + }); + suiteTeardown(() => closeActiveWindows()); + teardown(async () => { + await closeActiveWindows(); + PythonSettings.dispose(); + }); -// async function testLinterInWorkspaceFolder(linter: baseLinter.BaseLinter, workspaceFolderRelativePath: string, mustHaveErrors: boolean) { -// const fileToLint = path.join(multirootPath, workspaceFolderRelativePath, 'file.py'); -// const cancelToken = new CancellationTokenSource(); -// const document = await workspace.openTextDocument(fileToLint); -// const editor = await window.showTextDocument(document); -// const messages = await linter.lint(editor.document, cancelToken.token); -// const errorMessage = mustHaveErrors ? 'No errors returned by linter' : 'Errors returned by linter'; -// assert.equal(messages.length > 0, mustHaveErrors, errorMessage); -// } -// async function enableDisableSetting(workspaceFolder, configTarget: ConfigurationTarget, setting: string, value: boolean) { -// const folderUri = Uri.file(workspaceFolder); -// const settings = workspace.getConfiguration('python.linting', folderUri); -// await settings.update(setting, value, configTarget); -// } + async function testLinterInWorkspaceFolder(linter: baseLinter.BaseLinter, workspaceFolderRelativePath: string, mustHaveErrors: boolean) { + const fileToLint = path.join(multirootPath, workspaceFolderRelativePath, 'file.py'); + const cancelToken = new CancellationTokenSource(); + const document = await workspace.openTextDocument(fileToLint); + const editor = await window.showTextDocument(document); + const messages = await linter.lint(editor.document, cancelToken.token); + const errorMessage = mustHaveErrors ? 'No errors returned by linter' : 'Errors returned by linter'; + assert.equal(messages.length > 0, mustHaveErrors, errorMessage); + } + async function enableDisableSetting(workspaceFolder, configTarget: ConfigurationTarget, setting: string, value: boolean) { + const folderUri = Uri.file(workspaceFolder); + const settings = workspace.getConfiguration('python.linting', folderUri); + await settings.update(setting, value, configTarget); + } -// test('Enabling Pylint in root and also in Workspace, should return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); -// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); -// }); + test('Enabling Pylint in root and also in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); + }); -// test('Enabling Pylint in root and disabling in Workspace, should not return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); -// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', false); -// }); + test('Enabling Pylint in root and disabling in Workspace, should not return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', false); + }); -// test('Disabling Pylint in root and enabling in Workspace, should return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', false); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); -// await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); -// }); + test('Disabling Pylint in root and enabling in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'pylintEnabled', false); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterInWorkspaceFolder(new pyLint.Linter(ch), 'workspace1', true); + }); -// test('Enabling Flake8 in root and also in Workspace, should return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); -// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); -// }); + test('Enabling Flake8 in root and also in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); + }); -// test('Enabling Flake8 in root and disabling in Workspace, should not return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', false); -// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', false); -// }); + test('Enabling Flake8 in root and disabling in Workspace, should not return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', true); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', false); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', false); + }); -// test('Disabling Flake8 in root and enabling in Workspace, should return errors', async () => { -// let ch = new MockOutputChannel('Lint'); -// await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', false); -// await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); -// await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); -// }); -// }); + test('Disabling Flake8 in root and enabling in Workspace, should return errors', async () => { + let ch = new MockOutputChannel('Lint'); + await enableDisableSetting(multirootPath, ConfigurationTarget.Workspace, 'flake8Enabled', false); + await enableDisableSetting(path.join(multirootPath, 'workspace1'), ConfigurationTarget.WorkspaceFolder, 'flake8Enabled', true); + await testLinterInWorkspaceFolder(new flake8.Linter(ch), 'workspace1', true); + }); +}); diff --git a/src/test/multiRootWkspc/disableLinters/file.py b/src/test/multiRootWkspc/disableLinters/file.py index 047ba0dc679e..439f899e9e22 100644 --- a/src/test/multiRootWkspc/disableLinters/file.py +++ b/src/test/multiRootWkspc/disableLinters/file.py @@ -84,4 +84,4 @@ def meth8(self): # pylint: disable=no-member # no error print self.bla - print self.blop \ No newline at end of file + print self.blop diff --git a/src/test/multiRootWkspc/multi.code-workspace b/src/test/multiRootWkspc/multi.code-workspace index bd948abca8a3..bdb471d241b5 100644 --- a/src/test/multiRootWkspc/multi.code-workspace +++ b/src/test/multiRootWkspc/multi.code-workspace @@ -15,8 +15,8 @@ "python.linting.mypyEnabled": true, "python.linting.pydocstyleEnabled": true, "python.linting.pylamaEnabled": true, - "python.linting.pylintEnabled": true, + "python.linting.pylintEnabled": false, "python.linting.pep8Enabled": true, "python.linting.prospectorEnabled": true } -} \ No newline at end of file +} diff --git a/src/test/multiRootWkspc/parent/child/file.py b/src/test/multiRootWkspc/parent/child/file.py index 047ba0dc679e..439f899e9e22 100644 --- a/src/test/multiRootWkspc/parent/child/file.py +++ b/src/test/multiRootWkspc/parent/child/file.py @@ -84,4 +84,4 @@ def meth8(self): # pylint: disable=no-member # no error print self.bla - print self.blop \ No newline at end of file + print self.blop diff --git a/src/test/multiRootWkspc/workspace1/.vscode/settings.json b/src/test/multiRootWkspc/workspace1/.vscode/settings.json index 6155e911580c..a783cfe01962 100644 --- a/src/test/multiRootWkspc/workspace1/.vscode/settings.json +++ b/src/test/multiRootWkspc/workspace1/.vscode/settings.json @@ -1,5 +1,5 @@ { - "python.linting.pylintEnabled": false, + "python.linting.pylintEnabled": true, "python.linting.enabled": false, "python.linting.flake8Enabled": true -} \ No newline at end of file +} diff --git a/src/test/multiRootWkspc/workspace1/file.py b/src/test/multiRootWkspc/workspace1/file.py index 047ba0dc679e..439f899e9e22 100644 --- a/src/test/multiRootWkspc/workspace1/file.py +++ b/src/test/multiRootWkspc/workspace1/file.py @@ -84,4 +84,4 @@ def meth8(self): # pylint: disable=no-member # no error print self.bla - print self.blop \ No newline at end of file + print self.blop From b4295efde2522fd4459b209fdafee6648ff1ef69 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 17:09:43 -0700 Subject: [PATCH 21/24] fix sys variables --- src/client/common/systemVariables.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/systemVariables.ts b/src/client/common/systemVariables.ts index e4adcaf67862..444d782d6eea 100644 --- a/src/client/common/systemVariables.ts +++ b/src/client/common/systemVariables.ts @@ -136,7 +136,7 @@ export class SystemVariables extends AbstractSystemVariables { constructor(workspaceRoot?: string) { super(); - this._workspaceRoot = typeof workspaceRoot === 'string' ? vscode.workspace.rootPath : __dirname; + this._workspaceRoot = typeof workspaceRoot === 'string' ? workspaceRoot : __dirname; this._workspaceRootFolderName = Path.basename(this._workspaceRoot); Object.keys(process.env).forEach(key => { this[`env:${key}`] = this[`env.${key}`] = process.env[key]; From 360e9198233a57784aee43cf306264ea08464d26 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 11 Oct 2017 17:19:33 -0700 Subject: [PATCH 22/24] added unit test to resolve ${workspaceRoot} in settings.json --- .gitignore | 1 + .../common/configSettings.multiroot.test.ts | 16 ++++ src/test/multiRootWkspc/multi.code-workspace | 6 ++ .../workspace2/.vscode/settings.json | 3 + src/test/multiRootWkspc/workspace2/file.py | 87 +++++++++++++++++++ .../workspace3/.vscode/settings.json | 3 + src/test/multiRootWkspc/workspace3/file.py | 87 +++++++++++++++++++ 7 files changed, 203 insertions(+) create mode 100644 src/test/multiRootWkspc/workspace2/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace2/file.py create mode 100644 src/test/multiRootWkspc/workspace3/.vscode/settings.json create mode 100644 src/test/multiRootWkspc/workspace3/file.py diff --git a/.gitignore b/.gitignore index cb3999278277..6c4d63cbb81a 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ node_modules *.pyc .vscode/.ropeproject/** src/test/.vscode/** +**/.vscode/tags **/testFiles/**/.cache/** *.noseids .vscode-test diff --git a/src/test/common/configSettings.multiroot.test.ts b/src/test/common/configSettings.multiroot.test.ts index b40b92ec6308..c7852babdb00 100644 --- a/src/test/common/configSettings.multiroot.test.ts +++ b/src/test/common/configSettings.multiroot.test.ts @@ -161,5 +161,21 @@ suite('Multiroot Config Settings', () => { assert.equal(cfg.linting.pylintEnabled, false, 'Pylint should not be enabled in workspace'); }); + test('${workspaceRoot} variable in settings should be replaced with the right value', async () => { + const workspace2Uri = Uri.file(path.join(multirootPath, 'workspace2')); + let fileToOpen = path.join(workspace2Uri.fsPath, 'file.py'); + let document = await workspace.openTextDocument(fileToOpen); + let cfg = PythonSettings.getInstance(document.uri); + assert.equal(path.dirname(cfg.workspaceSymbols.ctagsPath), workspace2Uri.fsPath, 'ctags file for workspace2 is incorrect'); + PythonSettings.dispose(); + + const workspace3Uri = Uri.file(path.join(multirootPath, 'workspace2')); + fileToOpen = path.join(workspace3Uri.fsPath, 'file.py'); + + document = await workspace.openTextDocument(fileToOpen); + cfg = PythonSettings.getInstance(document.uri); + assert.equal(path.dirname(cfg.workspaceSymbols.ctagsPath), workspace3Uri.fsPath, 'ctags file for workspace3 is incorrect'); + PythonSettings.dispose(); + }); }); diff --git a/src/test/multiRootWkspc/multi.code-workspace b/src/test/multiRootWkspc/multi.code-workspace index bdb471d241b5..67ea6996216d 100644 --- a/src/test/multiRootWkspc/multi.code-workspace +++ b/src/test/multiRootWkspc/multi.code-workspace @@ -3,6 +3,12 @@ { "path": "workspace1" }, + { + "path": "workspace2" + }, + { + "path": "workspace3" + }, { "path": "parent\\child" }, diff --git a/src/test/multiRootWkspc/workspace2/.vscode/settings.json b/src/test/multiRootWkspc/workspace2/.vscode/settings.json new file mode 100644 index 000000000000..1398df90f45b --- /dev/null +++ b/src/test/multiRootWkspc/workspace2/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "python.workspaceSymbols.ctagsPath": "${workspaceRoot}/workspace2.tags.file" +} diff --git a/src/test/multiRootWkspc/workspace2/file.py b/src/test/multiRootWkspc/workspace2/file.py new file mode 100644 index 000000000000..439f899e9e22 --- /dev/null +++ b/src/test/multiRootWkspc/workspace2/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print self + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print self\ + + "foo" + + def meth3(self): + """test one line disabling""" + # no error + print self.bla # pylint: disable=no-member + # error + print self.blop + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + print self.blop + # pylint: enable=no-member + # error + print self.blip + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + if self.blop: + # pylint: enable=no-member + # error + print self.blip + else: + # no error + print self.blip + # no error + print self.blip + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + try: + # pylint: enable=no-member + # error + print self.blip + except UndefinedName: # pylint: disable=undefined-variable + # no error + print self.blip + # no error + print self.blip + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print self.blip + else: + # error + print self.blip + # error + print self.blip + + + def meth8(self): + """test late disabling""" + # error + print self.blip + # pylint: disable=no-member + # no error + print self.bla + print self.blop diff --git a/src/test/multiRootWkspc/workspace3/.vscode/settings.json b/src/test/multiRootWkspc/workspace3/.vscode/settings.json new file mode 100644 index 000000000000..1398df90f45b --- /dev/null +++ b/src/test/multiRootWkspc/workspace3/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "python.workspaceSymbols.ctagsPath": "${workspaceRoot}/workspace2.tags.file" +} diff --git a/src/test/multiRootWkspc/workspace3/file.py b/src/test/multiRootWkspc/workspace3/file.py new file mode 100644 index 000000000000..439f899e9e22 --- /dev/null +++ b/src/test/multiRootWkspc/workspace3/file.py @@ -0,0 +1,87 @@ +"""pylint option block-disable""" + +__revision__ = None + +class Foo(object): + """block-disable test""" + + def __init__(self): + pass + + def meth1(self, arg): + """this issues a message""" + print self + + def meth2(self, arg): + """and this one not""" + # pylint: disable=unused-argument + print self\ + + "foo" + + def meth3(self): + """test one line disabling""" + # no error + print self.bla # pylint: disable=no-member + # error + print self.blop + + def meth4(self): + """test re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + print self.blop + # pylint: enable=no-member + # error + print self.blip + + def meth5(self): + """test IF sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + if self.blop: + # pylint: enable=no-member + # error + print self.blip + else: + # no error + print self.blip + # no error + print self.blip + + def meth6(self): + """test TRY/EXCEPT sub-block re-enabling""" + # pylint: disable=no-member + # no error + print self.bla + try: + # pylint: enable=no-member + # error + print self.blip + except UndefinedName: # pylint: disable=undefined-variable + # no error + print self.blip + # no error + print self.blip + + def meth7(self): + """test one line block opening disabling""" + if self.blop: # pylint: disable=no-member + # error + print self.blip + else: + # error + print self.blip + # error + print self.blip + + + def meth8(self): + """test late disabling""" + # error + print self.blip + # pylint: disable=no-member + # no error + print self.bla + print self.blop From b9a03601affb467bc63379566404ef8e5ff55c4f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 12 Oct 2017 11:47:11 -0700 Subject: [PATCH 23/24] fixed code review comments --- src/client/common/installer.ts | 4 ++ .../common/configSettings.multiroot.test.ts | 5 +- .../multiRootWkspc/disableLinters/file.py | 46 +++++++++---------- src/test/multiRootWkspc/parent/child/file.py | 46 +++++++++---------- src/test/multiRootWkspc/workspace1/file.py | 46 +++++++++---------- src/test/multiRootWkspc/workspace2/file.py | 46 +++++++++---------- src/test/multiRootWkspc/workspace3/file.py | 46 +++++++++---------- 7 files changed, 121 insertions(+), 118 deletions(-) diff --git a/src/client/common/installer.ts b/src/client/common/installer.ts index 8710273ee857..59e22331f743 100644 --- a/src/client/common/installer.ts +++ b/src/client/common/installer.ts @@ -1,3 +1,4 @@ +import { error } from './logger'; import * as vscode from 'vscode'; import * as settings from './configSettings'; import * as os from 'os'; @@ -223,6 +224,9 @@ export class Installer implements vscode.Disposable { features.push(productName); return pythonConfig.update('disablePromptForFeatures', features, true).then(() => InstallerResponse.Ignore); } + default: { + throw new Error('Invalid selection'); + } } } public async install(product: Product, resource?: Uri): Promise { diff --git a/src/test/common/configSettings.multiroot.test.ts b/src/test/common/configSettings.multiroot.test.ts index c7852babdb00..ca7d9cc68762 100644 --- a/src/test/common/configSettings.multiroot.test.ts +++ b/src/test/common/configSettings.multiroot.test.ts @@ -109,16 +109,15 @@ suite('Multiroot Config Settings', () => { test('Enabling/Disabling Pylint in root and workspace should be reflected in config settings', async () => { const workspaceUri = Uri.file(path.join(multirootPath, 'workspace1')); - const workspaceFolder = Uri.file(path.join(multirootPath, 'workspace1')); - await testLinterSetting(workspaceFolder, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', false); await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', true); let cfgSetting = PythonSettings.getInstance(workspaceUri); assert.equal(cfgSetting.linting.pylintEnabled, false, 'Workspace folder pylint setting is true when it should not be'); PythonSettings.dispose(); - await testLinterSetting(workspaceFolder, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); + await testLinterSetting(workspaceUri, ConfigurationTarget.WorkspaceFolder, 'pylintEnabled', true); await testLinterSetting(workspaceUri, ConfigurationTarget.Workspace, 'pylintEnabled', false); cfgSetting = PythonSettings.getInstance(workspaceUri); diff --git a/src/test/multiRootWkspc/disableLinters/file.py b/src/test/multiRootWkspc/disableLinters/file.py index 439f899e9e22..e02e34f6282e 100644 --- a/src/test/multiRootWkspc/disableLinters/file.py +++ b/src/test/multiRootWkspc/disableLinters/file.py @@ -10,78 +10,78 @@ def __init__(self): def meth1(self, arg): """this issues a message""" - print self + print(self) def meth2(self, arg): """and this one not""" # pylint: disable=unused-argument - print self\ - + "foo" + print (self\ + + "foo") def meth3(self): """test one line disabling""" # no error - print self.bla # pylint: disable=no-member + print (self.bla) # pylint: disable=no-member # error - print self.blop + print (self.blop) def meth4(self): """test re-enabling""" # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) # pylint: enable=no-member # error - print self.blip + print (self.blip) def meth5(self): """test IF sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla - if self.blop: + print (self.bla) + if (self.blop): # pylint: enable=no-member # error - print self.blip + print (self.blip) else: # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth6(self): """test TRY/EXCEPT sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla + print (self.bla) try: # pylint: enable=no-member # error - print self.blip + print (self.blip) except UndefinedName: # pylint: disable=undefined-variable # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth7(self): """test one line block opening disabling""" if self.blop: # pylint: disable=no-member # error - print self.blip + print (self.blip) else: # error - print self.blip + print (self.blip) # error - print self.blip + print (self.blip) def meth8(self): """test late disabling""" # error - print self.blip + print (self.blip) # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/parent/child/file.py b/src/test/multiRootWkspc/parent/child/file.py index 439f899e9e22..e02e34f6282e 100644 --- a/src/test/multiRootWkspc/parent/child/file.py +++ b/src/test/multiRootWkspc/parent/child/file.py @@ -10,78 +10,78 @@ def __init__(self): def meth1(self, arg): """this issues a message""" - print self + print(self) def meth2(self, arg): """and this one not""" # pylint: disable=unused-argument - print self\ - + "foo" + print (self\ + + "foo") def meth3(self): """test one line disabling""" # no error - print self.bla # pylint: disable=no-member + print (self.bla) # pylint: disable=no-member # error - print self.blop + print (self.blop) def meth4(self): """test re-enabling""" # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) # pylint: enable=no-member # error - print self.blip + print (self.blip) def meth5(self): """test IF sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla - if self.blop: + print (self.bla) + if (self.blop): # pylint: enable=no-member # error - print self.blip + print (self.blip) else: # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth6(self): """test TRY/EXCEPT sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla + print (self.bla) try: # pylint: enable=no-member # error - print self.blip + print (self.blip) except UndefinedName: # pylint: disable=undefined-variable # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth7(self): """test one line block opening disabling""" if self.blop: # pylint: disable=no-member # error - print self.blip + print (self.blip) else: # error - print self.blip + print (self.blip) # error - print self.blip + print (self.blip) def meth8(self): """test late disabling""" # error - print self.blip + print (self.blip) # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace1/file.py b/src/test/multiRootWkspc/workspace1/file.py index 439f899e9e22..e02e34f6282e 100644 --- a/src/test/multiRootWkspc/workspace1/file.py +++ b/src/test/multiRootWkspc/workspace1/file.py @@ -10,78 +10,78 @@ def __init__(self): def meth1(self, arg): """this issues a message""" - print self + print(self) def meth2(self, arg): """and this one not""" # pylint: disable=unused-argument - print self\ - + "foo" + print (self\ + + "foo") def meth3(self): """test one line disabling""" # no error - print self.bla # pylint: disable=no-member + print (self.bla) # pylint: disable=no-member # error - print self.blop + print (self.blop) def meth4(self): """test re-enabling""" # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) # pylint: enable=no-member # error - print self.blip + print (self.blip) def meth5(self): """test IF sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla - if self.blop: + print (self.bla) + if (self.blop): # pylint: enable=no-member # error - print self.blip + print (self.blip) else: # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth6(self): """test TRY/EXCEPT sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla + print (self.bla) try: # pylint: enable=no-member # error - print self.blip + print (self.blip) except UndefinedName: # pylint: disable=undefined-variable # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth7(self): """test one line block opening disabling""" if self.blop: # pylint: disable=no-member # error - print self.blip + print (self.blip) else: # error - print self.blip + print (self.blip) # error - print self.blip + print (self.blip) def meth8(self): """test late disabling""" # error - print self.blip + print (self.blip) # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace2/file.py b/src/test/multiRootWkspc/workspace2/file.py index 439f899e9e22..e02e34f6282e 100644 --- a/src/test/multiRootWkspc/workspace2/file.py +++ b/src/test/multiRootWkspc/workspace2/file.py @@ -10,78 +10,78 @@ def __init__(self): def meth1(self, arg): """this issues a message""" - print self + print(self) def meth2(self, arg): """and this one not""" # pylint: disable=unused-argument - print self\ - + "foo" + print (self\ + + "foo") def meth3(self): """test one line disabling""" # no error - print self.bla # pylint: disable=no-member + print (self.bla) # pylint: disable=no-member # error - print self.blop + print (self.blop) def meth4(self): """test re-enabling""" # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) # pylint: enable=no-member # error - print self.blip + print (self.blip) def meth5(self): """test IF sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla - if self.blop: + print (self.bla) + if (self.blop): # pylint: enable=no-member # error - print self.blip + print (self.blip) else: # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth6(self): """test TRY/EXCEPT sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla + print (self.bla) try: # pylint: enable=no-member # error - print self.blip + print (self.blip) except UndefinedName: # pylint: disable=undefined-variable # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth7(self): """test one line block opening disabling""" if self.blop: # pylint: disable=no-member # error - print self.blip + print (self.blip) else: # error - print self.blip + print (self.blip) # error - print self.blip + print (self.blip) def meth8(self): """test late disabling""" # error - print self.blip + print (self.blip) # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) diff --git a/src/test/multiRootWkspc/workspace3/file.py b/src/test/multiRootWkspc/workspace3/file.py index 439f899e9e22..e02e34f6282e 100644 --- a/src/test/multiRootWkspc/workspace3/file.py +++ b/src/test/multiRootWkspc/workspace3/file.py @@ -10,78 +10,78 @@ def __init__(self): def meth1(self, arg): """this issues a message""" - print self + print(self) def meth2(self, arg): """and this one not""" # pylint: disable=unused-argument - print self\ - + "foo" + print (self\ + + "foo") def meth3(self): """test one line disabling""" # no error - print self.bla # pylint: disable=no-member + print (self.bla) # pylint: disable=no-member # error - print self.blop + print (self.blop) def meth4(self): """test re-enabling""" # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) # pylint: enable=no-member # error - print self.blip + print (self.blip) def meth5(self): """test IF sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla - if self.blop: + print (self.bla) + if (self.blop): # pylint: enable=no-member # error - print self.blip + print (self.blip) else: # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth6(self): """test TRY/EXCEPT sub-block re-enabling""" # pylint: disable=no-member # no error - print self.bla + print (self.bla) try: # pylint: enable=no-member # error - print self.blip + print (self.blip) except UndefinedName: # pylint: disable=undefined-variable # no error - print self.blip + print (self.blip) # no error - print self.blip + print (self.blip) def meth7(self): """test one line block opening disabling""" if self.blop: # pylint: disable=no-member # error - print self.blip + print (self.blip) else: # error - print self.blip + print (self.blip) # error - print self.blip + print (self.blip) def meth8(self): """test late disabling""" # error - print self.blip + print (self.blip) # pylint: disable=no-member # no error - print self.bla - print self.blop + print (self.bla) + print (self.blop) From 023b66b8fba76a6b9b9db4bda72a79fddf1a9110 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 12 Oct 2017 13:29:55 -0700 Subject: [PATCH 24/24] fixed code review comments --- src/test/multiRootWkspc/disableLinters/file.py | 2 +- src/test/multiRootWkspc/parent/child/file.py | 2 +- src/test/multiRootWkspc/workspace1/file.py | 2 +- src/test/multiRootWkspc/workspace2/file.py | 2 +- src/test/multiRootWkspc/workspace3/file.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/multiRootWkspc/disableLinters/file.py b/src/test/multiRootWkspc/disableLinters/file.py index e02e34f6282e..27509dd2fcd6 100644 --- a/src/test/multiRootWkspc/disableLinters/file.py +++ b/src/test/multiRootWkspc/disableLinters/file.py @@ -40,7 +40,7 @@ def meth5(self): # pylint: disable=no-member # no error print (self.bla) - if (self.blop): + if self.blop: # pylint: enable=no-member # error print (self.blip) diff --git a/src/test/multiRootWkspc/parent/child/file.py b/src/test/multiRootWkspc/parent/child/file.py index e02e34f6282e..27509dd2fcd6 100644 --- a/src/test/multiRootWkspc/parent/child/file.py +++ b/src/test/multiRootWkspc/parent/child/file.py @@ -40,7 +40,7 @@ def meth5(self): # pylint: disable=no-member # no error print (self.bla) - if (self.blop): + if self.blop: # pylint: enable=no-member # error print (self.blip) diff --git a/src/test/multiRootWkspc/workspace1/file.py b/src/test/multiRootWkspc/workspace1/file.py index e02e34f6282e..27509dd2fcd6 100644 --- a/src/test/multiRootWkspc/workspace1/file.py +++ b/src/test/multiRootWkspc/workspace1/file.py @@ -40,7 +40,7 @@ def meth5(self): # pylint: disable=no-member # no error print (self.bla) - if (self.blop): + if self.blop: # pylint: enable=no-member # error print (self.blip) diff --git a/src/test/multiRootWkspc/workspace2/file.py b/src/test/multiRootWkspc/workspace2/file.py index e02e34f6282e..27509dd2fcd6 100644 --- a/src/test/multiRootWkspc/workspace2/file.py +++ b/src/test/multiRootWkspc/workspace2/file.py @@ -40,7 +40,7 @@ def meth5(self): # pylint: disable=no-member # no error print (self.bla) - if (self.blop): + if self.blop: # pylint: enable=no-member # error print (self.blip) diff --git a/src/test/multiRootWkspc/workspace3/file.py b/src/test/multiRootWkspc/workspace3/file.py index e02e34f6282e..27509dd2fcd6 100644 --- a/src/test/multiRootWkspc/workspace3/file.py +++ b/src/test/multiRootWkspc/workspace3/file.py @@ -40,7 +40,7 @@ def meth5(self): # pylint: disable=no-member # no error print (self.bla) - if (self.blop): + if self.blop: # pylint: enable=no-member # error print (self.blip)