fix(voice): import hotkey items in non-macOS dictation listener#4728
fix(voice): import hotkey items in non-macOS dictation listener#4728YOMXXX wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe dictation listener now conditionally imports hotkey symbols only on non-macOS targets, so macOS builds exclude unused hotkey imports. ChangesPlatform-specific import guard
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
start_rdev_listener (cfg not(target_os = "macos")) uses the hotkey module's parse_hotkey/start_listener plus the ActivationMode and HotkeyEvent enums, but none were imported — so cargo clippy/build fails on Linux with "cannot find module hotkey" / "cannot find type ActivationMode" / "cannot find type HotkeyEvent". macOS is unaffected because start_rdev_listener is cfg'd out there. Add a cfg-gated use matching the only cfg that references these items (no unused-import warning on macOS).
3d28eca to
9b378c2
Compare
M3gA-Mind
left a comment
There was a problem hiding this comment.
PR #4728 — fix(voice): import hotkey items in non-macOS dictation listener
Walkthrough
This PR adds a single #[cfg(not(target_os = "macos"))]-gated use bringing hotkey, ActivationMode, and HotkeyEvent into dictation_listener.rs. start_rdev_listener (itself non-macOS) references those symbols, but the import was never wired in when voice/hotkey.rs was added in #4722 (088102386), so Linux/Windows fail to compile with E0433 while macOS — where the whole path is cfg'd out — stays green. I verified against upstream/main that the import is genuinely missing and that every reference (lines 140–182) sits inside the non-macOS start_rdev_listener, so the gate matches the usage exactly. The fix is correct, minimal, and warning-clean on macOS. Assessment: correct and ready — the only substantive issue is that it duplicates PR #4736.
Changes
| File | Summary |
|---|---|
src/openhuman/voice/dictation_listener.rs |
Adds a non-macOS-gated use crate::openhuman::voice::hotkey::{self, ActivationMode, HotkeyEvent}; so the rdev listener compiles on Linux/Windows. |
Actionable comments (0)
No blocking or major issues. The change does exactly what the title claims and the cfg gate is placed correctly.
Duplicate PR — needs maintainer decision (1)
src/openhuman/voice/dictation_listener.rs:19-20— duplicate of #4736. PR #4736 (fix/voice-dictation-imports) makes the functionally identical fix, differing only in import spelling: this PR usesuse crate::openhuman::voice::hotkey::{...}while #4736 usesuse super::hotkey::{...}. Both resolve to the same module (dictation_listener.rslives insrc/openhuman/voice/, sosuper==crate::openhuman::voice) and both carry the same#[cfg(not(target_os = "macos"))]gate. Only one should merge; the other should be closed to avoid a conflicting no-op. Note this PR was opened first (08:42Z vs 11:42Z), and itscrate::…spelling matches the adjacentuse crate::openhuman::config::Config;on line 15 — a slight consistency point in this PR's favor. This is a maintainer call, not a code defect.
Nitpicks (0)
- None. The explanatory comment on lines 16–18 is accurate and appropriately scoped ("why", not "what").
Questions for the author (0)
- None.
Verified / looks good
- Bug is real:
upstream/main(7659c75dc) is missing this import;start_rdev_listenerreferenceshotkey::parse_hotkey,hotkey::start_listener,ActivationMode::{Push,Tap}, andHotkeyEvent::{Pressed,Released}with nouse→E0433on non-macOS. Introduced by #4722. - cfg gate is exact: every reference to the imported symbols (lines 140–182) is inside
start_rdev_listener, which is#[cfg(not(target_os = "macos"))](line 136). No macOSunused_importswarning; no missing-symbol error on Linux/Windows. - Symbols exist:
hotkey.rsexportspub enum ActivationMode(:25),pub enum HotkeyEvent(:35),pub fn parse_hotkey(:154),pub fn start_listener(:191);voice/mod.rsdeclarespub mod hotkey;. - Testing N/A is justified: import-only change with no executable lines; CI Rust Quality (Linux clippy) is the authoritative signal.
- CodeRabbit posted an APPROVED review with a trivial-effort walkthrough and no actionable inline threads; there are no maintainer threads to resolve.
Leaving this as a comment per instruction — approve/merge is the maintainer's decision, especially given the #4736 duplication.
| // `ActivationMode`, `HotkeyEvent`, and `parse_hotkey`/`start_listener` fns — so | ||
| // gate the import to the same cfg to avoid unused-import warnings on macOS. | ||
| #[cfg(not(target_os = "macos"))] | ||
| use crate::openhuman::voice::hotkey::{self, ActivationMode, HotkeyEvent}; |
There was a problem hiding this comment.
Duplicate of #4736 — one should be closed. This import is functionally identical to the one added by PR #4736; the only difference is spelling: here it's use crate::openhuman::voice::hotkey::{self, ActivationMode, HotkeyEvent};, in #4736 it's use super::hotkey::{...}. Both resolve to the same module and both carry the same #[cfg(not(target_os = "macos"))] gate, so exactly one of these PRs should land and the other be closed. Two points favor this PR as the canonical one: it was opened first (08:42Z vs 11:42Z), and the crate::… spelling matches the adjacent use crate::openhuman::config::Config; on line 15. Otherwise the fix is correct — every reference to these symbols (lines 140–182) lives inside the non-macOS start_rdev_listener, so the cfg gate matches usage exactly. Maintainer call.
|
Confirming this is the canonical PR for the missing non-macOS |
Summary
use crate::openhuman::voice::hotkey::{self, ActivationMode, HotkeyEvent};todictation_listener.rsso the non-macOSrdevhotkey listener compiles.main(cannot find module hotkey/cannot find type ActivationMode/cannot find type HotkeyEvent) that breakcargo clippy -p openhuman— and therefore every branch merged up tomain.Problem
start_rdev_listener(#[cfg(not(target_os = "macos"))]) uses thehotkeymodule (parse_hotkey,start_listener) plus theActivationModeandHotkeyEventenums, but none of those items were imported into the module.Rust Quality (fmt, clippy)) this fails with threeE0433errors. macOS is unaffected becausestart_rdev_listeneriscfg'd out there — so the gap is invisible when building locally on macOS.Solution
use, gated with a matching#[cfg(not(target_os = "macos"))]so it exists exactly on the cfg that references them and does not become an unused-import warning on macOS.Submission Checklist
cfg-gated code; no new behavior. The proof is that the crate builds/clippies on non-macOS.useimport line, no executable lines to cover.Closes #NNN— N/A: no tracked issue; found via CI failure on Linux.Impact
Rust Quality (fmt, clippy)onmain. No runtime behavior change. macOS unchanged.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Summary by CodeRabbit