From bfc54b3dad5a7f1a9784458d5fd3184b17d11717 Mon Sep 17 00:00:00 2001 From: Ricardo Henriques Date: Wed, 12 Nov 2025 11:49:58 +0000 Subject: [PATCH 1/2] fix: Address code review feedback - improve platform compatibility and error handling - Fix Windows command compatibility by detecting platform before running shell commands - Fix Homebrew detection to avoid false positives (use /opt/homebrew/ instead of /opt/) - Add support for pre-release version parsing (e.g., 1.8.8-beta1) - Add 10-second timeout to PyPI network requests - Replace console.error with VS Code output channel for better logging - Improve error visibility for users during troubleshooting Addresses feedback from PR #4 code review --- src/utils/installDetector.ts | 27 ++++++++++++++++++++++----- src/utils/versionChecker.ts | 36 +++++++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/utils/installDetector.ts b/src/utils/installDetector.ts index 9680afc..e64de98 100644 --- a/src/utils/installDetector.ts +++ b/src/utils/installDetector.ts @@ -7,9 +7,20 @@ import { exec } from 'child_process'; import { promisify } from 'util'; +import * as vscode from 'vscode'; const execAsync = promisify(exec); +// Output channel for logging +let outputChannel: vscode.OutputChannel | null = null; + +function getOutputChannel(): vscode.OutputChannel { + if (!outputChannel) { + outputChannel = vscode.window.createOutputChannel('Rxiv-Maker'); + } + return outputChannel; +} + export type InstallMethod = 'homebrew' | 'pipx' | 'uv' | 'pip-user' | 'pip' | 'dev' | 'unknown'; /** @@ -20,7 +31,10 @@ export type InstallMethod = 'homebrew' | 'pipx' | 'uv' | 'pip-user' | 'pip' | 'd export async function detectInstallMethod(): Promise { try { // Get the path to rxiv executable - const { stdout: whichOutput } = await execAsync('which rxiv 2>/dev/null || where rxiv 2>nul'); + // Use platform-specific command to avoid shell compatibility issues + const isWindows = process.platform === 'win32'; + const cmd = isWindows ? 'where rxiv 2>nul' : 'which rxiv 2>/dev/null'; + const { stdout: whichOutput } = await execAsync(cmd); const executablePath = whichOutput.trim(); if (!executablePath) { @@ -37,8 +51,8 @@ export async function detectInstallMethod(): Promise { for (const prefix of homebrewPrefixes) { if (executablePath.startsWith(prefix)) { - // Additional verification: check if Cellar path exists - if (executablePath.includes('/Cellar/') || executablePath.includes('/opt/')) { + // Additional verification: check if Cellar or opt/homebrew path exists + if (executablePath.includes('/Cellar/') || executablePath.includes('/opt/homebrew/')) { return 'homebrew'; } } @@ -80,7 +94,8 @@ export async function detectInstallMethod(): Promise { return 'unknown'; } catch (error) { - console.error('Error detecting install method:', error); + const output = getOutputChannel(); + output.appendLine(`Error detecting install method: ${error}`); return 'unknown'; } } @@ -130,7 +145,9 @@ export function getFriendlyInstallName(installMethod: InstallMethod): string { */ export async function isRxivMakerInstalled(): Promise { try { - const { stdout } = await execAsync('rxiv --version 2>/dev/null || rxiv.exe --version 2>nul'); + const isWindows = process.platform === 'win32'; + const cmd = isWindows ? 'rxiv.exe --version 2>nul' : 'rxiv --version 2>/dev/null'; + const { stdout } = await execAsync(cmd); return stdout.trim().length > 0; } catch (error) { return false; diff --git a/src/utils/versionChecker.ts b/src/utils/versionChecker.ts index c31ab23..b68a5f2 100644 --- a/src/utils/versionChecker.ts +++ b/src/utils/versionChecker.ts @@ -12,6 +12,16 @@ import { detectInstallMethod, type InstallMethod } from './installDetector'; const execAsync = promisify(exec); +// Output channel for logging +let outputChannel: vscode.OutputChannel | null = null; + +function getOutputChannel(): vscode.OutputChannel { + if (!outputChannel) { + outputChannel = vscode.window.createOutputChannel('Rxiv-Maker'); + } + return outputChannel; +} + interface VersionInfo { current: string | null; latest: string | null; @@ -32,9 +42,12 @@ interface CacheData { */ export async function getRxivMakerVersion(): Promise { try { - const { stdout } = await execAsync('rxiv --version 2>/dev/null || rxiv.exe --version 2>nul'); - // Parse output like "rxiv-maker version 1.8.8" - const match = stdout.trim().match(/(\d+\.\d+\.\d+)/); + const isWindows = process.platform === 'win32'; + const cmd = isWindows ? 'rxiv.exe --version 2>nul' : 'rxiv --version 2>/dev/null'; + const { stdout } = await execAsync(cmd); + // Parse output like "rxiv-maker version 1.8.8" or "rxiv-maker version 1.8.8-beta1" + // Support semantic versioning with optional pre-release identifiers + const match = stdout.trim().match(/(\d+\.\d+\.\d+(?:-[a-zA-Z0-9.]+)?)/); return match ? match[1] : null; } catch (error) { return null; @@ -81,8 +94,9 @@ export async function checkHomebrewOutdated(): Promise<{ current: string; latest export async function fetchLatestVersion(): Promise { return new Promise((resolve) => { const url = 'https://pypi.org/pypi/rxiv-maker/json'; + const output = getOutputChannel(); - https.get(url, (res) => { + const req = https.get(url, { timeout: 10000 }, (res) => { let data = ''; res.on('data', (chunk) => { @@ -94,12 +108,20 @@ export async function fetchLatestVersion(): Promise { const json = JSON.parse(data); resolve(json.info.version); } catch (error) { - console.error('Error parsing PyPI response:', error); + output.appendLine(`Error parsing PyPI response: ${error}`); resolve(null); } }); - }).on('error', (error) => { - console.error('Error fetching latest version:', error); + }); + + req.on('timeout', () => { + output.appendLine('PyPI request timed out after 10 seconds'); + req.destroy(); + resolve(null); + }); + + req.on('error', (error) => { + output.appendLine(`Error fetching latest version from PyPI: ${error}`); resolve(null); }); }); From 10af0823f31ef006d2a7cf945db66b5bef23d9b7 Mon Sep 17 00:00:00 2001 From: Ricardo Henriques Date: Wed, 12 Nov 2025 11:55:39 +0000 Subject: [PATCH 2/2] fix: Fix command name bug and add command validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix incorrect command name: 'rxiv-maker.install' → 'rxiv-maker.installRxivMaker' - Add command validation before execution to prevent errors - Add COMMANDS constant object to avoid typos in command names - Show helpful error message if install command is not available This addresses the command validation feedback from PR #4 code review and fixes a critical bug where the wrong command name was being used. --- src/commands/upgradeRxivMaker.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/commands/upgradeRxivMaker.ts b/src/commands/upgradeRxivMaker.ts index fc67f5f..7999666 100644 --- a/src/commands/upgradeRxivMaker.ts +++ b/src/commands/upgradeRxivMaker.ts @@ -13,6 +13,12 @@ import { } from '../utils/installDetector'; import { getRxivMakerVersion, forceCheckForUpdates } from '../utils/versionChecker'; +// Command name constants to avoid typos +const COMMANDS = { + INSTALL: 'rxiv-maker.installRxivMaker', + UPGRADE: 'rxiv-maker.upgrade' +} as const; + /** * Upgrade rxiv-maker to the latest version. * @@ -30,8 +36,13 @@ export async function upgradeRxivMakerCommand(context: vscode.ExtensionContext): ); if (install === 'Install') { - // Trigger the install command - vscode.commands.executeCommand('rxiv-maker.install'); + // Verify command exists before executing + const commands = await vscode.commands.getCommands(); + if (commands.includes(COMMANDS.INSTALL)) { + vscode.commands.executeCommand(COMMANDS.INSTALL); + } else { + vscode.window.showErrorMessage('Install command not available. Please reinstall the extension.'); + } } return; }