feat(instance): register .mrpack file association for system-level op…#1621
feat(instance): register .mrpack file association for system-level op…#1621zaixiZaixiSJTU wants to merge 1 commit into
Conversation
…en with Add OS-level file handler for .mrpack (Modrinth modpack) files so that double-clicking a .mrpack file in the file manager launches SJMCL and opens the import-modpack modal. - Add bundle.fileAssociations in tauri.conf.json for .mrpack - Add UTExportedTypeDeclarations in Info.plist for macOS custom UTI - Handle cold start via std::env::args() (Windows/Linux) and RunEvent::Opened (macOS) - Handle warm start via single-instance plugin callback (Windows/Linux) - Add check_pending_modpack_import Tauri command for cold-start race condition - Add sjmcl://import-modpack deep link handler in GlobalEventHandler - Add Tauri event bridge (sjmcl://import) for Rust-to-frontend communication
Reviewer's GuideAdds OS-level .mrpack (Modrinth modpack) file association support across platforms and implements the backend/frontend plumbing to open the import-modpack modal when a .mrpack file is opened, handling both cold and warm starts. Sequence diagram for .mrpack file association handling (cold and warm starts)sequenceDiagram
actor User
participant OS
participant TauriBackend
participant GlobalEventHandler
User->>OS: Open .mrpack file
OS->>TauriBackend: Launch SJMCL with .mrpack arg
alt [cold start]
TauriBackend->>TauriBackend: std::env::args
TauriBackend->>TauriBackend: PENDING_MODPACK_IMPORT.lock
TauriBackend->>TauriBackend: store sjmcl://import-modpack?path=...
GlobalEventHandler->>TauriBackend: invoke check_pending_modpack_import
TauriBackend-->>GlobalEventHandler: sjmcl://import-modpack?path=...
GlobalEventHandler->>GlobalEventHandler: openModpackImportFromUrl
GlobalEventHandler->>GlobalEventHandler: openSharedModal import-modpack
else [warm start]
TauriBackend->>TauriBackend: tauri_plugin_single_instance callback
TauriBackend->>TauriBackend: emit sjmcl://import
TauriBackend-->>GlobalEventHandler: sjmcl://import event
GlobalEventHandler->>GlobalEventHandler: openModpackImportFromUrl
GlobalEventHandler->>GlobalEventHandler: openSharedModal import-modpack
end
opt [macOS RunEvent::Opened]
OS->>TauriBackend: RunEvent::Opened urls
TauriBackend->>TauriBackend: update PENDING_MODPACK_IMPORT
TauriBackend->>TauriBackend: emit sjmcl://import
TauriBackend-->>GlobalEventHandler: sjmcl://import event
GlobalEventHandler->>GlobalEventHandler: openModpackImportFromUrl
GlobalEventHandler->>GlobalEventHandler: openSharedModal import-modpack
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The frontend has two nearly identical code paths for opening the import-modpack modal (
importModpackByDeeplinkandopenModpackImportFromUrl); consider consolidating them into a single helper to avoid divergence and make future changes easier. - The deep-link construction logic for
.mrpackfiles in Rust (single-instance callback, cold-startsetup, and macOSRunEvent::Opened) is duplicated; extracting a small helper that takes a path and returns/dispatches thesjmcl://import-modpack?path=...URL would reduce repetition and keep behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The frontend has two nearly identical code paths for opening the import-modpack modal (`importModpackByDeeplink` and `openModpackImportFromUrl`); consider consolidating them into a single helper to avoid divergence and make future changes easier.
- The deep-link construction logic for `.mrpack` files in Rust (single-instance callback, cold-start `setup`, and macOS `RunEvent::Opened`) is duplicated; extracting a small helper that takes a path and returns/dispatches the `sjmcl://import-modpack?path=...` URL would reduce repetition and keep behavior consistent.
## Individual Comments
### Comment 1
<location path="src-tauri/src/lib.rs" line_range="68-69" />
<code_context>
let _ = main_window.set_focus();
+
+ // Handle .mrpack file association on warm re-launch
+ for arg in &args {
+ if arg.ends_with(".mrpack") {
+ let encoded = urlencoding::encode(arg);
+ let deep_link = format!("sjmcl://import-modpack?path={}", encoded);
</code_context>
<issue_to_address>
**suggestion:** Using `ends_with(".mrpack")` directly may miss valid files on case-insensitive platforms or with trailing spaces/quotes.
Consider normalizing each argument before checking: trim surrounding quotes/whitespace, parse as a `Path`, and compare the extension case-insensitively. That way variants like `PACK.MRPACK` or arguments with extra spaces/quotes are still correctly detected.
Suggested implementation:
```rust
// Handle .mrpack file association on warm re-launch
for arg in &args {
// Normalize argument: trim whitespace and surrounding quotes
let normalized = arg
.trim()
.trim_matches(|c| c == '"' || c == '\'');
// Check extension case-insensitively using Path
let has_mrpack_ext = std::path::Path::new(normalized)
.extension()
.and_then(|ext| ext.to_str())
.map(|ext| ext.eq_ignore_ascii_case("mrpack"))
.unwrap_or(false);
if has_mrpack_ext {
let encoded = urlencoding::encode(normalized);
let deep_link = format!("sjmcl://import-modpack?path={}", encoded);
let _ = app.emit("sjmcl://import", deep_link);
}
}
```
If `std::path::Path` is not already in scope at the top of this file, you can optionally add:
- `use std::path::Path;`
and then replace `std::path::Path::new(...)` with `Path::new(...)` for readability.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/lib.rs" line_range="365-373" />
<code_context>
- if let tauri::RunEvent::Exit = event {
+ .run_return(|app_handle, event| match event {
+ #[cfg(target_os = "macos")]
+ tauri::RunEvent::Opened { urls } => {
+ for url in urls {
+ if let Ok(path) = url.to_file_path() {
+ if path.extension().map_or(false, |ext| ext == "mrpack") {
+ if let Some(path_str) = path.to_str() {
+ let encoded = urlencoding::encode(path_str);
+ let deep_link = format!("sjmcl://import-modpack?path={}", encoded);
+ *PENDING_MODPACK_IMPORT.lock().unwrap() = Some(deep_link.clone());
+ let _ = app_handle.emit("sjmcl://import", deep_link);
+ }
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** Potential for the same modpack import to be processed twice on macOS (pending state + emitted event).
In `Opened`, you both set `PENDING_MODPACK_IMPORT` and emit `"sjmcl://import"`. On cold start the frontend also calls `check_pending_modpack_import`, so the same file can be handled via both the event and the pending check. If double-processing is an issue, make one path authoritative (e.g., only use pending state on cold start and skip the emit then, or have the renderer ignore the event if it already consumed a pending import).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| for arg in &args { | ||
| if arg.ends_with(".mrpack") { |
There was a problem hiding this comment.
suggestion: Using ends_with(".mrpack") directly may miss valid files on case-insensitive platforms or with trailing spaces/quotes.
Consider normalizing each argument before checking: trim surrounding quotes/whitespace, parse as a Path, and compare the extension case-insensitively. That way variants like PACK.MRPACK or arguments with extra spaces/quotes are still correctly detected.
Suggested implementation:
// Handle .mrpack file association on warm re-launch
for arg in &args {
// Normalize argument: trim whitespace and surrounding quotes
let normalized = arg
.trim()
.trim_matches(|c| c == '"' || c == '\'');
// Check extension case-insensitively using Path
let has_mrpack_ext = std::path::Path::new(normalized)
.extension()
.and_then(|ext| ext.to_str())
.map(|ext| ext.eq_ignore_ascii_case("mrpack"))
.unwrap_or(false);
if has_mrpack_ext {
let encoded = urlencoding::encode(normalized);
let deep_link = format!("sjmcl://import-modpack?path={}", encoded);
let _ = app.emit("sjmcl://import", deep_link);
}
}If std::path::Path is not already in scope at the top of this file, you can optionally add:
use std::path::Path;
and then replacestd::path::Path::new(...)withPath::new(...)for readability.
| tauri::RunEvent::Opened { urls } => { | ||
| for url in urls { | ||
| if let Ok(path) = url.to_file_path() { | ||
| if path.extension().map_or(false, |ext| ext == "mrpack") { | ||
| if let Some(path_str) = path.to_str() { | ||
| let encoded = urlencoding::encode(path_str); | ||
| let deep_link = format!("sjmcl://import-modpack?path={}", encoded); | ||
| *PENDING_MODPACK_IMPORT.lock().unwrap() = Some(deep_link.clone()); | ||
| let _ = app_handle.emit("sjmcl://import", deep_link); |
There was a problem hiding this comment.
question (bug_risk): Potential for the same modpack import to be processed twice on macOS (pending state + emitted event).
In Opened, you both set PENDING_MODPACK_IMPORT and emit "sjmcl://import". On cold start the frontend also calls check_pending_modpack_import, so the same file can be handled via both the event and the pending check. If double-processing is an issue, make one path authoritative (e.g., only use pending state on cold start and skip the emit then, or have the renderer ignore the event if it already consumed a pending import).
UNIkeEN
left a comment
There was a problem hiding this comment.
如果 pending... command 是防止前一个请求未结束、弹出新的 shared import modal 覆盖,我觉得可以简化。覆盖行为和现在的 single instance 设计是一致的,只需要 sjmcl://import-modpack 一个 deeplink 即可;
如果是为了别的用处,请评论 ovo
|
|
||
| #[tauri::command] | ||
| pub fn check_pending_modpack_import() -> SJMCLResult<Option<String>> { | ||
| let pending = crate::PENDING_MODPACK_IMPORT.lock().unwrap().take(); |
| let unlisten: (() => void) | undefined; | ||
| (async () => { | ||
| unlisten = await listen<string>("sjmcl://import", (event) => { | ||
| openModpackImportFromUrl(event.payload); |
There was a problem hiding this comment.
这里手搓的 sjmcl://import 和上面的 importModpackTrigger 都是 deeplink,有什么区别呢
| if let tauri::RunEvent::Exit = event { | ||
| .run_return(|app_handle, event| match event { | ||
| #[cfg(target_os = "macos")] | ||
| tauri::RunEvent::Opened { urls } => { |
…en with
Add OS-level file handler for .mrpack (Modrinth modpack) files so that double-clicking a .mrpack file in the file manager launches SJMCL and opens the import-modpack modal.
Checklist
This PR is a ..
Related Issues
Summary by Sourcery
Add OS-level .mrpack file association handling so opening Modrinth modpack files launches the app and opens the import-modpack flow across supported platforms.
New Features:
Enhancements: