feat(deeplink): add pause, resume, and device switching actions (#1540) #1549
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| DeepLinkAction::PauseRecording => { | ||
| let state = app.state::<ArcLock<App>>(); | ||
| let guard = state.read().await; | ||
| if let Some(recording) = guard.current_recording() { | ||
| recording.pause().await.map_err(|e| e.to_string()) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Holding the state.read() guard across an .await can deadlock / block other actions. Consider grabbing the recording handle first so the lock is dropped before calling pause().
| DeepLinkAction::PauseRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let guard = state.read().await; | |
| if let Some(recording) = guard.current_recording() { | |
| recording.pause().await.map_err(|e| e.to_string()) | |
| } else { | |
| Ok(()) | |
| } | |
| } | |
| DeepLinkAction::PauseRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let recording = { | |
| let guard = state.read().await; | |
| guard.current_recording() | |
| }; | |
| if let Some(recording) = recording { | |
| recording.pause().await.map_err(|e| e.to_string()) | |
| } else { | |
| Ok(()) | |
| } | |
| } |
| DeepLinkAction::ResumeRecording => { | ||
| let state = app.state::<ArcLock<App>>(); | ||
| let guard = state.read().await; | ||
| if let Some(recording) = guard.current_recording() { | ||
| recording.resume().await.map_err(|e| e.to_string()) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Same idea here: drop the read lock before awaiting resume().
| DeepLinkAction::ResumeRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let guard = state.read().await; | |
| if let Some(recording) = guard.current_recording() { | |
| recording.resume().await.map_err(|e| e.to_string()) | |
| } else { | |
| Ok(()) | |
| } | |
| } | |
| DeepLinkAction::ResumeRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let recording = { | |
| let guard = state.read().await; | |
| guard.current_recording() | |
| }; | |
| if let Some(recording) = recording { | |
| recording.resume().await.map_err(|e| e.to_string()) | |
| } else { | |
| Ok(()) | |
| } | |
| } |
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
| }, | ||
| PauseRecording, | ||
| ResumeRecording, | ||
| SwitchMicrophone { |
There was a problem hiding this comment.
For SwitchMicrophone / SwitchCamera, using Option means a missing/null field in the deeplink will remove the device. If that’s not intended, consider making these fields required or treating None as a no-op/error to avoid accidental device disable.
Sadik111
force-pushed
the
feat/deeplink-raycast
branch
2 times, most recently
from
2930b64 to
e30b448
Compare
| DeepLinkAction::SwitchCamera { camera } => { | ||
| if let Some(ref id) = camera { | ||
| let cameras: Vec<_> = cap_camera::list_cameras().collect(); | ||
| let exists = cameras.iter().any(|info| match id { |
There was a problem hiding this comment.
Minor thing: you can avoid allocating Vec here by using the iterator directly.
| let exists = cameras.iter().any(|info| match id { | |
| let exists = cap_camera::list_cameras().any(|info| match id { | |
| DeviceOrModelID::DeviceID(device_id) => info.device_id() == device_id, | |
| DeviceOrModelID::ModelID(model_id) => { | |
| info.model_id().is_some_and(|existing| existing == model_id) | |
| } | |
| }); |
Sadik111
force-pushed
the
feat/deeplink-raycast
branch
from
e30b448 to
d58d3d4
Compare
| } | ||
| DeepLinkAction::PauseRecording => { | ||
| let state = app.state::<ArcLock<App>>(); | ||
| let (instant_handle, studio_handle) = { |
There was a problem hiding this comment.
Pause/Resume are basically the same block — might be worth extracting the handle lookup into a tiny helper so they stay in sync (and you can add "instant"/"studio" context to the error string in one place).
| }); | ||
|
|
||
| if !exists { | ||
| return Err(format!("Camera with ID \"{:?}\" not found", id)); |
There was a problem hiding this comment.
Minor UX: {:?} here ends up returning DeviceID(...) / ModelID(...) in the string. A variant-specific message reads nicer.
| return Err(format!("Camera with ID \"{:?}\" not found", id)); | |
| return Err(match id { | |
| DeviceOrModelID::DeviceID(device_id) => { | |
| format!("Camera with device ID \"{}\" not found", device_id) | |
| } | |
| DeviceOrModelID::ModelID(model_id) => { | |
| format!("Camera with model ID \"{}\" not found", model_id) | |
| } | |
| }); |
|
@greptile review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| DeepLinkAction::PauseRecording => { | ||
| let state = app.state::<ArcLock<App>>(); | ||
| let (instant_handle, studio_handle) = { | ||
| let guard = state.read().await; | ||
| match guard.current_recording() { | ||
| Some(crate::recording::InProgressRecording::Instant { handle, .. }) => { | ||
| (Some(handle.clone()), None) | ||
| } | ||
| Some(crate::recording::InProgressRecording::Studio { handle, .. }) => { | ||
| (None, Some(handle.clone())) | ||
| } | ||
| None => (None, None), | ||
| } | ||
| }; | ||
|
|
||
| if let Some(handle) = instant_handle { | ||
| handle.pause().await.map_err(|e| e.to_string())?; | ||
| } | ||
| if let Some(handle) = studio_handle { | ||
| handle.pause().await.map_err(|e| e.to_string())?; | ||
| } | ||
| Ok(()) | ||
| } | ||
| DeepLinkAction::ResumeRecording => { | ||
| let state = app.state::<ArcLock<App>>(); | ||
| let (instant_handle, studio_handle) = { | ||
| let guard = state.read().await; | ||
| match guard.current_recording() { | ||
| Some(crate::recording::InProgressRecording::Instant { handle, .. }) => { | ||
| (Some(handle.clone()), None) | ||
| } | ||
| Some(crate::recording::InProgressRecording::Studio { handle, .. }) => { | ||
| (None, Some(handle.clone())) | ||
| } | ||
| None => (None, None), | ||
| } | ||
| }; | ||
|
|
||
| if let Some(handle) = instant_handle { | ||
| handle.resume().await.map_err(|e| e.to_string())?; | ||
| } | ||
| if let Some(handle) = studio_handle { | ||
| handle.resume().await.map_err(|e| e.to_string())?; | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
logic: PauseRecording and ResumeRecording implementations don't emit RecordingEvent::Paused and RecordingEvent::Resumed events, unlike the existing pause_recording() and resume_recording() Tauri commands in recording.rs:966-989. This could leave the UI out of sync since these events are how the frontend gets notified of pause/resume state changes.
Consider calling the existing Tauri commands instead of reimplementing the pause/resume logic, or ensure these events are emitted here:
| DeepLinkAction::PauseRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let (instant_handle, studio_handle) = { | |
| let guard = state.read().await; | |
| match guard.current_recording() { | |
| Some(crate::recording::InProgressRecording::Instant { handle, .. }) => { | |
| (Some(handle.clone()), None) | |
| } | |
| Some(crate::recording::InProgressRecording::Studio { handle, .. }) => { | |
| (None, Some(handle.clone())) | |
| } | |
| None => (None, None), | |
| } | |
| }; | |
| if let Some(handle) = instant_handle { | |
| handle.pause().await.map_err(|e| e.to_string())?; | |
| } | |
| if let Some(handle) = studio_handle { | |
| handle.pause().await.map_err(|e| e.to_string())?; | |
| } | |
| Ok(()) | |
| } | |
| DeepLinkAction::ResumeRecording => { | |
| let state = app.state::<ArcLock<App>>(); | |
| let (instant_handle, studio_handle) = { | |
| let guard = state.read().await; | |
| match guard.current_recording() { | |
| Some(crate::recording::InProgressRecording::Instant { handle, .. }) => { | |
| (Some(handle.clone()), None) | |
| } | |
| Some(crate::recording::InProgressRecording::Studio { handle, .. }) => { | |
| (None, Some(handle.clone())) | |
| } | |
| None => (None, None), | |
| } | |
| }; | |
| if let Some(handle) = instant_handle { | |
| handle.resume().await.map_err(|e| e.to_string())?; | |
| } | |
| if let Some(handle) = studio_handle { | |
| handle.resume().await.map_err(|e| e.to_string())?; | |
| } | |
| Ok(()) | |
| DeepLinkAction::PauseRecording => { | |
| crate::recording::pause_recording(app.clone(), app.state()).await | |
| } | |
| DeepLinkAction::ResumeRecording => { | |
| crate::recording::resume_recording(app.clone(), app.state()).await | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 163:207
Comment:
**logic:** PauseRecording and ResumeRecording implementations don't emit `RecordingEvent::Paused` and `RecordingEvent::Resumed` events, unlike the existing `pause_recording()` and `resume_recording()` Tauri commands in `recording.rs:966-989`. This could leave the UI out of sync since these events are how the frontend gets notified of pause/resume state changes.
Consider calling the existing Tauri commands instead of reimplementing the pause/resume logic, or ensure these events are emitted here:
```suggestion
DeepLinkAction::PauseRecording => {
crate::recording::pause_recording(app.clone(), app.state()).await
}
DeepLinkAction::ResumeRecording => {
crate::recording::resume_recording(app.clone(), app.state()).await
}
```
How can I resolve this? If you propose a fix, please make it concise.
Sadik111
force-pushed
the
feat/deeplink-raycast
branch
from
d58d3d4 to
ff1e3c1
Compare
|
@greptile review |
|
Fixed. I updated the implementation to call |
|
@greptile review |
1 participant
Fixes #1540
Changes
Added
PauseRecording,
ResumeRecording,
SwitchMicrophone,
and SwitchCamera to DeepLinkAction.
These implementations hook into the existing recording state and device management functions.
Verification
Due to local build environment constraints (Windows/FFmpeg), I could not record a live demo video. However:
Static Analysis
Verified that deeplink_actions.rs correctly hooks into crate::recording.
Edge Cases
Validated that Pause/Resume now use the official commands to emit Paused/Resumed events.
Safety
Added existence checks for microphones and cameras to prevent crashes or errors on invalid IDs.
How to Test
Trigger cap://action?value={"pause_recording":{}} → The app should pause and emit a Paused event.
Trigger cap://action?value={"resume_recording":{}} → The app should resume and emit a Resumed event.
Demo Video Note: I am developing on a Windows machine where the build environment for this project is difficult to set up fully (FFmpeg/Rust/Tauri dependencies). Therefore, I cannot generate a runtime video recording.
However, I have verified the changes via:
Static Analysis: Ensuring correct hookups to crate::recording.
Validating Fixes: I implemented the reviewer's feedback to use
pause_recording()
commands (ensuring event emission) and added defensive checks for device IDs.
The code compiles and passes the CI checks. I am happy to address any further code review feedback!
/claim #1540
Greptile Summary
This PR extends deeplink support by adding four new actions:
PauseRecording,ResumeRecording,SwitchMicrophone, andSwitchCamera. These implementations properly integrate with existing recording state management and device handling functions, enabling Raycast extension support and other external tools to control the app through deeplinks.PauseRecordingandResumeRecordingactions that delegate to existing Tauri commands which emit proper state-change eventsSwitchMicrophoneaction with validation to ensure the microphone exists before attempting to switchSwitchCameraaction with comprehensive validation supporting both DeviceID and ModelID typesConfidence Score: 5/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant External as External App participant DeepLink as DeepLink Handler participant RecordingCmd as Recording Commands participant DeviceCmd as Device Commands participant State as App State External->>DeepLink: cap://action?value={...} DeepLink->>DeepLink: Parse & validate URL rect rgb(200, 220, 255) note over DeepLink,RecordingCmd: Pause/Resume Actions DeepLink->>RecordingCmd: pause_recording() RecordingCmd->>State: Pause recording & emit Paused event end rect rgb(200, 255, 220) note over DeepLink,DeviceCmd: Device Switch Actions DeepLink->>DeviceCmd: Validate device exists DeepLink->>DeviceCmd: set_mic_input() or set_camera_input() DeviceCmd->>State: Update device & emit event if restored end