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

Implement LogOutputWindow for Logging #5065

Merged
merged 22 commits into from
Nov 15, 2024
Merged

Conversation

JustinGrote
Copy link
Collaborator

@JustinGrote JustinGrote commented Oct 18, 2024

Implements the LogOutputWindow vscode API for PowerShell extension and PSES logs received via LSP.

Recording.2024-10-18.170017.mp4

There will now be 3 separate log output panels:
PowerShell Messages logged for the PowerShell client and messages sent via the LSP adapter (prepended with [PSES])
PowerShell: Trace LSP Traces the LSP messages back and forth. Omnisharp messages will show up as traces here
PowerShell: Trace DAP Traces the DAP messages back and forth (already implemented)

This has several benefits:

  1. Output to file is handled by vscode, we don't need our own custom handler anymore
  2. Log levels and switching log levels handled by vscode, our code related to this (settings etc.) can be removed
  3. Logs can be tailed and opened in separate editors within vscode, providing a very nice experience. Logs are visible using the log syntax highlighting which can be enhanced by my Crisp Logs Highlighter
  4. Centralizes to vscode extension log folder that will be used with Issue Reporter, there's a contribution API for issue reporter where we can gather additional data as well (PR forthcoming)

Log windows do not appear unless enabled.

Right now the Log Verbosity with "Set Log Level" only controls the client-side logging level, but I hope in a future PR to be able to update the logging level of PSES dynamically. Has a minor LSP performance benefit but difficult to implement due to MEL not having dynamic log level change support natively, but I think I found a way with IOptionsMonitor to do it.

Should be merged with PowerShell/PowerShellEditorServices#2200

Fixes #5079

@JustinGrote JustinGrote self-assigned this Oct 18, 2024
@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Oct 19, 2024

@andyleejordan I think you'll like this new utility function too for taking action on a config change:

/**
* Invokes the specified action when a PowerShell setting changes
* @param setting a string representation of the setting you wish to evaluate
* @param action the action to take when the setting changes
* @param scope the scope in which the vscode setting should be evaluated. Defaults to
* @param workspace
* @param onDidChangeConfiguration
* @example onPowerShellSettingChange("settingName", (newValue) => console.log(newValue));
* @returns a Disposable object that can be used to stop listening for changes
*/
// Because we actually do use the constraint in the callback
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters
export function onPowerShellSettingChange<T>(
setting: string,
listener: (newValue: T | undefined) => void,
section: "powershell",
scope?: vscode.ConfigurationScope,
): vscode.Disposable {
return vscode.workspace.onDidChangeConfiguration(e => {
if (!e.affectsConfiguration(section, scope)) { return; }
const value = vscode.workspace.getConfiguration(section, scope).get<T>(setting);
listener(value);
});
}

EDIT: I see a flaw here, will fix :)

@JustinGrote JustinGrote force-pushed the feature/logImprovements branch from 9471227 to 69e3209 Compare October 29, 2024 17:28
@JustinGrote
Copy link
Collaborator Author

JustinGrote commented Oct 29, 2024

Currently pending microsoft/vscode-languageserver-node#1116 otherwise all of our log messages surface as "trace" with a header which is not ideal.

@andyleejordan
Copy link
Member

This is going to be so nice though.

@JustinGrote
Copy link
Collaborator Author

No longer pending that, I wrote a new trace handler :).

Currently debating if I want to add realtime logging changes based on changing the LSP setting to this PR or save that for another one.

@JustinGrote JustinGrote marked this pull request as ready for review November 15, 2024 07:29
@JustinGrote JustinGrote requested a review from a team as a code owner November 15, 2024 07:29
Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Love it, yay.

src/extension.ts Show resolved Hide resolved
}
}

private registerCommands(): void {
this.registeredCommands = [
vscode.commands.registerCommand("PowerShell.RestartSession", async () => { await this.restartSession(); }),
vscode.commands.registerCommand(this.ShowSessionMenuCommandName, async () => { await this.showSessionMenu(); }),
vscode.workspace.onDidChangeConfiguration(async () => { await this.onConfigurationUpdated(); }),
vscode.workspace.onDidChangeConfiguration((e) => this.restartOnCriticalConfigChange(e)),
Copy link
Member

Choose a reason for hiding this comment

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

Ok nice that there's more information now.

@andyleejordan andyleejordan force-pushed the feature/logImprovements branch from 6d6f23c to 79954e2 Compare November 15, 2024 21:53
@andyleejordan andyleejordan enabled auto-merge (squash) November 15, 2024 21:54
@andyleejordan andyleejordan added the Issue-Enhancement A feature request (enhancement). label Nov 15, 2024
@andyleejordan andyleejordan merged commit 400fd75 into main Nov 15, 2024
7 checks passed
@andyleejordan andyleejordan deleted the feature/logImprovements branch November 15, 2024 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement A feature request (enhancement).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vscode-powershell logs rotation
2 participants