From a93ed8341d593354b6a8c591e6dec77c71aa6943 Mon Sep 17 00:00:00 2001 From: Ethan Troy Date: Tue, 17 Mar 2026 20:11:42 -0400 Subject: [PATCH] Fix Python detection on macOS/Windows and settings UX issues (#13) - Debounce Python path input (1.5s) to stop firing on every keystroke - Guard against empty paths crashing spawn() - Cancel debounce timer on settings tab hide/re-render - Fire pending refreshDependencies() on hide so status stays current - Add platform-specific Python fallback paths (Homebrew, Framework, Windows AppData) - Prefer the first Python where markitdown actually imports over stale metadata - Verify actual MarkItDown import in check_install.py, not just package metadata - Add ALLOWED_PACKAGES allowlist to check_install.py - Remove APPDATA path traversal in favor of LOCALAPPDATA --- python/check_install.py | 15 +++++- src/settings/SettingsTab.ts | 34 ++++++++++++-- src/utils/python.ts | 92 ++++++++++++++++++++++++++++++------- 3 files changed, 120 insertions(+), 21 deletions(-) diff --git a/python/check_install.py b/python/check_install.py index 29267ff..0a66099 100644 --- a/python/check_install.py +++ b/python/check_install.py @@ -14,8 +14,21 @@ import json import sys +ALLOWED_PACKAGES = {"markitdown"} + + def check_package(name: str) -> dict: - """Check if a Python package is installed and return its version.""" + """Check if a Python package is installed, importable, and return its version.""" + if name not in ALLOWED_PACKAGES: + return {"installed": False, "version": None} + + # Verify the package can actually be imported (not just metadata) + if name == "markitdown": + try: + from markitdown import MarkItDown # noqa: F401 + except (ImportError, AttributeError, TypeError): + return {"installed": False, "version": None} + try: from importlib.metadata import version, PackageNotFoundError try: diff --git a/src/settings/SettingsTab.ts b/src/settings/SettingsTab.ts index f3ba53a..7cc2a42 100644 --- a/src/settings/SettingsTab.ts +++ b/src/settings/SettingsTab.ts @@ -4,13 +4,32 @@ import { PluginArgsEditor } from './PluginArgsEditor'; export class SettingsTab extends PluginSettingTab { plugin: MarkitdownPlugin; + private pythonPathDebounceTimer: ReturnType | null = null; constructor(app: App, plugin: MarkitdownPlugin) { super(app, plugin); this.plugin = plugin; } + hide(): void { + if (this.pythonPathDebounceTimer) { + clearTimeout(this.pythonPathDebounceTimer); + this.pythonPathDebounceTimer = null; + // Settings were already saved on keystroke; fire the dependency + // refresh so status is current when the user reopens settings. + this.plugin.refreshDependencies().catch(console.error); + } + } + + private cancelPythonPathDebounce(): void { + if (this.pythonPathDebounceTimer) { + clearTimeout(this.pythonPathDebounceTimer); + this.pythonPathDebounceTimer = null; + } + } + display(): void { + this.cancelPythonPathDebounce(); const { containerEl } = this; containerEl.empty(); @@ -21,15 +40,24 @@ export class SettingsTab extends PluginSettingTab { new Setting(containerEl) .setName('Python path') - .setDesc('Path to Python executable (e.g., python, python3, or a full path)') + .setDesc('Path to Python executable (e.g., python, python3, or a full path like C:\\Python311\\python.exe)') .addText(text => text .setPlaceholder('python') .setValue(this.plugin.settings.pythonPath) .onChange(async (value) => { this.plugin.settings.pythonPath = value; await this.plugin.saveSettings(); - await this.plugin.refreshDependencies(); - this.display(); + // Debounce: wait for user to stop typing before checking + if (this.pythonPathDebounceTimer) { + clearTimeout(this.pythonPathDebounceTimer); + } + this.pythonPathDebounceTimer = setTimeout(async () => { + this.pythonPathDebounceTimer = null; + await this.plugin.refreshDependencies(); + if (this.containerEl.isConnected) { + this.display(); + } + }, 1500); })); // ── Conversion ────────────────────────── diff --git a/src/utils/python.ts b/src/utils/python.ts index e258105..e084c11 100644 --- a/src/utils/python.ts +++ b/src/utils/python.ts @@ -62,9 +62,6 @@ export function getPythonScriptPath(scriptName: string, pluginDir: string): stri return path.join(pluginDir, 'python', scriptName); } -/** - * Check Python installation and package versions using check_install.py. - */ /** * Check Python installation and package versions using check_install.py. * Returns the status and the resolved python path (which may differ from @@ -81,38 +78,97 @@ export async function checkDependencies( markitdownVersion: null, }; - const scriptPath = getPythonScriptPath('check_install.py', pluginDir); - - // Try configured path first, then python3 fallback - const pathsToTry = [pythonPath]; - if (pythonPath === 'python') { - pathsToTry.push('python3'); + // Guard against empty or whitespace-only paths + const trimmed = pythonPath.trim(); + if (!trimmed) { + return { status, resolvedPythonPath: pythonPath }; } - for (const tryPath of pathsToTry) { + const scriptPath = getPythonScriptPath('check_install.py', pluginDir); + + // Build candidate list: configured path first, then platform-specific fallbacks. + // GUI apps (Obsidian/Electron) often don't inherit the user's shell PATH, + // so we try well-known Python locations to find one with markitdown installed. + const pathsToTry = [trimmed]; + if (trimmed === 'python' || trimmed === 'python3') { + // Generic name — add common platform-specific locations + const isWin = process.platform === 'win32'; + if (isWin) { + // Windows: try python, python3, and common install locations + if (trimmed === 'python') pathsToTry.push('python3'); + const localAppData = process.env.LOCALAPPDATA || ''; + if (localAppData) { + // Standard Python installer locations + for (const ver of ['313', '312', '311', '310', '39']) { + pathsToTry.push(`${localAppData}\\Programs\\Python\\Python${ver}\\python.exe`); + } + // Microsoft Store Python + pathsToTry.push(`${localAppData}\\Microsoft\\WindowsApps\\python3.exe`); + } + } else { + // macOS / Linux: try python3, python, and common framework/brew/system locations + if (trimmed === 'python') pathsToTry.push('python3'); + else pathsToTry.push('python'); + // Homebrew (Apple Silicon + Intel) + pathsToTry.push('/opt/homebrew/bin/python3'); + pathsToTry.push('/usr/local/bin/python3'); + // Framework installs (python.org macOS installer) + for (const ver of ['3.13', '3.12', '3.11', '3.10', '3.9']) { + pathsToTry.push(`/Library/Frameworks/Python.framework/Versions/${ver}/bin/python3`); + } + // Linux common paths + pathsToTry.push('/usr/bin/python3'); + } + } + + // Deduplicate while preserving order + const seen = new Set(); + const uniquePaths = pathsToTry.filter(p => { + if (seen.has(p)) return false; + seen.add(p); + return true; + }); + + // Try each candidate; prefer the first one that has markitdown installed. + // If none has markitdown, fall back to the first working Python. + let firstWorkingPython: { status: DependencyStatus; resolvedPythonPath: string } | null = null; + + for (const tryPath of uniquePaths) { try { const result = await runPythonScript(tryPath, scriptPath, ['--check', 'all']); if (result.exitCode === 0 && result.stdout) { const data = JSON.parse(result.stdout); - status.pythonInstalled = true; - status.pythonVersion = data.python_version ?? null; + const depStatus: DependencyStatus = { + pythonInstalled: true, + pythonVersion: data.python_version ?? null, + markitdownInstalled: false, + markitdownVersion: null, + }; if (data.packages?.markitdown) { - status.markitdownInstalled = data.packages.markitdown.installed; - status.markitdownVersion = data.packages.markitdown.version ?? null; + depStatus.markitdownInstalled = data.packages.markitdown.installed; + depStatus.markitdownVersion = data.packages.markitdown.version ?? null; } - return { status, resolvedPythonPath: tryPath }; + // If this Python has markitdown, use it immediately + if (depStatus.markitdownInstalled) { + return { status: depStatus, resolvedPythonPath: tryPath }; + } + + // Otherwise, remember the first working Python as a fallback + if (!firstWorkingPython) { + firstWorkingPython = { status: depStatus, resolvedPythonPath: tryPath }; + } } } catch (err) { - // This path didn't work — log for debugging, try next console.debug(`markitdown: ${tryPath} failed:`, err); continue; } } - return { status, resolvedPythonPath: pythonPath }; + // No Python with markitdown found — return the first working Python (or the default) + return firstWorkingPython ?? { status, resolvedPythonPath: pythonPath }; } /** @@ -124,6 +180,8 @@ export async function installPackage( packageSpec: string, onProgress?: (line: string) => void ): Promise { + if (!pythonPath.trim()) return false; + return new Promise((resolve, reject) => { const scriptPath = getPythonScriptPath('install_package.py', pluginDir); const child = spawn(pythonPath, [scriptPath, '--package', packageSpec], {