-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Delete on backend after closing window #1029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new Tauri command Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Header as Header.tsx
participant Cmd as commands.editorDeleteProject()
participant Tauri as Tauri Runtime
participant Rust as editor_delete_project
User->>Header: Click "Delete" + confirm
Header->>Cmd: await editorDeleteProject()
Cmd->>Tauri: invoke("editor_delete_project")
Tauri->>Rust: call command (app, editor_instance, window)
rect rgba(200,230,255,0.25)
Rust->>Rust: close window (errors ignored)
Rust->>Rust: sleep 20ms
end
rect rgba(230,255,230,0.25)
Rust->>Rust: remove_dir_all(project_path) (async)
Rust->>Tauri: emit RecordingDeleted(project_path)
end
Tauri-->>Cmd: resolved
Cmd-->>Header: resolved
Header-->>User: Deletion completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/Header.tsx (1)
78-79: Surface deletion failures to the user.Wrap the call to handle backend errors (e.g., if Windows still holds file locks) and notify via the existing global dialog.
- await commands.editorDeleteProject(); + try { + await commands.editorDeleteProject(); + } catch (e) { + await commands.globalMessageDialog("Failed to delete recording. Please try again."); + }Also,
getCurrentWindowandremoveimports are now unused after this refactor; consider removing them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/lib.rs(2 hunks)apps/desktop/src/routes/editor/Header.tsx(1 hunks)apps/desktop/src/utils/tauri.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Do not edit auto-generated file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Header.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Header.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/Header.tsx
apps/desktop/src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use tauri_specta on the Rust side for IPC (typed commands/events) and emit events via generated types
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
{crates,apps/cli,apps/desktop/src-tauri}/**/*.rs: Rust code must be formatted with rustfmt and adhere to workspace clippy lints
Rust module files should use snake_case names
Files:
apps/desktop/src-tauri/src/lib.rs
{crates,apps/cli,apps/desktop/src-tauri}/**/{src,tests}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust tests should live in src (inline/unit) or tests (integration) directories
Files:
apps/desktop/src-tauri/src/lib.rs
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/editor/Header.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-08T16:48:20.727Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: AGENTS.md:0-0
Timestamp: 2025-09-08T16:48:20.727Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated file: tauri.ts
Applied to files:
apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/editor_window.rs (3)
window(64-64)window(76-76)window(123-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src/utils/tauri.ts (1)
265-268: Generated bindings: confirm this came from specta export (do not hand‑edit).This file is auto‑generated. The new
editorDeleteProject()entry looks consistent, but please ensure it was produced bytauri_specta::Builder::exportand not edited manually. If you change the Rust command signature later (e.g., to returnResult), re‑export to regenerate this file.apps/desktop/src-tauri/src/lib.rs (1)
1910-1911: Command registration looks good.
editor_delete_projectis properly added to the specta builder. If you adopt theResult<(), String>return in the command, remember to re‑export TS bindings so the frontend receives rethrown errors as expected.
| #[tauri::command] | ||
| #[specta::specta] | ||
| async fn editor_delete_project(editor_instance: WindowEditorInstance, window: tauri::Window) { | ||
| let _ = window.close(); | ||
|
|
||
| let _ = tokio::fs::remove_dir_all(&editor_instance.0.project_path).await; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: incorrect field access; plus fragile delete timing on Windows.
- Critical:
editor_instance.0.project_pathlikely won’t compile; elsewhereWindowEditorInstanceexposesproject_pathdirectly. - Reliability: closing the window and immediately deleting can still race with WebView teardown on Windows. Add a short wait/until‑destroyed check and retry deletion. Emit
RecordingDeletedafter success so the UI can update.
-#[tauri::command]
-#[specta::specta]
-async fn editor_delete_project(editor_instance: WindowEditorInstance, window: tauri::Window) {
- let _ = window.close();
-
- let _ = tokio::fs::remove_dir_all(&editor_instance.0.project_path).await;
-}
+#[tauri::command]
+#[specta::specta]
+async fn editor_delete_project(
+ editor_instance: WindowEditorInstance,
+ window: tauri::Window,
+) -> Result<(), String> {
+ let app = window.app_handle().clone();
+ let label = window.label().to_string();
+
+ // Best-effort close; proceed regardless.
+ let _ = window.close();
+
+ // Wait briefly for the window to actually disappear (helps on Windows).
+ for _ in 0..50 {
+ if !app.webview_windows().contains_key(&label) {
+ break;
+ }
+ tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+ }
+
+ let path = editor_instance.project_path.clone();
+
+ // Retry deletion a few times to avoid "file in use" races.
+ let mut last_err: Option<std::io::Error> = None;
+ for _ in 0..10 {
+ match tokio::fs::remove_dir_all(&path).await {
+ Ok(_) => {
+ RecordingDeleted { path: path.clone() }.emit(&app).ok();
+ return Ok(());
+ }
+ Err(e) => {
+ last_err = Some(e);
+ tokio::time::sleep(std::time::Duration::from_millis(200)).await;
+ }
+ }
+ }
+ Err(format!(
+ "Failed to delete project at {:?}: {}",
+ path,
+ last_err.unwrap_or_else(|| std::io::Error::new(std::io::ErrorKind::Other, "unknown"))
+ ))
+}Committable suggestion skipped: line range outside the PR's diff.
…psoftware/cap into fix-windows-delete-via-editor
Fixes delete from editor on Windows by doing the delete after closing the window, preventing the 'File is in use' error
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Style