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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

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

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)

A sister PR for PSES (everything coming across the LSP currently shows up as info) is forthcoming but is not urgently necessary to merge this PR.

@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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants