fix: find whisper-cli in ~/.local/bin when PATH omits it#34
Conversation
The engine's find_command only walked PATH, so a menu-launched app on a desktop session without ~/.local/bin on PATH could not find the whisper-cli that setup itself installs there. Fall back to ~/.local/bin after the PATH walk, and make the CLI's find_command reuse the engine's instead of duplicating the lookup. Fixes #5
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa6f913f4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .map(|path| std::env::split_paths(&path).collect()) | ||
| .unwrap_or_default(); | ||
| if let Some(home) = std::env::var_os("HOME") { | ||
| dirs.push(PathBuf::from(home).join(".local/bin")); |
There was a problem hiding this comment.
When a GUI session omits ~/.local/bin from PATH, this fallback makes command_exists("wl-copy")/ydotool/wtype return true for files that Command::new("wl-copy") cannot actually spawn, because paste.rs and sounds.rs still execute the bare command name rather than the resolved PathBuf. In that environment, a local-only wl-copy can now short-circuit clipboard fallback to /usr/bin/xclip and fail with starting wl-copy; before this change command_exists stayed false and the next PATH-visible backend was tried.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed. paste.rs, sounds.rs, and kwin.rs now resolve tools via find_command and spawn the returned path instead of the bare name, so command_exists and the actual spawn can no longer disagree. The recorder default (pw-record) gets the same resolution.
…pawns With the ~/.local/bin fallback, command_exists could report a tool that Command::new(bare_name) cannot spawn. Resolve via find_command and spawn the returned path in paste, sounds, kwin, and the recorder default.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What
The engine's
find_commandonly walkedPATH, so a menu-launched app on a desktop session whose PATH omits~/.local/bincould not find the whisper-cli that setup itself installs there (GNOME/Wayland sessions often don't source.profilefor GUI apps).engine::find_commandnow falls back to~/.local/binafter the PATH walk; lookup extracted into a testablefind_in_dirs.find_commandnow reuses the engine's (keeping its explicit-path handling), sowhisper-cli,pickscribe-cleanup-gui, etc. get the same fallback..desktopPATH-wrapper idea from the issue: fixing lookup in the engine covers every launch path (menu, autostart, file manager), not just the installer-written launcher.install.shis untouched.docs/releases/UNRELEASED.mdupdated.Tested
cargo test --workspace --locked --all-targets(80 passed, incl. newfind_in_dirsregression test)cargo check -p pickscribe-app --features pickscribe-app/custom-protocolNot tested
~/.local/binon PATH.Closes #5