Skip to content

🧹 Refactor logging to use OutputChannel in extension.ts #47

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.

Sign up for GitHub

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

Open
nur-srijan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🧹 Refactor logging to use OutputChannel in extension.ts #47

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 31 additions & 13 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,25 @@ declare const global: { browserInstance?: puppeteer.Browser };
// Reusable browser instance for PDF exports
let browserInstance: puppeteer.Browser | null = null;

// Output channel for logging
let outputChannel: vscode.OutputChannel | null = null;

function log(message: string, isError: boolean = false): void {
if (!outputChannel) {
return;
}
const prefix = isError ? '[ERROR]' : '[INFO]';
outputChannel.appendLine(`${prefix} ${message}`);
}
Comment on lines +21 to +27
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

The current log function only supports info and error levels via a boolean flag. This loses the distinction of warnings (e.g., console.warn was used previously). To improve logging clarity and maintainability, I suggest introducing an enum for log levels. This allows for more granular logging (e.g., INFO, WARN, ERROR) and makes the log output easier to parse and filter. I'll provide suggestions for updating the call sites in subsequent comments.

enum LogLevel {
    Info = 'INFO',
    Warn = 'WARN',
    Error = 'ERROR',
}

function log(message: string, level: LogLevel = LogLevel.Info): void {
    if (!outputChannel) {
        return;
    }
    outputChannel.appendLine(`[${level}] ${message}`);
}


// Function to get or create browser instance
async function getBrowserInstance(): Promise<puppeteer.Browser> {
if (!browserInstance || !await isBrowserAvailable(browserInstance)) {
if (browserInstance) {
try {
await browserInstance.close();
} catch (error) {
console.error('Error closing browser instance:', error);
log(`Error closing browser instance: ${error instanceof Error ? error.message : String(error)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

                log(`Error closing browser instance: ${error instanceof Error ? error.message : String(error)}`, LogLevel.Error);

}
}
browserInstance = await launchPuppeteerWithFallbacks();
Expand Down Expand Up @@ -157,17 +168,20 @@ async function cleanupBrowser(): Promise<void> {
await browserInstance.close();
browserInstance = null;
} catch (error) {
console.error('Error cleaning up browser instance:', error);
log(`Error cleaning up browser instance: ${error instanceof Error ? error.message : String(error)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

            log(`Error cleaning up browser instance: ${error instanceof Error ? error.message : String(error)}`, LogLevel.Error);

}
}
}

// Helpers moved to `src/helpers.ts` to allow unit tests without `vscode` runtime

export function activate(context: vscode.ExtensionContext) {
console.log('Markdown Rich Preview & Export: activate() started');
outputChannel = vscode.window.createOutputChannel('Markdown Rich Preview & Export');
context.subscriptions.push(outputChannel);

log('activate() started');
try {
console.log('Markdown Rich Preview & Export: Registering commands...');
log('Registering commands...');
const disposable = vscode.commands.registerCommand('markdown-rich-preview.showPreview', () => {
const editor = vscode.window.activeTextEditor;

Expand Down Expand Up @@ -249,7 +263,7 @@ export function activate(context: vscode.ExtensionContext) {

// Register the export to PDF command
const exportToPdfCommand = vscode.commands.registerCommand('markdown-rich-preview.exportToPdf', async () => {
console.log('Markdown Rich Preview & Export: exportToPdf command triggered');
log('exportToPdf command triggered');
const editor = vscode.window.activeTextEditor;

if (!editor || editor.document.languageId !== 'markdown') {
Expand Down Expand Up @@ -320,7 +334,7 @@ export function activate(context: vscode.ExtensionContext) {
vscode.window.showInformationMessage(`Successfully exported PDF to ${path.basename(uri.fsPath)}`);
} catch (error) {
vscode.window.showErrorMessage(`Failed to export PDF: ${error instanceof Error ? error.message : String(error)}`);
console.error('PDF Export Error:', error);
log(`PDF Export Error: ${error instanceof Error ? error.message : String(error)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

                            log(`PDF Export Error: ${error instanceof Error ? error.message : String(error)}`, LogLevel.Error);

throw error; // Re-throw to ensure the progress indicator shows the error
} finally {
// Clean up temp file
Expand All @@ -330,26 +344,26 @@ export function activate(context: vscode.ExtensionContext) {
fs.unlinkSync(tempHtmlPath);
}
} catch (e) {
console.warn('Failed to delete temp HTML file:', e);
log(`Failed to delete temp HTML file: ${e instanceof Error ? e.message : String(e)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

This was previously a console.warn. Failing to delete a temporary file is not a critical error that stops the extension, so it's better to log it as a warning instead of an error using the new LogLevel.Warn.

                                    log(`Failed to delete temp HTML file: ${e instanceof Error ? e.message : String(e)}`, LogLevel.Warn);

}
}

if (page && !page.isClosed()) {
await page.close().catch(console.error);
await page.close().catch(err => log(`Error closing page: ${err instanceof Error ? err.message : String(err)}`, true));
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

                                await page.close().catch(err => log(`Error closing page: ${err instanceof Error ? err.message : String(err)}`, LogLevel.Error));

}
}
});
} catch (error) {
console.error('Error in PDF export:', error);
log(`Error in PDF export: ${error instanceof Error ? error.message : String(error)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

                    log(`Error in PDF export: ${error instanceof Error ? error.message : String(error)}`, LogLevel.Error);

// Don't close the browser here as we want to reuse it
}
}
});

context.subscriptions.push(exportToPdfCommand);
console.log('Markdown Rich Preview & Export: activate() completed successfully');
log('activate() completed successfully');
} catch (error) {
console.error('Markdown Rich Preview & Export: activate() failed:', error);
log(`activate() failed: ${error instanceof Error ? error.message : String(error)}`, true);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

With the proposed change to the log function, this call should be updated to use the LogLevel enum for error logging.

        log(`activate() failed: ${error instanceof Error ? error.message : String(error)}`, LogLevel.Error);

vscode.window.showErrorMessage(`Failed to activate Markdown Rich Preview & Export: ${error instanceof Error ? error.message : String(error)}`);
}
}
Expand Down Expand Up @@ -385,6 +399,10 @@ function updateContent(panel: vscode.WebviewPanel, document: vscode.TextDocument



export function deactivate() {
void cleanupBrowser();
export async function deactivate(): Promise<void> {
await cleanupBrowser();
if (outputChannel) {
outputChannel.dispose();
outputChannel = null;
}
}
Loading