Uh oh!
There was an error while loading. Please reload this page.
docs: add comprehensive docstrings to ipc_common module - #24
Conversation
Add module-level, struct-level, field-level, and function-level documentation to ipc_common.rs to meet the 80% docstring coverage threshold flagged by CodeRabbit. Documents the wire protocol (newline-delimited JSON), all three supported IPC commands (open, ping, show), platform compatibility notes, and JSON examples for both IpcCommand and IpcResponse. Closes#18
📝 WalkthroughWalkthroughThis change introduces a public IPC command system in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/tauri/src-tauri/src/ipc_common.rs (2)
111-111:⚠️ Potential issue | 🟡 Minor
std::fs::canonicalizefails on non-existent paths — doc and error message could be misleading.
std::fs::canonicalizerequires the path to already exist on the filesystem (it resolves symlinks and callsrealpath/GetFullPathNameunder the hood). If a caller passes a syntactically valid but non-existent path, the OS error returned isNotFound, yet the response surfaces as"Invalid path: {e}"— which implies the path string itself is malformed rather than absent.Two related gaps:
- The
process_commanddoc (line 89) says "invalid" path but doesn't note the existence requirement imposed bycanonicalize.- The error message
"Invalid path: …"conflates "path doesn't exist" with "path is syntactically invalid".Consider distinguishing the two cases or updating the doc to call out the existence requirement:
💡 Suggested improvement
Err(e) => IpcResponse { success: false, - error: Some(format!("Invalid path: {}", e)),+ error: Some(format!("Path not found or inaccessible: {}", e)), },And in the docstring:
-/// Returns an error if the path is missing, invalid, or the event-/// fails to emit.+/// Returns an error if the path field is absent, the path does not exist on+/// disk (or is otherwise inaccessible to `std::fs::canonicalize`), or the+/// event fails to emit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/ipc_common.rs` at line 111, Update the process_command documentation to state that std::fs::canonicalize requires the path to exist (it resolves symlinks/real paths) and then change the canonicalize error handling in the match where std::fs::canonicalize(&path) is called: detect io::ErrorKind::NotFound and return an explicit "path does not exist" error message, while preserving a separate "invalid path" or generic message for other error kinds; reference the process_command docstring and the canonicalize match branch to make the two cases distinct.
148-158:⚠️ Potential issue | 🟡 Minor
showsilently no-ops and returnssuccess: truewhen the main window is absent.If
get_webview_window("main")returnsNone(e.g., during app teardown or if the window label changes), theshowcommand does nothing at all yet still reports success. The doc accurately says "Always returnssuccess: true", but the behavior is a silent no-op that callers cannot distinguish from a genuine window-focus operation.The same pattern exists in the
openbranch (lines 115–119): window focus is silently skipped if the window isn't found, yet theopen-fileevent is still emitted — which could cause the frontend to try to open a file in a hidden/minimized window.Consider returning an error (or at minimum logging a warning) when the window cannot be found:
💡 Suggested improvement for `show`
"show" => { - if let Some(window) = app.get_webview_window("main") {- let _ = window.unminimize();- let _ = window.show();- let _ = window.set_focus();+ match app.get_webview_window("main") {+ Some(window) => {+ let _ = window.unminimize();+ let _ = window.show();+ let _ = window.set_focus();+ }+ None => {+ return IpcResponse {+ success: false,+ error: Some("Main window not found".to_string()),+ };+ } } IpcResponse { success: true, error: None, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/ipc_common.rs` around lines 148 - 158, The "show" branch currently silently no-ops and returns IpcResponse { success: true } when app.get_webview_window("main") is None; change this so that when get_webview_window("main") returns None you either return IpcResponse with success: false and an explanatory error message or at minimum log a warning before returning success=false; update the same pattern in the "open" branch (where the open-file event is emitted) to detect a missing window and return an error/log instead of proceeding silently. Locate the branches using get_webview_window("main") and the IpcResponse construction to implement the check and adjust return values/messages accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/tauri/src-tauri/src/ipc_common.rs`:
- Line 111: Update the process_command documentation to state that
std::fs::canonicalize requires the path to exist (it resolves symlinks/real
paths) and then change the canonicalize error handling in the match where
std::fs::canonicalize(&path) is called: detect io::ErrorKind::NotFound and
return an explicit "path does not exist" error message, while preserving a
separate "invalid path" or generic message for other error kinds; reference the
process_command docstring and the canonicalize match branch to make the two
cases distinct.
- Around line 148-158: The "show" branch currently silently no-ops and returns
IpcResponse { success: true } when app.get_webview_window("main") is None;
change this so that when get_webview_window("main") returns None you either
return IpcResponse with success: false and an explanatory error message or at
minimum log a warning before returning success=false; update the same pattern in
the "open" branch (where the open-file event is emitted) to detect a missing
window and return an error/log instead of proceeding silently. Locate the
branches using get_webview_window("main") and the IpcResponse construction to
implement the check and adjust return values/messages accordingly.
Uh oh!
There was an error while loading. Please reload this page.
Summary
What’s documented
Test plan
Closes#18
Summary by CodeRabbit