Skip to content

Commit edbd57a

Browse files
DonJayamanneDonJayamanne
authored
Ensure configuration settings are retrieved using the ConfigurationService (#3780)
* Store python path in the `settings.json` file of the workspace folder * Do not use getInstance in config settings * Fix tests * Fixed tests * Fix lintger
1 parent 8256843 commit edbd57a

37 files changed

Lines changed: 635 additions & 579 deletions

‎src/client/common/configSettingMonitor.ts‎

Lines changed: 0 additions & 87 deletions
This file was deleted.

‎src/client/common/variables/environmentVariablesProvider.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@
33

44
import { inject, injectable } from 'inversify';
55
import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode';
6-
import { PythonSettings } from '../configSettings';
7-
import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../platform/constants';
8-
import { ICurrentProcess, IDisposableRegistry, IsWindows } from '../types';
6+
import { IPlatformService } from '../platform/types';
7+
import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types';
98
import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types';
109

1110
@injectable()
@@ -15,7 +14,9 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
1514
private disposables: Disposable[] = [];
1615
private changeEventEmitter: EventEmitter<Uri | undefined>;
1716
constructor(@inject(IEnvironmentVariablesService) private envVarsService: IEnvironmentVariablesService,
18-
@inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IsWindows) private isWidows: boolean,
17+
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
18+
@inject(IPlatformService) private platformService: IPlatformService,
19+
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
1920
@inject(ICurrentProcess) private process: ICurrentProcess) {
2021
disposableRegistry.push(this);
2122
this.changeEventEmitter = new EventEmitter();
@@ -32,7 +33,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
3233
});
3334
}
3435
public async getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
35-
const settings = PythonSettings.getInstance(resource);
36+
const settings = this.configurationService.getSettings(resource);
3637
if (!this.cache.has(settings.envFile)) {
3738
const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
3839
this.createFileWatcher(settings.envFile, workspaceFolderUri);
@@ -41,7 +42,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
4142
mergedVars = {};
4243
}
4344
this.envVarsService.mergeVariables(this.process.env, mergedVars!);
44-
const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
45+
const pathVariable = this.platformService.pathVariableName;
4546
const pathValue = this.process.env[pathVariable];
4647
if (pathValue) {
4748
this.envVarsService.appendPath(mergedVars!, pathValue);

‎src/client/interpreter/configuration/interpreterSelector.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import { inject, injectable } from 'inversify';
22
import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri } from 'vscode';
33
import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
4-
import * as settings from '../../common/configSettings';
54
import { Commands } from '../../common/constants';
6-
import { IPathUtils } from '../../common/types';
5+
import { IConfigurationService, IPathUtils } from '../../common/types';
76
import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts';
87
import { IInterpreterComparer, IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';
98

@@ -23,6 +22,7 @@ export class InterpreterSelector implements IInterpreterSelector {
2322
@inject(IInterpreterComparer) private readonly interpreterComparer: IInterpreterComparer,
2423
@inject(IPythonPathUpdaterServiceManager) private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
2524
@inject(IShebangCodeLensProvider) private readonly shebangCodeLensProvider: IShebangCodeLensProvider,
25+
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
2626
@inject(ICommandManager) private readonly commandManager: ICommandManager) {
2727
}
2828
public dispose() {
@@ -64,7 +64,7 @@ export class InterpreterSelector implements IInterpreterSelector {
6464
}
6565

6666
const suggestions = await this.getSuggestions(wkspace);
67-
const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, wkspace ? wkspace.fsPath : undefined);
67+
const currentPythonPath = this.pathUtils.getDisplayName(this.configurationService.getSettings(wkspace).pythonPath, wkspace ? wkspace.fsPath : undefined);
6868
const quickPickOptions: QuickPickOptions = {
6969
matchOnDetail: true,
7070
matchOnDescription: true,

‎src/client/interpreter/display/shebangCodeLensProvider.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
import { inject, injectable } from 'inversify';
2-
import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri, workspace } from 'vscode';
3-
import * as settings from '../../common/configSettings';
4-
import { IS_WINDOWS } from '../../common/platform/constants';
2+
import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri } from 'vscode';
3+
import { IWorkspaceService } from '../../common/application/types';
4+
import { IPlatformService } from '../../common/platform/types';
55
import { IProcessServiceFactory } from '../../common/process/types';
6-
import { IServiceContainer } from '../../ioc/types';
6+
import { IConfigurationService } from '../../common/types';
77
import { IShebangCodeLensProvider } from '../contracts';
88

99
@injectable()
1010
export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
11-
// tslint:disable-next-line:no-any
12-
public onDidChangeCodeLenses: Event<void> = workspace.onDidChangeConfiguration as any as Event<void>;
13-
private readonly processServiceFactory: IProcessServiceFactory;
14-
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
15-
this.processServiceFactory = serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
11+
public readonly onDidChangeCodeLenses: Event<void>;
12+
constructor(@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
13+
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
14+
@inject(IPlatformService) private readonly platformService: IPlatformService,
15+
@inject(IWorkspaceService) workspaceService: IWorkspaceService) {
16+
// tslint:disable-next-line:no-any
17+
this.onDidChangeCodeLenses = workspaceService.onDidChangeConfiguration as any as Event<void>;
18+
1619
}
1720
public async detectShebang(document: TextDocument): Promise<string | undefined> {
1821
const firstLine = document.lineAt(0);
@@ -28,14 +31,13 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
2831
const pythonPath = await this.getFullyQualifiedPathToInterpreter(shebang, document.uri);
2932
return typeof pythonPath === 'string' && pythonPath.length > 0 ? pythonPath : undefined;
3033
}
31-
public async provideCodeLenses(document: TextDocument, token: CancellationToken): Promise<CodeLens[]> {
32-
const codeLenses = await this.createShebangCodeLens(document);
33-
return Promise.resolve(codeLenses);
34+
public async provideCodeLenses(document: TextDocument, _token?: CancellationToken): Promise<CodeLens[]> {
35+
return this.createShebangCodeLens(document);
3436
}
3537
private async getFullyQualifiedPathToInterpreter(pythonPath: string, resource: Uri) {
3638
let cmdFile = pythonPath;
3739
let args = ['-c', 'import sys;print(sys.executable)'];
38-
if (pythonPath.indexOf('bin/env ') >= 0 && !IS_WINDOWS) {
40+
if (pythonPath.indexOf('bin/env ') >= 0 && !this.platformService.isWindows) {
3941
// In case we have pythonPath as '/usr/bin/env python'.
4042
const parts = pythonPath.split(' ').map(part => part.trim()).filter(part => part.length > 0);
4143
cmdFile = parts.shift()!;
@@ -48,12 +50,10 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
4850
}
4951
private async createShebangCodeLens(document: TextDocument) {
5052
const shebang = await this.detectShebang(document);
51-
const pythonPath = settings.PythonSettings.getInstance(document.uri).pythonPath;
52-
const resolvedPythonPath = await this.getFullyQualifiedPathToInterpreter(pythonPath, document.uri);
53-
if (!shebang || shebang === resolvedPythonPath) {
53+
const pythonPath = this.configurationService.getSettings(document.uri).pythonPath;
54+
if (!shebang || shebang === pythonPath) {
5455
return [];
5556
}
56-
5757
const firstLine = document.lineAt(0);
5858
const startOfShebang = new Position(0, 0);
5959
const endOfShebang = new Position(0, firstLine.text.length - 1);

‎src/client/providers/definitionProvider.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as proxy from './jediProxy';
88

99
export class PythonDefinitionProvider implements vscode.DefinitionProvider {
1010
public constructor(private jediFactory: JediFactory) { }
11-
private static parseData(data: proxy.IDefinitionResult, possibleWord: string): vscode.Definition {
11+
private static parseData(data: proxy.IDefinitionResult, possibleWord: string): vscode.Definition | undefined {
1212
if (data && Array.isArray(data.definitions) && data.definitions.length > 0) {
1313
const definitions = data.definitions.filter(d => d.text === possibleWord);
1414
const definition = definitions.length > 0 ? definitions[0] : data.definitions[data.definitions.length - 1];
@@ -18,19 +18,21 @@ export class PythonDefinitionProvider implements vscode.DefinitionProvider {
1818
definition.range.endLine, definition.range.endColumn);
1919
return new vscode.Location(definitionResource, range);
2020
}
21-
return null;
2221
}
2322
@captureTelemetry(DEFINITION)
24-
public provideDefinition(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Thenable<vscode.Definition> {
23+
public async provideDefinition(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<vscode.Definition | undefined> {
2524
const filename = document.fileName;
2625
if (document.lineAt(position.line).text.match(/^\s*\/\//)) {
27-
return Promise.resolve(null);
26+
return;
2827
}
2928
if (position.character <= 0) {
30-
return Promise.resolve(null);
29+
return;
3130
}
3231

3332
const range = document.getWordRangeAtPosition(position);
33+
if (!range) {
34+
return;
35+
}
3436
const columnIndex = range.isEmpty ? position.character : range.end.character;
3537
const cmd: proxy.ICommand<proxy.IDefinitionResult> = {
3638
command: proxy.CommandType.Definitions,
@@ -42,8 +44,7 @@ export class PythonDefinitionProvider implements vscode.DefinitionProvider {
4244
cmd.source = document.getText();
4345
}
4446
const possibleWord = document.getText(range);
45-
return this.jediFactory.getJediProxyHandler<proxy.IDefinitionResult>(document.uri).sendCommand(cmd, token).then(data => {
46-
return PythonDefinitionProvider.parseData(data, possibleWord);
47-
});
47+
const data = await this.jediFactory.getJediProxyHandler<proxy.IDefinitionResult>(document.uri).sendCommand(cmd, token);
48+
return data ? PythonDefinitionProvider.parseData(data, possibleWord) : undefined;
4849
}
4950
}

‎src/client/providers/jediProxy.ts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@ import {
1010
CancellationToken, CancellationTokenSource, CompletionItemKind,
1111
Disposable, SymbolKind, Uri
1212
} from 'vscode';
13-
import { PythonSettings } from '../common/configSettings';
1413
import { isTestExecution } from '../common/constants';
1514
import '../common/extensions';
1615
import { IS_WINDOWS } from '../common/platform/constants';
1716
import { IPythonExecutionFactory } from '../common/process/types';
18-
import { BANNER_NAME_PROPOSE_LS, ILogger, IPythonExtensionBanner } from '../common/types';
17+
import { BANNER_NAME_PROPOSE_LS, IConfigurationService, ILogger, IPythonExtensionBanner, IPythonSettings } from '../common/types';
1918
import { createDeferred, Deferred } from '../common/utils/async';
2019
import { debounce, swallowExceptions } from '../common/utils/decorators';
2120
import { StopWatch } from '../common/utils/stopWatch';
2221
import { IEnvironmentVariablesProvider } from '../common/variables/types';
22+
import { IInterpreterService } from '../interpreter/contracts';
2323
import { IServiceContainer } from '../ioc/types';
2424
import { Logger } from './../common/logger';
2525

@@ -134,7 +134,7 @@ commandNames.set(CommandType.Symbols, 'names');
134134

135135
export class JediProxy implements Disposable {
136136
private proc?: ChildProcess;
137-
private pythonSettings: PythonSettings;
137+
private pythonSettings: IPythonSettings;
138138
private cmdId: number = 0;
139139
private lastKnownPythonInterpreter: string;
140140
private previousData = '';
@@ -152,13 +152,17 @@ export class JediProxy implements Disposable {
152152
private lastCmdIdProcessed?: number;
153153
private lastCmdIdProcessedForPidUsage?: number;
154154
private proposeNewLanguageServerPopup: IPythonExtensionBanner;
155+
private readonly disposables: Disposable[] = [];
155156

156157
public constructor(private extensionRootDir: string, workspacePath: string, private serviceContainer: IServiceContainer) {
157158
this.workspacePath = workspacePath;
158-
this.pythonSettings = PythonSettings.getInstance(Uri.file(workspacePath));
159+
const configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
160+
this.pythonSettings = configurationService.getSettings(Uri.file(workspacePath));
159161
this.lastKnownPythonInterpreter = this.pythonSettings.pythonPath;
160162
this.logger = serviceContainer.get<ILogger>(ILogger);
161-
this.pythonSettings.on('change', () => this.pythonSettingsChangeHandler());
163+
const interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
164+
const disposable = interpreterService.onDidChangeInterpreter(this.onDidChangeInterpreter.bind(this));
165+
this.disposables.push(disposable);
162166
this.initialized = createDeferred<void>();
163167
this.startLanguageServer().then(() => this.initialized.resolve()).ignoreErrors();
164168

@@ -172,6 +176,12 @@ export class JediProxy implements Disposable {
172176
}
173177

174178
public dispose() {
179+
while (this.disposables.length > 0) {
180+
const disposable = this.disposables.pop();
181+
if (disposable) {
182+
disposable.dispose();
183+
}
184+
}
175185
this.killProcess();
176186
}
177187

@@ -277,7 +287,7 @@ export class JediProxy implements Disposable {
277287
}
278288

279289
@swallowExceptions('JediProxy')
280-
private async pythonSettingsChangeHandler() {
290+
private async onDidChangeInterpreter() {
281291
if (this.lastKnownPythonInterpreter === this.pythonSettings.pythonPath) {
282292
return;
283293
}

0 commit comments

Comments
 (0)