Skip to content

Fix how we pass the log directory to Editor Services #4933

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

Merged
merged 1 commit into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 1 addition & 4 deletions src/features/DebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,6 @@ export const DebugConfigurations: Record<DebugConfig, DebugConfiguration> = {

export class DebugSessionFeature extends LanguageClientConsumer
implements DebugConfigurationProvider, DebugAdapterDescriptorFactory {

private sessionCount = 1;
private tempDebugProcess: PowerShellProcess | undefined;
private tempSessionDetails: IEditorServicesSessionDetails | undefined;
private commands: Disposable[] = [];
Expand Down Expand Up @@ -392,8 +390,7 @@ export class DebugSessionFeature extends LanguageClientConsumer
this.tempDebugProcess = await this.sessionManager.createDebugSessionProcess(settings);
// TODO: Maybe set a timeout on the cancellation token?
const cancellationTokenSource = new CancellationTokenSource();
this.tempSessionDetails = await this.tempDebugProcess.start(
`DebugSession-${this.sessionCount++}`, cancellationTokenSource.token);
this.tempSessionDetails = await this.tempDebugProcess.start(cancellationTokenSource.token);

// NOTE: Dotnet attach debugging is only currently supported if a temporary debug terminal is used, otherwise we get lots of lock conflicts from loading the assemblies.
if (session.configuration.attachDotnetDebugger) {
Expand Down
13 changes: 4 additions & 9 deletions src/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export enum LogLevel {
* This will allow for easy mocking of the logger during unit tests.
*/
export interface ILogger {
getLogFilePath(baseName: string): vscode.Uri;
logDirectoryPath: vscode.Uri;
updateLogLevel(logLevelName: string): void;
write(message: string, ...additionalMessages: string[]): void;
writeAndShowInformation(message: string, ...additionalMessages: string[]): Promise<void>;
Expand All @@ -35,12 +35,11 @@ export interface ILogger {
}

export class Logger implements ILogger {
public logDirectoryPath: vscode.Uri;

public logDirectoryPath: vscode.Uri; // The folder for all the logs
private logLevel: LogLevel;
private commands: vscode.Disposable[];
private logChannel: vscode.OutputChannel;
private logFilePath: vscode.Uri;
private logFilePath: vscode.Uri; // The client's logs
private logDirectoryCreated = false;
private writingLog = false;

Expand All @@ -53,7 +52,7 @@ export class Logger implements ILogger {
globalStorageUri.with({ scheme: "file" }),
"logs",
`${Math.floor(Date.now() / 1000)}-${vscode.env.sessionId}`);
this.logFilePath = this.getLogFilePath("vscode-powershell");
this.logFilePath = vscode.Uri.joinPath(this.logDirectoryPath, "vscode-powershell.log");

// Early logging of the log paths for debugging.
if (LogLevel.Diagnostic >= this.logLevel) {
Expand All @@ -79,10 +78,6 @@ export class Logger implements ILogger {
}
}

public getLogFilePath(baseName: string): vscode.Uri {
return vscode.Uri.joinPath(this.logDirectoryPath, `${baseName}.log`);
}

private writeAtLevel(logLevel: LogLevel, message: string, ...additionalMessages: string[]): void {
if (logLevel >= this.logLevel) {
void this.writeLine(message, logLevel);
Expand Down
6 changes: 2 additions & 4 deletions src/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ export class PowerShellProcess {
this.onExited = this.onExitedEmitter.event;
}

public async start(logFileName: string, cancellationToken: vscode.CancellationToken): Promise<IEditorServicesSessionDetails | undefined> {
const editorServicesLogPath = this.logger.getLogFilePath(logFileName);

public async start(cancellationToken: vscode.CancellationToken): Promise<IEditorServicesSessionDetails | undefined> {
const psesModulePath =
path.resolve(
__dirname,
Expand All @@ -50,7 +48,7 @@ export class PowerShellProcess {
: "";

this.startPsesArgs +=
`-LogPath '${utils.escapeSingleQuotes(editorServicesLogPath.fsPath)}' ` +
`-LogPath '${utils.escapeSingleQuotes(this.logger.logDirectoryPath.fsPath)}' ` +
`-SessionDetailsPath '${utils.escapeSingleQuotes(this.sessionFilePath.fsPath)}' ` +
`-FeatureFlags @(${featureFlags}) `;

Expand Down
2 changes: 1 addition & 1 deletion src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ export class SessionManager implements Middleware {
}
});

this.sessionDetails = await languageServerProcess.start("EditorServices", cancellationToken);
this.sessionDetails = await languageServerProcess.start(cancellationToken);

return languageServerProcess;
}
Expand Down
4 changes: 1 addition & 3 deletions test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ const packageJSON: any = require(path.resolve(rootPath, "package.json"));
export const extensionId = `${packageJSON.publisher}.${packageJSON.name}`;

export class TestLogger implements ILogger {
getLogFilePath(_baseName: string): vscode.Uri {
return vscode.Uri.file("");
}
logDirectoryPath: vscode.Uri = vscode.Uri.file("");
updateLogLevel(_logLevelName: string): void {
return;
}
Expand Down