diff --git a/crates/ui-egui/src/dialogs/plugin.rs b/crates/ui-egui/src/dialogs/plugin.rs index a5736c47e..001bfd695 100644 --- a/crates/ui-egui/src/dialogs/plugin.rs +++ b/crates/ui-egui/src/dialogs/plugin.rs @@ -227,7 +227,8 @@ pub(super) const IMPORT_SPEC: DialogSpec = DialogSpec { }; /// Open With with importer `plugin`, which has parameters: its fields at their defaults, and -/// `open`, Open With's own parameters (`path`, `importer`), kept for Open. +/// `open`, Open With's own parameters (`path`, `importer`, and `picked` for a file from the +/// picker, which is read off the UI thread), kept for Open. pub fn open_import(app: &mut VectorcraftApp, plugin: &Plugin, open: Value) { let mut fields = fields_for(plugin.manifest()); fields.insert("__open".into(), open); @@ -239,5 +240,6 @@ fn import_confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result let mut p = d.fields.get("__open").cloned().filter(Value::is_object).ok_or("no file to open")?; p["params"] = form::params(d); app.ui.dialog = None; - crate::io::open_with(app, &p) + let picked = p["picked"] == true; + crate::io::open_with_on(app, &p, picked && crate::opening::available()) } diff --git a/crates/ui-egui/src/dialogs/tests_plugin_import.rs b/crates/ui-egui/src/dialogs/tests_plugin_import.rs index 9d75db484..2c1e2d091 100644 --- a/crates/ui-egui/src/dialogs/tests_plugin_import.rs +++ b/crates/ui-egui/src/dialogs/tests_plugin_import.rs @@ -113,6 +113,17 @@ fn open_with_asks_for_an_importer_s_parameters_and_passes_them_on() { assert_eq!(artboard(&app), (10.0, 20.0, 110.0, 70.0)); assert_eq!(app.ui.recent_files, std::slice::from_ref(&file)); + // Only a file from the picker goes off the UI thread after the dialog: a caller naming `path` + // gets the document. + app.run("file.openWith", json!({"importer": "plugin.test.ui.params", "path": file})).unwrap(); + assert_eq!(app.ui.dialog.as_ref().unwrap().fields["__open"]["picked"], false); + app.ui.dialog = None; + let picked = file.clone(); + app.services.pick_open = Some(Box::new(move |_: &crate::FilePick| Some(picked.clone()))); + app.run("file.openWith", json!({"importer": "plugin.test.ui.params"})).unwrap(); + assert_eq!(app.ui.dialog.as_ref().unwrap().fields["__open"]["picked"], true); + app.ui.dialog = None; + let r = app.run("file.openWith", json!({"importer": "plugin.test.ui.params", "path": file, "params": {"density": 7}})).unwrap(); assert_eq!(r["format"], "plugin.test.ui.params"); assert!(app.ui.dialog.is_none()); diff --git a/crates/ui-egui/src/io.rs b/crates/ui-egui/src/io.rs index 41694fdbe..bba294f0a 100644 --- a/crates/ui-egui/src/io.rs +++ b/crates/ui-egui/src/io.rs @@ -21,7 +21,7 @@ pub use export::{EXPORT_CANCELLED, cancel_exports, export, export_for_screens, e pub(crate) use export::{ export_files, exported_with_notes, file_url, open_in_app, open_original, reveal_path, run_to_file, save_command_output_named, }; -pub(crate) use open::{EXPORT_PRESETS, import_presets, opened}; +pub(crate) use open::{EXPORT_PRESETS, import_presets, open_with_on, opened}; pub use open::{ MAX_RECENT_FILES, OpensAs, ask_revert, install_module, install_plugin, new_from_template, note_recent, open_bytes, open_dialog, open_document, open_path, open_reporting, open_with, opens_as, place_bytes, recent_files, report_open_error, diff --git a/crates/ui-egui/src/io/open.rs b/crates/ui-egui/src/io/open.rs index fc0aa8481..5373ce9d6 100644 --- a/crates/ui-egui/src/io/open.rs +++ b/crates/ui-egui/src/io/open.rs @@ -264,8 +264,21 @@ pub fn open_dialog(app: &mut VectorcraftApp) -> Result<(), String> { /// failure is reported as File › Open reports it. An importer with parameters, given no `params`, /// asks for them first in dialog `pluginImport`, at their defaults. The web's file picker can't /// carry the importer: there File › Open's picker opens, and the file opens as File › Open opens -/// it. +/// it. A file the user picked is read off the UI thread where that is available +/// ([`crate::opening`]), and the result is then `{background: true, status}`; with `path` the file +/// opens before the reply, as automation needs. pub fn open_with(app: &mut VectorcraftApp, p: &Value) -> Result { + open_with_on(app, p, picks_a_file(p) && crate::opening::available()) +} + +/// Do File › Open With's parameters leave the file to the picker? +fn picks_a_file(p: &Value) -> bool { + p.get("path").and_then(Value::as_str).is_none_or(str::is_empty) +} + +/// [`open_with`]; with `background`, the importer runs off the UI thread +/// ([`crate::opening::start_with`]), its progress window offering Cancel. +pub(crate) fn open_with_on(app: &mut VectorcraftApp, p: &Value, background: bool) -> Result { let importer = p.get("importer").and_then(Value::as_str).ok_or("missing `importer` (see document.formats)")?; let plugin = vectorcraft_plugins::import::named(importer).map_err(|e| e.to_string())?; let path = match p.get("path").and_then(Value::as_str).filter(|s| !s.is_empty()) { @@ -285,14 +298,21 @@ pub fn open_with(app: &mut VectorcraftApp, p: &Value) -> Result { let given = p.get("params").filter(|v| !v.is_null()); let opened = vectorcraft_engine::file_access::check_read(&path).and_then(|()| check_import_size(&path, true)).and_then(|()| { if given.is_none() && !plugin.manifest().params.is_empty() { - dialogs::plugin::open_import(app, &plugin, json!({ "path": path, "importer": importer })); + dialogs::plugin::open_import(app, &plugin, json!({ "path": path, "importer": importer, "picked": picks_a_file(p) })); return Ok(Value::Null); } let mut o = json!({ "importer": importer }); if let Some(params) = given { o["params"] = params.clone(); } - let bytes = read(app, &path)?; + let mut bytes = read(app, &path)?; + if background { + match crate::opening::start_with(app, &path, bytes, o.clone()) { + Ok(r) => return Ok(r), + // No thread: at once. + Err(back) => bytes = back, + } + } let r = open_document(app, &path, &bytes, Some(path.clone()), &o)?; note_recent(app, &path); Ok(r) diff --git a/crates/ui-egui/src/menus/ui_commands.rs b/crates/ui-egui/src/menus/ui_commands.rs index debd6c224..d310e9318 100644 --- a/crates/ui-egui/src/menus/ui_commands.rs +++ b/crates/ui-egui/src/menus/ui_commands.rs @@ -34,7 +34,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[ "file.openWith", "Open With", "", - "{importer: \"plugin.\" (an importer plug-in's format id, see document.formats), path?} File › Open With: open the file with that importer (document.open's importer), also a file whose extension a built-in format reads; without a path pick one of the files with the extensions it lists (the web opens File › Open's picker, and the file opens as File › Open opens it) → document.open's result | null while the picker is open", + "{importer: \"plugin.\" (an importer plug-in's format id, see document.formats), path?} File › Open With: open the file with that importer (document.open's importer), also a file whose extension a built-in format reads; without a path pick one of the files with the extensions it lists (the web opens File › Open's picker, and the file opens as File › Open opens it) → with a path, document.open's result; a picked file is read off the UI thread on the desktop, {background: true, status}, and opens once the importer returns (file.cancelOpen stops it) | null while the picker is open", ), ( "file.save", diff --git a/crates/ui-egui/src/opening.rs b/crates/ui-egui/src/opening.rs index 9191509ae..99e0e9d6b 100644 --- a/crates/ui-egui/src/opening.rs +++ b/crates/ui-egui/src/opening.rs @@ -11,6 +11,7 @@ use std::sync::Arc; use std::sync::mpsc::{Receiver, TryRecvError, channel}; use serde_json::{Value, json}; +use vectorcraft_engine::cancel::{self, CancelToken}; use vectorcraft_engine::cmd::fileio::{self, ReadFile}; use vectorcraft_engine::file_access::ScopeHandle; @@ -30,6 +31,8 @@ pub struct Pending { /// The automation roots in force where the open started: the document is installed under /// them too (its links are read as it opens). scope: ScopeHandle, + /// Raised by Cancel, on the thread reading the file: an importer plug-in stops. + token: CancelToken, /// Continue in Background: the progress window no longer shows it. pub hidden: bool, } @@ -65,6 +68,11 @@ pub fn available() -> bool { /// own ([`fileio::read_for_open`]) → `{background: true, status}`, or the bytes back when no thread /// could start (the caller opens the file at once). pub fn start(app: &mut VectorcraftApp, path: &str, bytes: Vec) -> Result> { + start_with(app, path, bytes, Value::Null) +} + +/// [`start`] with the `document.open` options in `params` (`importer` and its `params`). +pub fn start_with(app: &mut VectorcraftApp, path: &str, bytes: Vec, params: Value) -> Result> { let (tx, done) = channel(); let bytes = Arc::new(bytes); let (name, file) = (path.to_string(), bytes.clone()); @@ -74,11 +82,15 @@ pub fn start(app: &mut VectorcraftApp, path: &str, bytes: Vec) -> Result) -> Result Value { let cancelled = std::mem::take(&mut app.opening.pending); for p in &cancelled { + p.token.cancel(); // The status bar no longer says it is opening. if app.ui.status == format!("{}…", p.label) { app.ui.status.clear(); diff --git a/crates/ui-egui/src/opening/tests.rs b/crates/ui-egui/src/opening/tests.rs index 1f5a6cbd1..ef62509bf 100644 --- a/crates/ui-egui/src/opening/tests.rs +++ b/crates/ui-egui/src/opening/tests.rs @@ -50,6 +50,74 @@ fn open_ready(app: &mut VectorcraftApp) { poll(app); } +fn install_importer(app: &mut VectorcraftApp, src: &str) { + let wasm = wat::parse_str(src).unwrap(); + app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wasm)})).unwrap(); +} + +/// A file for the importers below, in a folder of its own. +fn importer_file(tag: &str) -> (std::path::PathBuf, String) { + let dir = std::env::temp_dir().join(format!("vc-ui-opening-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("a.tob"); + std::fs::write(&file, "not json").unwrap(); + let file = file.to_string_lossy().into_owned(); + (dir, file) +} + +/// Open With off the UI thread: nothing opens until the importer has read the file with the +/// parameters it was given; then the document opens and joins Open Recent, and a failure is +/// reported in a dialog. +#[test] +fn open_with_runs_the_importer_off_the_ui_thread() { + use vectorcraft_plugins::wat::{self as tpl, Given}; + let rect = r#"{"objects":[{"type":"path","path":{"subpaths":[{"closed":true,"anchors":[{"p":[10,20]},{"p":[110,20]},{"p":[110,70]},{"p":[10,70]}]}]}}]}"#; + let (dir, file) = importer_file("with"); + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.services.read = Some(Box::new(|p: &str| std::fs::read(p).map_err(|e| e.to_string()))); + let params = r#"{"density":{"type":"number","min":0,"max":10,"default":4}}"#; + install_importer(&mut app, &tpl::when_given(&tpl::importer("test.opening.with", r#"["tob"]"#, params), Given::Params, r#""density":7"#, rect)); + + let open = |density: i64| json!({"importer": "plugin.test.opening.with", "path": file, "params": {"density": density}}); + let r = crate::io::open_with_on(&mut app, &open(7), true).unwrap(); + assert_eq!(r["background"], true, "{r}"); + assert!(app.session.documents().is_empty(), "nothing opens before the importer has read the file"); + assert!(app.ui.recent_files.is_empty()); + open_ready(&mut app); + assert_eq!(app.session.documents().len(), 1); + assert_eq!(app.ui.recent_files, std::slice::from_ref(&file)); + + // Density 4 gives the file back, which isn't JSON. + app.ui.recent_files.clear(); + crate::io::open_with_on(&mut app, &open(4), true).unwrap(); + open_ready(&mut app); + assert_eq!(app.session.documents().len(), 1); + assert!(app.ui.status.contains("Couldn't open a.tob"), "{}", app.ui.status); + assert!(app.ui.dialog.is_some(), "the failure is reported in a dialog"); + assert!(app.ui.recent_files.is_empty()); + app.run("plugin.remove", json!({"id": "test.opening.with"})).unwrap(); + let _ = std::fs::remove_dir_all(&dir); +} + +/// Cancel stops an importer that never returns, on the thread reading the file. +#[test] +fn cancel_stops_a_running_importer() { + use vectorcraft_plugins::wat as tpl; + let (dir, file) = importer_file("cancel"); + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + app.services.read = Some(Box::new(|p: &str| std::fs::read(p).map_err(|e| e.to_string()))); + install_importer(&mut app, &tpl::module(&tpl::importer("test.opening.spins", r#"["tob"]"#, "{}"), "(loop $l (br $l)) (i64.const 0)", "", 1)); + crate::io::open_with_on(&mut app, &json!({"importer": "plugin.test.opening.spins", "path": file}), true).unwrap(); + let done = std::mem::replace(&mut app.opening.pending[0].done, std::sync::mpsc::channel().1); + assert_eq!(cancel_all(&mut app)["cancelled"], 1); + let r = done.recv().unwrap(); + assert!(r.as_ref().err().is_some_and(|e| e.contains("cancelled")), "{:?}", r.err()); + assert!(app.session.documents().is_empty()); + app.run("plugin.remove", json!({"id": "test.opening.spins"})).unwrap(); + let _ = std::fs::remove_dir_all(&dir); +} + /// #1694: a file that finishes opening while a dialog is up waits for it to close, so the /// dialog's document stays the active one. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 8d77e4d82..4c83cbcd5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6099,8 +6099,10 @@ as `svg`. The importer is given at most 128 MiB of the file; its objects become their bounds, Save then asks for a name, and `warnings` holds the importer's warnings. `plugin.run` of an importer is an error that names `document.open`. In the app, File › Open With lists the installed importers, and `file.openWith {importer, path?, params?}` opens `path`, or a file picked among the importer's extensions, with one (on -the web it opens File › Open's picker) and adds it to Open Recent; an importer with parameters, given no `params`, -first asks for them in the `pluginImport` dialog. `params` holds the importer's parameters (their defaults when +the web it opens File › Open's picker) and adds it to Open Recent. With `path` the reply is `document.open`'s result; +on the desktop a picked file is read off the UI thread and the reply is `{background: true, status}`, and +`file.cancelOpen` stops the importer. An importer with parameters, given no `params`, first asks for them in the +`pluginImport` dialog. `params` holds the importer's parameters (their defaults when omitted). A failing plug-in returns an error and leaves the document as it was (an export writes no file, and an import opens no document). A filter whose manifest asks for a heavy budget (minutes rather than a minute, `budget: "heavy"` in its `plugin.inspect {path | dataBase64}` entry) installs only with `allowHeavy: true`: ask the user first. `plugin.run` diff --git a/docs/plugins.md b/docs/plugins.md index 187add504..eb4537e17 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -65,7 +65,9 @@ extensions that a built-in format reads, and those File › Open installs, impor `.kys`, `.jsx`, presets, libraries and swatch or graphic style libraries such as `.ase`). File › Open does not give those files to the importer, and Open With does. **File › Open With** lists the installed importers by name (`file.openWith {"importer", "path"?, "params"?}`), and each opens a file picked among its -extensions with that importer (on the web, File › Open's picker opens instead) and adds it to Open Recent. An importer +extensions with that importer (on the web, File › Open's picker opens instead) and adds it to Open Recent. On the +desktop the importer runs off the UI thread, and a run that takes a while shows a progress window with Cancel, which +stops it. An importer with parameters, given no `params`, asks for them in a dialog of the same kind once the file is picked, and its Open button opens the file; File › Open runs an importer with its parameters' defaults. **File › Open** of a `.wasm` file installs a plug-in too (on the web this is how plug-ins get in). A heavy filter asks before it is installed, and runs in the