Repository navigation
fix(cli): doctor shows platform-correct install hints - #319
Conversation
The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg` for any non-macOS platform — Windows users would see an apt command that doesn't exist on their system, and Red Hat / Arch users got the wrong package manager too. `getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is already used by render.ts) and handles darwin / linux / win32 correctly. Use it here too. Also rewrite checkFFprobe: - it previously used `which ffprobe` which is not available on Windows (cmd uses `where`), so on Windows the check always reported "Not found" even when ffprobe was on PATH - run `ffprobe -version` directly instead, which works cross-platform whenever ffprobe is resolvable on PATH, and surfaces the version string in the same style as the FFmpeg check
jrusso1020
left a comment
There was a problem hiding this comment.
Thanks for the clean fix and the thorough write-up! The before/after examples and the scoping note made this really easy to review.
Approving as-is. This is a strict improvement: Windows users go from a wrong hint + broken ffprobe check to correct hints + a working check, and reusing getFFmpegInstallHint() is exactly the right move.
Good call leaving findFFmpeg() out — the which issue there has a wider blast radius (render.ts depends on it too) and deserves its own PR. I'll open follow-up issues for that one, plus one for making the Linux install hint distro-aware (the helper still returns sudo apt install ffmpeg for all Linux, so Arch/RHEL folks get the wrong package manager — not something to fix here).
Appreciate the contribution!
The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg` for any non-macOS platform — Windows users would see an apt command that doesn't exist on their system, and Red Hat / Arch users got the wrong package manager too. `getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is already used by render.ts) and handles darwin / linux / win32 correctly. Use it here too. Also rewrite checkFFprobe: - it previously used `which ffprobe` which is not available on Windows (cmd uses `where`), so on Windows the check always reported "Not found" even when ffprobe was on PATH - run `ffprobe -version` directly instead, which works cross-platform whenever ffprobe is resolvable on PATH, and surfaces the version string in the same style as the FFmpeg check
The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg` for any non-macOS platform — Windows users would see an apt command that doesn't exist on their system, and Red Hat / Arch users got the wrong package manager too. `getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is already used by render.ts) and handles darwin / linux / win32 correctly. Use it here too. Also rewrite checkFFprobe: - it previously used `which ffprobe` which is not available on Windows (cmd uses `where`), so on Windows the check always reported "Not found" even when ffprobe was on PATH - run `ffprobe -version` directly instead, which works cross-platform whenever ffprobe is resolvable on PATH, and surfaces the version string in the same style as the FFmpeg check
Summary
hyperframes doctorshipped two bugs that misled non-Debian-Linux and Windows users:Wrong FFmpeg install hint. When FFmpeg was missing, the hint was hardcoded as
sudo apt install ffmpegfor every non-macOS platform. Windows users saw an apt command that doesn't exist on their system; Red Hat / Arch users got the wrong package manager.FFprobe check broken on Windows.
checkFFprobecalledwhich ffprobeviaexecSync.whichisn't a command on Windows (cmd useswhere), so the check always reported "Not found" even when ffprobe was on PATH.Fix
checkFFmpegnow usesgetFFmpegInstallHint()— the helper already exists inbrowser/ffmpeg.ts, already handlesdarwin/linux/win32correctly, and is already used byrender.ts. Just reuse it here.checkFFprobenow runsffprobe -versiondirectly instead of shelling out towhich. This works on every platform where ffprobe is resolvable on PATH, and surfaces the version string in the same style as the FFmpeg check.Before / After
Before (on Windows, ffprobe present):
After:
Before (on Windows, FFmpeg missing):
After:
Test plan
bunx oxlint packages/cli/src/commands/doctor.ts— cleanbunx oxfmt --check packages/cli/src/commands/doctor.ts— cleanbunx tsx packages/cli/src/cli.ts doctorruns locally on macOS — FFprobe now reports version instead of path, all other behavior unchangedNotes
Scope intentionally kept small.
findFFmpeg()inbrowser/ffmpeg.tsstill useswhich ffmpegwhich has the same Windows issue — happy to fix in a follow-up PR if the maintainers want it, but it has a wider blast radius (used byrender.tstoo) so I left it out of this one.