mirror of
https://github.com/ethanolivertroy/obsidian-markitdown.git
synced 2026-07-22 05:41:54 +00:00
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
This commit is contained in:
parent
25513c6cb0
commit
a93ed8341d
3 changed files with 120 additions and 21 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -4,13 +4,32 @@ import { PluginArgsEditor } from './PluginArgsEditor';
|
|||
|
||||
export class SettingsTab extends PluginSettingTab {
|
||||
plugin: MarkitdownPlugin;
|
||||
private pythonPathDebounceTimer: ReturnType<typeof setTimeout> | 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 ──────────────────────────
|
||||
|
|
|
|||
|
|
@ -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<string>();
|
||||
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<boolean> {
|
||||
if (!pythonPath.trim()) return false;
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const scriptPath = getPythonScriptPath('install_package.py', pluginDir);
|
||||
const child = spawn(pythonPath, [scriptPath, '--package', packageSpec], {
|
||||
|
|
|
|||
Loading…
Reference in a new issue