From 14d928b2c8cba7c09dfba02695e8d35e3a28f334 Mon Sep 17 00:00:00 2001 From: Igor Churmeev Date: Wed, 14 Oct 2026 13:19:08 +0000 Subject: [PATCH] fix(macos): open files double-clicked in Finder (#1031) macOS hands files opened from Finder, the Dock or open -a to the application delegate's application:openURLs:, which winit's delegate lacks, so AppKit showed "SolidCraft cannot open files in the SolidCraft design format". open_documents adds that method to the delegate's class before launch finishes and queues the paths for the UI, which opens them like files named on the command line. This is the one place SolidCraft's own code uses unsafe: the app crate denies unsafe_code instead of forbidding it, and only the two macOS-only functions in open_documents.rs allow it, with the reason documented there and in AGENTS.md. Adapted from VectorCraft (see ATTRIBUTION.md). Closes #997. --- AGENTS.md | 4 +- ATTRIBUTION.md | 1 + CLAUDE.md | 4 +- CONTRIBUTING.md | 3 +- Cargo.lock | 4 + apps/solidcraft/Cargo.toml | 20 ++++- apps/solidcraft/src/main.rs | 17 +++- apps/solidcraft/src/open_documents.rs | 120 ++++++++++++++++++++++++++ 8 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 apps/solidcraft/src/open_documents.rs diff --git a/AGENTS.md b/AGENTS.md index 05e54a1a..a83fb38b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,9 @@ new uses of it anywhere else. Public docs/web/Flatpak deployments use project Pa ## Never crash People trust SolidCraft with their designs; a crash loses their work. **This outranks feature work.** - **No panics in non-test code:** no `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!`, - `unimplemented!`; no `unsafe` (`unsafe_code = "forbid"`). + `unimplemented!`; no `unsafe` (`unsafe_code = "forbid"`). The one exception is two functions in + `apps/solidcraft/src/open_documents.rs` (that crate denies it instead): macOS hands Finder files + only to an Objective-C method. - **Errors are `Result`** through the crate's error type and `?`. An unfinished feature returns an error ("not supported yet: …"); it never panics. - **Input-derived numbers are hostile** (command parameters, design files, control-channel diff --git a/ATTRIBUTION.md b/ATTRIBUTION.md index 7db9d6e0..b315f198 100644 --- a/ATTRIBUTION.md +++ b/ATTRIBUTION.md @@ -119,6 +119,7 @@ Editable drawing code, native projects and shipped exports are collected under ` | crates/mcp/src/tools.rs (tool schema helpers and argument checking) | CADCraft `crates/mcp/src/tools.rs`, GridCraft `crates/mcp/src/tools.rs` | MIT OR Apache-2.0 | | crates/io/src/threemf.rs (bounded zip part reading and writing) | GridCraft `crates/xlsx/src/package.rs` | MIT OR Apache-2.0 | | crates/ui-egui/src/titlebar.rs (title bar drag, caption buttons, resize zones, their tests) and the window setup in apps/solidcraft/src/main.rs | VectorCraft `crates/ui-egui/src/titlebar.rs`, `apps/vectorcraft/src/main.rs` | MIT OR Apache-2.0 | +| apps/solidcraft/src/open_documents.rs (macOS: files opened from Finder and the Dock, its test) | VectorCraft `apps/vectorcraft/src/open_documents.rs` (https://github.com/storytold/vectorcraft) | MIT OR Apache-2.0 | | LICENSE-MIT, LICENSE-APACHE (licence text) | CADCraft | — | | .github/workflows/release.yml, .github/workflows/packaging-lint.yml and packaging/ (release pipeline: Linux AppImage/deb/rpm/Flatpak/tar.gz, FreeBSD, macOS dmg, Windows MSI/portable zip, web zip; AppRun, nfpm, Flatpak, WiX and Info.plist templates) | PhotoCraft (https://github.com/storytold/photocraft) `.github/workflows/{release,packaging-lint}.yml`, `packaging/` | Apache-2.0 | diff --git a/CLAUDE.md b/CLAUDE.md index 34609918..b2d05925 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,9 @@ new uses of it anywhere else. Public docs/web/Flatpak deployments use project Pa ## Never crash People trust SolidCraft with their designs; a crash loses their work. **This outranks feature work.** - **No panics in non-test code:** no `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!`, - `unimplemented!`; no `unsafe` (`unsafe_code = "forbid"`). + `unimplemented!`; no `unsafe` (`unsafe_code = "forbid"`). The one exception is two functions in + `apps/solidcraft/src/open_documents.rs` (that crate denies it instead): macOS hands Finder files + only to an Objective-C method. - **Errors are `Result`** through the crate's error type and `?`. An unfinished feature returns an error ("not supported yet: …"); it never panics. - **Input-derived numbers are hostile** (command parameters, design files, control-channel diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 94977271..5c696371 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -107,7 +107,8 @@ The [README](README.md#architecture) has the layer table. The short version: `cargo xtask assets` enforces this. 4. **Never crash.** People trust a CAD app with their work. - Non-test code never uses `unwrap`, `expect`, `panic!` or indexing on input-derived data. - There's no `unsafe`, and clippy denies it. + There's no `unsafe` outside `apps/solidcraft/src/open_documents.rs` (macOS file-open events), + and the compiler denies it. - An unsupported case returns an error ("not supported yet: …"); it never panics. - Every call into truck goes through `solidcraft_kernel::guard`. - Every crash fix comes with a regression test. diff --git a/Cargo.lock b/Cargo.lock index 619a70ab..2906e54d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3283,12 +3283,16 @@ dependencies = [ name = "solidcraft" version = "0.10.0" dependencies = [ + "block2 0.6.2", "eframe", "egui", "embed-resource", "getrandom 0.3.4", "hidapi", "log", + "objc2 0.6.5", + "objc2-app-kit 0.3.2", + "objc2-foundation 0.3.2", "quick-xml", "rfd", "serde_json", diff --git a/apps/solidcraft/Cargo.toml b/apps/solidcraft/Cargo.toml index ca1974d7..229cbc83 100644 --- a/apps/solidcraft/Cargo.toml +++ b/apps/solidcraft/Cargo.toml @@ -7,8 +7,17 @@ license.workspace = true publish = false default-run = "solidcraft" -[lints] -workspace = true +# The workspace lints, except unsafe_code: denied, not forbidden, so that src/open_documents.rs can +# allow it (macOS file-open events have no safe API). Keep the rest in sync with [workspace.lints]. +[lints.rust] +unsafe_code = "deny" + +[lints.clippy] +dbg_macro = "warn" +todo = "warn" +too_many_arguments = "allow" +type_complexity = "allow" +neg_cmp_op_on_partial_ord = "allow" [dependencies] solidcraft-engine = { workspace = true } @@ -26,6 +35,13 @@ quick-xml = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] hidapi = { version = "2.6.7", default-features = false, features = ["windows-native"] } +[target.'cfg(target_os = "macos")'.dependencies] +# Files opened from Finder and the Dock (src/open_documents.rs); all already come with eframe and rfd. +objc2 = "0.6" +objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSApplication", "NSResponder"] } +objc2-foundation = { version = "0.3", default-features = false, features = ["std", "block2", "NSArray", "NSNotification", "NSOperation", "NSString", "NSURL"] } +block2 = "0.6" + [target.'cfg(all(unix, not(target_os = "macos")))'.dependencies] winit = { version = "0.30", default-features = false, features = ["x11"] } diff --git a/apps/solidcraft/src/main.rs b/apps/solidcraft/src/main.rs index dc66c428..0ff292dc 100644 --- a/apps/solidcraft/src/main.rs +++ b/apps/solidcraft/src/main.rs @@ -10,12 +10,14 @@ //! See `solidcraft_ui_egui::control` and docs/control-protocol.md. #![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -#![forbid(unsafe_code)] +#![deny(unsafe_code)] #[cfg(not(target_os = "macos"))] #[path = "../../../assets/icons/app/shared/source/app_icon.rs"] mod app_icon; mod control_server; +#[cfg(target_os = "macos")] +mod open_documents; #[cfg(any(target_os = "windows", target_os = "linux"))] mod space_mouse; @@ -38,6 +40,10 @@ impl eframe::App for App { ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); solidcraft_ui_egui::documents::request_quit(&mut self.solve); } + #[cfg(target_os = "macos")] + for f in open_documents::take() { + self.solve.open_path_async(&f); + } self.solve.logic(ctx); #[cfg(any(target_os = "windows", target_os = "linux"))] self.space_mouse.tick(&mut self.solve, ctx); @@ -286,6 +292,9 @@ fn main() -> eframe::Result { b.with_x11(); })); } + // Files opened from Finder and the Dock arrive as events, not arguments. + #[cfg(target_os = "macos")] + open_documents::install(); let result = eframe::run_native( "SolidCraft", options, @@ -320,6 +329,12 @@ fn main() -> eframe::Result { app.session.mark_saved(); app.session.undo.clear(); } + // A launch from Finder: its files arrived before the window. + #[cfg(target_os = "macos")] + let files = { + open_documents::set_ui(&cc.egui_ctx); + [files, open_documents::take()].concat() + }; for f in &files { app.open_path_async(f); } diff --git a/apps/solidcraft/src/open_documents.rs b/apps/solidcraft/src/open_documents.rs new file mode 100644 index 00000000..6d527e47 --- /dev/null +++ b/apps/solidcraft/src/open_documents.rs @@ -0,0 +1,120 @@ +//! macOS: files opened from Finder (double-click, Open With, a drop on the Dock icon) or with +//! `open -a SolidCraft file` (#997). +//! +//! macOS doesn't pass these on the command line: AppKit hands them to the application delegate's +//! `application:openURLs:`, and when the delegate has no such method it asks `NSDocumentController`, +//! which fails with "SolidCraft cannot open files in the … format" because SolidCraft isn't an +//! `NSDocument` app. winit 0.30's delegate doesn't implement the method, so [`install`] adds it to +//! the delegate's class when the app is about to finish launching: after winit set the delegate and +//! before AppKit delivers the files the app was launched with. The handler queues the paths and +//! wakes the UI, which opens them with [`take`] like files named on the command line. +//! +//! Adding a method to another crate's Objective-C class has no safe API, hence the `unsafe_code` +//! allowance on [`install`] and [`add_open_urls_method`] (winit overrides `sendEvent:` on +//! `NSApplication` similarly). + +use std::ptr::NonNull; +use std::sync::{Mutex, OnceLock, PoisonError}; + +use block2::RcBlock; +use objc2::MainThreadMarker; +use objc2::runtime::{AnyObject, Imp, Sel}; +use objc2::sel; +use objc2_app_kit::{NSApplication, NSApplicationWillFinishLaunchingNotification}; +use objc2_foundation::{NSArray, NSNotification, NSNotificationCenter, NSURL}; + +/// Paths that arrived and haven't been opened yet. +static PENDING: Mutex> = Mutex::new(Vec::new()); +/// The UI to wake when files arrive (unset until the window exists; the first frame opens them). +static UI: OnceLock = OnceLock::new(); + +/// Handle open-document events from now on. Call before the event loop runs. +#[allow(unsafe_code)] +pub fn install() { + let block = RcBlock::new(|_: NonNull| add_open_urls_method()); + // SAFETY: the name is AppKit's notification constant; no object or queue filter, so the block + // runs on the posting thread (AppKit posts it on the main thread). The block captures nothing, + // so it is sendable. The notification center keeps the registration (and the block) for the + // life of the app. + unsafe { + NSNotificationCenter::defaultCenter().addObserverForName_object_queue_usingBlock( + Some(NSApplicationWillFinishLaunchingNotification), + None, + None, + &block, + ); + } +} + +/// Wake `ctx` when files arrive while the app runs. +pub fn set_ui(ctx: &egui::Context) { + // Set once; a second call would hand over the same context. + let _ = UI.set(ctx.clone()); +} + +/// The paths that arrived since the last call. +pub fn take() -> Vec { + std::mem::take(&mut *PENDING.lock().unwrap_or_else(PoisonError::into_inner)) +} + +/// `application:openURLs:` on the application delegate's class. +#[allow(unsafe_code)] +fn add_open_urls_method() { + let Some(mtm) = MainThreadMarker::new() else { return }; + let Some(delegate) = NSApplication::sharedApplication(mtm).delegate() else { + log::warn!("no application delegate; files opened from Finder won't open"); + return; + }; + let class = AsRef::::as_ref(&*delegate).class(); + let handler: extern "C-unwind" fn(&AnyObject, Sel, &AnyObject, &NSArray) = open_urls; + // SAFETY: `Imp` is the untyped form of a method implementation; the Objective-C runtime calls + // it with the arguments the type encoding declares, which match `open_urls`: no return value + // (`v`), the receiver (`@`), the selector (`:`), the application (`@`) and the URL array (`@`). + let added = unsafe { + let imp = std::mem::transmute::), Imp>(handler); + objc2::ffi::class_addMethod(std::ptr::from_ref(class).cast_mut(), sel!(application:openURLs:), imp, c"v@:@@".as_ptr()) + }; + if !added.as_bool() { + log::warn!("the application delegate already handles open-document events"); + } +} + +/// AppKit calls this; a panic stops here instead of unwinding through AppKit's frames. +extern "C-unwind" fn open_urls(_delegate: &AnyObject, _cmd: Sel, _app: &AnyObject, urls: &NSArray) { + if std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| queue(urls))).is_err() { + log::warn!("files opened from Finder were dropped after an internal error"); + } +} + +/// Queue the files among `urls` and wake the UI. +fn queue(urls: &NSArray) { + // Only file URLs: macOS 27 turns an https URL's path into a file path too. + let paths = urls.iter().filter(|u| u.isFileURL()).filter_map(|u| u.to_file_path()).map(|p| p.to_string_lossy().into_owned()); + PENDING.lock().unwrap_or_else(PoisonError::into_inner).extend(paths); + if let Some(ctx) = UI.get() { + ctx.request_repaint(); + } +} + +#[cfg(test)] +mod tests { + use objc2::runtime::NSObject; + use objc2_foundation::NSString; + + use super::*; + + /// Files queue until the UI takes them; URLs that aren't files are left out. + #[test] + fn opened_files_wait_for_the_ui() { + let web = NSURL::URLWithString(&NSString::from_str("https://example.com/a.solidcraft")).unwrap(); + let urls = NSArray::from_retained_slice(&[ + NSURL::from_file_path("/tmp/a b.solidcraft").unwrap(), + web, + NSURL::from_file_path("/tmp/c.step").unwrap(), + ]); + let any = NSObject::new(); + open_urls(&any, sel!(application:openURLs:), &any, &urls); + assert_eq!(take(), ["/tmp/a b.solidcraft", "/tmp/c.step"]); + assert!(take().is_empty()); + } +}