Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

19767 remove di in resolvers #20048

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@ export async function validateManagePy(
return error;
}
const resolvedPath = resolveVariables(selected, undefined, folder);

if (selected !== defaultValue && !(await fs.pathExists(resolvedPath))) {
return error;
}
if (!resolvedPath.trim().toLowerCase().endsWith('.py')) {
return error;
if (resolvedPath) {
if (selected !== defaultValue && !(await fs.pathExists(resolvedPath))) {
return error;
}
if (!resolvedPath.trim().toLowerCase().endsWith('.py')) {
return error;
}
}
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,13 @@ export async function validateIniPath(
return error;
}
const resolvedPath = resolveVariables(selected, undefined, folder);
if (selected !== defaultValue && !fs.pathExists(resolvedPath)) {
return error;
}
if (!resolvedPath.trim().toLowerCase().endsWith('.ini')) {
return error;
if (resolvedPath) {
if (selected !== defaultValue && !fs.pathExists(resolvedPath)) {
return error;
}
if (!resolvedPath.trim().toLowerCase().endsWith('.ini')) {
return error;
}
}
}

Expand Down
21 changes: 4 additions & 17 deletions src/client/debugger/extension/configuration/resolvers/attach.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,14 @@

'use strict';

import { inject, injectable } from 'inversify';
import { injectable } from 'inversify';
import { CancellationToken, Uri, WorkspaceFolder } from 'vscode';
import { IDocumentManager, IWorkspaceService } from '../../../../common/application/types';
import { IPlatformService } from '../../../../common/platform/types';
import { IConfigurationService } from '../../../../common/types';
import { IInterpreterService } from '../../../../interpreter/contracts';
import { AttachRequestArguments, DebugOptions, PathMapping } from '../../../types';
import { getOSType, OSType } from '../utils/platform';
import { BaseConfigurationResolver } from './base';

@injectable()
export class AttachConfigurationResolver extends BaseConfigurationResolver<AttachRequestArguments> {
constructor(
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
@inject(IDocumentManager) documentManager: IDocumentManager,
@inject(IPlatformService) platformService: IPlatformService,
@inject(IConfigurationService) configurationService: IConfigurationService,
@inject(IInterpreterService) interpreterService: IInterpreterService,
) {
super(workspaceService, documentManager, platformService, configurationService, interpreterService);
}

public async resolveDebugConfigurationWithSubstitutedVariables(
folder: WorkspaceFolder | undefined,
debugConfiguration: AttachRequestArguments,
Expand Down Expand Up @@ -87,10 +74,10 @@ export class AttachConfigurationResolver extends BaseConfigurationResolver<Attac
// We'll need paths to be fixed only in the case where local and remote hosts are the same
// I.e. only if hostName === 'localhost' or '127.0.0.1' or ''
const isLocalHost = this.isLocalHost(debugConfiguration.host);
if (this.platformService.isWindows && isLocalHost) {
if (getOSType() == OSType.Windows && isLocalHost) {
this.debugOption(debugOptions, DebugOptions.FixFilePathCase);
}
if (this.platformService.isWindows) {
if (getOSType() == OSType.Windows) {
this.debugOption(debugOptions, DebugOptions.WindowsClient);
} else {
this.debugOption(debugOptions, DebugOptions.UnixClient);
Expand Down
67 changes: 34 additions & 33 deletions src/client/debugger/extension/configuration/resolvers/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,25 @@
import { injectable } from 'inversify';
import * as path from 'path';
import { CancellationToken, DebugConfiguration, Uri, WorkspaceFolder } from 'vscode';
import { IDocumentManager, IWorkspaceService } from '../../../../common/application/types';
import { PYTHON_LANGUAGE } from '../../../../common/constants';
import { IPlatformService } from '../../../../common/platform/types';
import { IConfigurationService } from '../../../../common/types';
import { SystemVariables } from '../../../../common/variables/systemVariables';
import { IInterpreterService } from '../../../../interpreter/contracts';
import { sendTelemetryEvent } from '../../../../telemetry';
import { EventName } from '../../../../telemetry/constants';
import { DebuggerTelemetry } from '../../../../telemetry/types';
import { AttachRequestArguments, DebugOptions, LaunchRequestArguments, PathMapping } from '../../../types';
import { PythonPathSource } from '../../types';
import { IDebugConfigurationResolver } from '../types';
import { getActiveTextEditor, resolveVariables } from '../utils/common';
import { getOSType, OSType } from '../utils/platform';
import { getWorkspaceFolder as getVSCodeWorkspaceFolder, getWorkspaceFolders } from '../utils/workspaceFolder';

@injectable()
export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
implements IDebugConfigurationResolver<T> {
protected pythonPathSource: PythonPathSource = PythonPathSource.launchJson;

constructor(
protected readonly workspaceService: IWorkspaceService,
protected readonly documentManager: IDocumentManager,
protected readonly platformService: IPlatformService,
protected readonly configurationService: IConfigurationService,
protected readonly interpreterService: IInterpreterService,
) {}
Expand Down Expand Up @@ -59,27 +56,26 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
return folder.uri;
}
const program = this.getProgram();
if (
!Array.isArray(this.workspaceService.workspaceFolders) ||
this.workspaceService.workspaceFolders.length === 0
) {
let workspaceFolders = getWorkspaceFolders();

if (!Array.isArray(workspaceFolders) || workspaceFolders.length === 0) {
return program ? Uri.file(path.dirname(program)) : undefined;
}
if (this.workspaceService.workspaceFolders.length === 1) {
return this.workspaceService.workspaceFolders[0].uri;
if (workspaceFolders.length === 1) {
return workspaceFolders[0].uri;
}
if (program) {
const workspaceFolder = this.workspaceService.getWorkspaceFolder(Uri.file(program));
const workspaceFolder = getVSCodeWorkspaceFolder(Uri.file(program));
if (workspaceFolder) {
return workspaceFolder.uri;
}
}
}

protected getProgram(): string | undefined {
const editor = this.documentManager.activeTextEditor;
if (editor && editor.document.languageId === PYTHON_LANGUAGE) {
return editor.document.fileName;
const activeTextEditor = getActiveTextEditor();
if (activeTextEditor && activeTextEditor.document.languageId === PYTHON_LANGUAGE) {
return activeTextEditor.document.fileName;
}
}

Expand All @@ -99,11 +95,11 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
return;
}
if (debugConfiguration.envFile && (workspaceFolder || debugConfiguration.cwd)) {
const systemVariables = new SystemVariables(
undefined,
debugConfiguration.envFile = resolveVariables(
debugConfiguration.envFile,
(workspaceFolder ? workspaceFolder.fsPath : undefined) || debugConfiguration.cwd,
undefined,
);
debugConfiguration.envFile = systemVariables.resolveAny(debugConfiguration.envFile);
}
}

Expand All @@ -114,25 +110,28 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
if (!debugConfiguration) {
return;
}
const systemVariables: SystemVariables = new SystemVariables(
undefined,
workspaceFolder?.fsPath,
this.workspaceService,
);
if (debugConfiguration.pythonPath === '${command:python.interpreterPath}' || !debugConfiguration.pythonPath) {
const interpreterPath =
(await this.interpreterService.getActiveInterpreter(workspaceFolder))?.path ??
this.configurationService.getSettings(workspaceFolder).pythonPath;
debugConfiguration.pythonPath = interpreterPath;
} else {
debugConfiguration.pythonPath = systemVariables.resolveAny(debugConfiguration.pythonPath);
debugConfiguration.pythonPath = resolveVariables(
debugConfiguration.pythonPath ? debugConfiguration.pythonPath : undefined,
workspaceFolder?.fsPath,
undefined,
);
}
if (debugConfiguration.python === '${command:python.interpreterPath}' || !debugConfiguration.python) {
this.pythonPathSource = PythonPathSource.settingsJson;
} else {
this.pythonPathSource = PythonPathSource.launchJson;
}
debugConfiguration.python = systemVariables.resolveAny(debugConfiguration.python);
debugConfiguration.python = resolveVariables(
debugConfiguration.python ? debugConfiguration.python : undefined,
workspaceFolder?.fsPath,
undefined,
);
}

protected debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions) {
Expand Down Expand Up @@ -168,17 +167,19 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
];
} else {
// Expand ${workspaceFolder} variable first if necessary.
const systemVariables = new SystemVariables(undefined, defaultLocalRoot);
pathMappings = pathMappings.map(({ localRoot: mappedLocalRoot, remoteRoot }) => ({
localRoot: systemVariables.resolveAny(mappedLocalRoot),
// TODO: Apply to remoteRoot too?
remoteRoot,
}));
pathMappings = pathMappings.map(({ localRoot: mappedLocalRoot, remoteRoot }) => {
let resolvedLocalRoot = resolveVariables(mappedLocalRoot, defaultLocalRoot, undefined);
return {
localRoot: resolvedLocalRoot ? resolvedLocalRoot : '',
// TODO: Apply to remoteRoot too?
remoteRoot,
};
});
}

// If on Windows, lowercase the drive letter for path mappings.
// TODO: Apply even if no localRoot?
if (this.platformService.isWindows) {
if (getOSType() == OSType.Windows) {
// TODO: Apply to remoteRoot too?
pathMappings = pathMappings.map(({ localRoot: windowsLocalRoot, remoteRoot }) => {
let localRoot = windowsLocalRoot;
Expand Down
10 changes: 3 additions & 7 deletions src/client/debugger/extension/configuration/resolvers/launch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,25 @@ import { inject, injectable, named } from 'inversify';
import { CancellationToken, Uri, WorkspaceFolder } from 'vscode';
import { InvalidPythonPathInDebuggerServiceId } from '../../../../application/diagnostics/checks/invalidPythonPathInDebugger';
import { IDiagnosticsService, IInvalidPythonPathInDebuggerService } from '../../../../application/diagnostics/types';
import { IDocumentManager, IWorkspaceService } from '../../../../common/application/types';
import { IPlatformService } from '../../../../common/platform/types';
import { IConfigurationService } from '../../../../common/types';
import { IInterpreterService } from '../../../../interpreter/contracts';
import { DebuggerTypeName } from '../../../constants';
import { DebugOptions, DebugPurpose, LaunchRequestArguments } from '../../../types';
import { getOSType, OSType } from '../utils/platform';
import { BaseConfigurationResolver } from './base';
import { IDebugEnvironmentVariablesService } from './helper';

@injectable()
export class LaunchConfigurationResolver extends BaseConfigurationResolver<LaunchRequestArguments> {
constructor(
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
@inject(IDocumentManager) documentManager: IDocumentManager,
@inject(IDiagnosticsService)
@named(InvalidPythonPathInDebuggerServiceId)
private readonly invalidPythonPathInDebuggerService: IInvalidPythonPathInDebuggerService,
@inject(IPlatformService) platformService: IPlatformService,
@inject(IConfigurationService) configurationService: IConfigurationService,
@inject(IDebugEnvironmentVariablesService) private readonly debugEnvHelper: IDebugEnvironmentVariablesService,
@inject(IInterpreterService) interpreterService: IInterpreterService,
) {
super(workspaceService, documentManager, platformService, configurationService, interpreterService);
super(configurationService, interpreterService);
}

public async resolveDebugConfiguration(
Expand Down Expand Up @@ -153,7 +149,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
if (debugConfiguration.subProcess === true) {
this.debugOption(debugOptions, DebugOptions.SubProcess);
}
if (this.platformService.isWindows) {
if (getOSType() == OSType.Windows) {
this.debugOption(debugOptions, DebugOptions.FixFilePathCase);
}
const isFastAPI = this.isDebuggingFastAPI(debugConfiguration);
Expand Down
36 changes: 22 additions & 14 deletions src/client/debugger/extension/configuration/utils/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

'use strict';

import { WorkspaceFolder } from 'vscode';
import { WorkspaceFolder, window, TextEditor } from 'vscode';
import { getWorkspaceFolder } from './workspaceFolder';

/**
Expand All @@ -21,20 +21,28 @@ function isString(str: any): str is string {
}

export function resolveVariables(
value: string,
value: string | undefined,
rootFolder: string | undefined,
folder: WorkspaceFolder | undefined,
): string {
const workspace = folder ? getWorkspaceFolder(folder.uri) : undefined;
const variablesObject: { [key: string]: any } = {};
variablesObject.workspaceFolder = workspace ? workspace.uri.fsPath : rootFolder;
): string | undefined {
if (value) {
const workspace = folder ? getWorkspaceFolder(folder.uri) : undefined;
const variablesObject: { [key: string]: any } = {};
variablesObject.workspaceFolder = workspace ? workspace.uri.fsPath : rootFolder;

const regexp = /\$\{(.*?)\}/g;
return value.replace(regexp, (match: string, name: string) => {
const newValue = variablesObject[name];
if (isString(newValue)) {
return newValue;
}
return match && (match.indexOf('env.') > 0 || match.indexOf('env:') > 0) ? '' : match;
});
const regexp = /\$\{(.*?)\}/g;
return value.replace(regexp, (match: string, name: string) => {
const newValue = variablesObject[name];
if (isString(newValue)) {
return newValue;
}
return match && (match.indexOf('env.') > 0 || match.indexOf('env:') > 0) ? '' : match;
});
}
return value;
}

export function getActiveTextEditor(): TextEditor | undefined {
const { activeTextEditor } = window;
return activeTextEditor;
}
19 changes: 19 additions & 0 deletions src/client/debugger/extension/configuration/utils/platform.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export enum OSType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already defined in src\client\common\utils\platform.ts

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I am trying to separate the utils from the debugger so that when we move it to the other extension we only have the functions that we use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have to pull a lot of things from commons anyway. Separating it like this now might not help.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh okay, im going to use the one in common.

Unknown = 'Unknown',
Windows = 'Windows',
OSX = 'OSX',
Linux = 'Linux',
}

export function getOSType(platform: string = process.platform): OSType {
if (/^win/.test(platform)) {
return OSType.Windows;
}
if (/^darwin/.test(platform)) {
return OSType.OSX;
}
if (/^linux/.test(platform)) {
return OSType.Linux;
}
return OSType.Unknown;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ import * as vscode from 'vscode';
export function getWorkspaceFolder(uri: vscode.Uri): vscode.WorkspaceFolder | undefined {
return vscode.workspace.getWorkspaceFolder(uri);
}

export function getWorkspaceFolders(): readonly vscode.WorkspaceFolder[] | undefined {
return vscode.workspace.workspaceFolders;
}
Loading