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

feat: Logger util #36

Merged
merged 6 commits into from
Jul 15, 2024
Merged

feat: Logger util #36

merged 6 commits into from
Jul 15, 2024

Conversation

bmingles
Copy link
Collaborator

@bmingles bmingles commented Jul 12, 2024

  • Util that can be used by any module to log scoped messages. Handlers can be registered globally.
  • Registered additional "Deephaven Debug" output channel to capture Logger logs (I considered just logging to the existing panel, but I didn't want to clutter it with things more diagnostics in nature).
  • Added a "Download Logs" button to error toasts. Also added to cmd palette

Testing

  • Produce an error
  • Should now see a "Download Logs" button on the error toast
  • Clicking should prompt to save and open the file in vscode after save
  • This should show the same content as the "Deephaven Debug" output panel

resolves #33

@bmingles bmingles requested a review from mofojed July 12, 2024 23:16
src/util/Logger.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/util/Logger.ts Outdated Show resolved Hide resolved
@bmingles bmingles requested a review from mofojed July 15, 2024 17:58
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Minor cleanup

info: (label, ...args) => console.info(`[${label}] INFO:`, ...args),
debug: (label, ...args) => console.debug(`[${label}] DEBUG:`, ...args),
debug2: (label, ...args) => console.debug(`[${label}] DEBUG2:`, ...args),
error: console.error.bind(console),
Copy link
Member

Choose a reason for hiding this comment

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

Now the label's not being printed with the square brackets. In the private handle method below, wrap the this.label in square brackets so it's printed correctly.

Comment on lines 10 to 22
showErrorMessage = async (message: string) => {
const response = await vscode.window.showErrorMessage(
message,
DOWNLOAD_LOGS_TEXT
);

// If user clicks "Download Logs" button
if (response === DOWNLOAD_LOGS_TEXT) {
await vscode.commands.executeCommand(DOWNLOAD_LOGS_CMD);
}
};

showInfoMessage = async (message: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably just name these messages error and info respectively, similar to our Logger class. Up to you though.

) {
this.name = outputChannel.name;

// Have to bind this explicilty since function overloads prevent using
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Have to bind this explicilty since function overloads prevent using
// Have to bind this explicitly since function overloads prevent using

@bmingles bmingles requested a review from mofojed July 15, 2024 18:29
@bmingles bmingles merged commit dbd14f7 into main Jul 15, 2024
1 check passed
@bmingles bmingles deleted the 33-error-logging branch July 15, 2024 18:48
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.

Expose error logs for support purposes
2 participants