diff --git a/crates/ui-egui/src/canvas/home.rs b/crates/ui-egui/src/canvas/home.rs index 2987a03c5..d73a5c5bb 100644 --- a/crates/ui-egui/src/canvas/home.rs +++ b/crates/ui-egui/src/canvas/home.rs @@ -74,7 +74,7 @@ fn recent(app: &mut VectorcraftApp, ui: &mut Ui) { for path in files { if recent_row(ui, &path).clicked() { // A failure is reported to the user there. - let _ = crate::io::open_recent_reporting(app, &path); + let _ = crate::io::open_reporting(app, &path); } } } diff --git a/crates/ui-egui/src/dialogs/find_links/tests.rs b/crates/ui-egui/src/dialogs/find_links/tests.rs index 0c7f5d2e8..36659c53b 100644 --- a/crates/ui-egui/src/dialogs/find_links/tests.rs +++ b/crates/ui-egui/src/dialogs/find_links/tests.rs @@ -63,6 +63,8 @@ fn opened_without(app: &mut VectorcraftApp, root: &Path, names: &[&str]) -> Vec< std::fs::create_dir_all(doc.parent().unwrap()).unwrap(); app.run("document.save", json!({"path": doc})).unwrap(); std::fs::remove_dir_all(root.join("orig")).unwrap(); + // Reopened: an open file would only come to the front (#3455). + app.run("file.close", json!({})).unwrap(); crate::io::open_path(app, &doc.to_string_lossy()).unwrap(); files } @@ -271,6 +273,7 @@ fn copy_leaves_placed_documents_where_they_are() { std::fs::create_dir_all(doc.parent().unwrap()).unwrap(); app.run("document.save", json!({"path": doc})).unwrap(); std::fs::remove_dir_all(root.join("orig")).unwrap(); + app.run("file.close", json!({})).unwrap(); crate::io::open_path(&mut app, &doc.to_string_lossy()).unwrap(); super::super::cancel(&mut app); app.run("ui.findLinksInFolder", json!({"folder": root.join("found")})).unwrap(); diff --git a/crates/ui-egui/src/dialogs/missing_fonts/tests.rs b/crates/ui-egui/src/dialogs/missing_fonts/tests.rs index 19d5a1359..0f57b99f4 100644 --- a/crates/ui-egui/src/dialogs/missing_fonts/tests.rs +++ b/crates/ui-egui/src/dialogs/missing_fonts/tests.rs @@ -49,6 +49,8 @@ fn save_doc(app: &mut VectorcraftApp, path: &Path, font: &str, link: bool) { if link { std::fs::remove_file(&pic).unwrap(); } + // Opened afresh by `open`: an open file would only come to the front (#3455). + app.run("file.close", json!({})).unwrap(); } fn open(app: &mut VectorcraftApp, path: &Path) { @@ -135,6 +137,8 @@ fn find_fonts_opens_find_font_on_the_missing_font() { app.run("text.create", json!({"x": 10, "y": 80, "text": "Gone", "font": "Zz Missing Family"})).unwrap(); let doc = root.join("doc.vectorcraft"); app.run("document.save", json!({"path": doc.to_string_lossy()})).unwrap(); + // Opened afresh: an open file would only come to the front (#5369). + app.run("file.close", json!({})).unwrap(); open(&mut app, &doc); assert_eq!(kind(&app), Some(KIND)); app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); @@ -223,6 +227,7 @@ fn one_dialog_at_a_time_document_by_document() { super::super::cancel(&mut app); assert!(app.ui.dialog.is_none()); // A document closed before its turn is passed over. + app.run("file.closeAll", json!({})).unwrap(); open(&mut app, &a); open(&mut app, &b); let index = app.session.documents().iter().rposition(|d| d.path.as_deref() == Some(&*a.to_string_lossy())).unwrap(); @@ -295,6 +300,24 @@ fn a_search_folder_that_fails_is_not_tried_again() { assert_eq!(super::super::confirm(&mut app).err().as_deref(), Some("choose the font files to add")); } +/// Opening a file that is already open only brings its tab to the front: Missing Fonts, closed +/// once, is not asked again (#5369). +#[test] +fn reopening_an_open_document_does_not_ask_about_its_fonts_again() { + let root = folder("reopen"); + let mut app = app_for(&root); + let doc = root.join("doc.vectorcraft"); + save_doc(&mut app, &doc, "No Such Font Family", false); + open(&mut app, &doc); + assert_eq!(kind(&app), Some(KIND)); + super::super::cancel(&mut app); + app.run("file.new", json!({})).unwrap(); + open(&mut app, &doc); + frame(&mut app); + assert_eq!((app.session.documents().len(), app.session.active_index()), (2, Some(0))); + assert_eq!(kind(&app), None); +} + /// A placed document that Links › Edit Original opens asks about its fonts, as File › Open does. #[test] fn edit_original_asks_about_the_placed_documents_fonts() { @@ -302,7 +325,6 @@ fn edit_original_asks_about_the_placed_documents_fonts() { let mut app = app_for(&root); let badge = root.join("badge.vectorcraft"); save_doc(&mut app, &badge, "Badge Missing Family", false); - app.session.execute("file.close", &json!({})).unwrap(); app.run("file.new", json!({"width": 400, "height": 300})).unwrap(); app.run("file.place", json!({"path": badge.to_string_lossy(), "link": true})).unwrap(); let tabs = app.session.documents().len(); diff --git a/crates/ui-egui/src/dialogs/missing_links.rs b/crates/ui-egui/src/dialogs/missing_links.rs index 78affd3c2..81a19912a 100644 --- a/crates/ui-egui/src/dialogs/missing_links.rs +++ b/crates/ui-egui/src/dialogs/missing_links.rs @@ -249,6 +249,8 @@ mod tests { for n in ["a.png", "b.png"] { std::fs::remove_file(dir.join(n)).unwrap(); } + // Reopened: an open file would only come to the front (#3455). + app.run("file.close", json!({})).unwrap(); crate::io::open_path(&mut app, &doc).unwrap(); (app, dir, doc) } @@ -344,6 +346,8 @@ mod tests { }; std::fs::write(&pic, png([0, 0, 200])).unwrap(); mark_modified(2); + // Reopened: an open file would only come to the front (#3455). + app.run("file.close", json!({})).unwrap(); crate::io::open_path(&mut app, &doc).unwrap(); let d = app.ui.dialog.as_ref().expect("asks"); assert_eq!((d.kind.as_str(), d.str("__command")), (super::super::confirm::KIND, "links.update".to_string())); @@ -353,6 +357,7 @@ mod tests { app.run("prefs.set", json!({"key": "updateLinks", "value": "manually"})).unwrap(); std::fs::write(&pic, png([0, 200, 0])).unwrap(); mark_modified(4); + app.session.execute("file.close", &json!({})).unwrap(); crate::io::open_path(&mut app, &doc).unwrap(); assert!(app.ui.dialog.is_none()); assert_eq!(app.run("links.check", json!({})).unwrap()["modified"], 1); diff --git a/crates/ui-egui/src/io.rs b/crates/ui-egui/src/io.rs index 38a67400a..4818a0c15 100644 --- a/crates/ui-egui/src/io.rs +++ b/crates/ui-egui/src/io.rs @@ -24,7 +24,7 @@ pub(crate) use export::{ pub(crate) use open::{EXPORT_PRESETS, import_presets, 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_recent_reporting, open_reporting, opens_as, place_bytes, recent_files, report_open_error, + open_path, open_reporting, opens_as, place_bytes, recent_files, report_open_error, }; #[cfg(test)] pub(crate) use open::{open_file, opened_with_notes}; diff --git a/crates/ui-egui/src/io/open.rs b/crates/ui-egui/src/io/open.rs index 9ea56a0a6..ace4f2694 100644 --- a/crates/ui-egui/src/io/open.rs +++ b/crates/ui-egui/src/io/open.rs @@ -229,15 +229,9 @@ pub fn open_reporting(app: &mut VectorcraftApp, path: &str) -> Result Result { - if let Some(r) = try_activate_open_document(app, path)? { - return Ok(r); - } - open_reporting(app, path) -} - +/// A file already open in a tab is not opened again: its tab comes to the front, unsaved edits +/// and all (#3455), without the questions asked after an open (missing links and fonts). +/// → `{activated: true, …}` as `document.open`'s result, or None. fn try_activate_open_document(app: &mut VectorcraftApp, path: &str) -> Result, String> { let Some(index) = document_index_for_path(app, path) else { return Ok(None); @@ -252,7 +246,7 @@ fn try_activate_open_document(app: &mut VectorcraftApp, path: &str) -> Result Result /// [`open_path`]; with `background`, a document (not a file that installs, imports or asks /// first) is decoded off the UI thread ([`crate::opening::start`]). pub(crate) fn open_file(app: &mut VectorcraftApp, path: &str, background: bool) -> Result { + // File › Open, Open Recent, the Home screen, a drop and the Finder all focus a document that + // is already open, or still being read, instead of opening a second copy. + if let Some(r) = try_activate_open_document(app, path)? { + return Ok(r); + } + if app.opening.pending.iter().any(|p| same_open_path(&p.path, path)) { + return Ok(Value::Null); + } // A shortcut set is read by Import Shortcut Set, up to its reader's cap. if dialogs::shortcut_import::is_kys(path) { dialogs::shortcut_import::command(app, &json!({ "path": path }))?; diff --git a/crates/ui-egui/src/menus/run.rs b/crates/ui-egui/src/menus/run.rs index d97d09222..566145026 100644 --- a/crates/ui-egui/src/menus/run.rs +++ b/crates/ui-egui/src/menus/run.rs @@ -126,7 +126,7 @@ fn run_ui_part1(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option Ok(io::cancel_exports(app)), "file.exportInBackground" => Ok(io::hide_exports(app)), id if id.starts_with("file.openRecent") => match recent_slot(app, id).cloned() { - Some(path) => io::open_recent_reporting(app, &path), + Some(path) => io::open_reporting(app, &path), None => Err("no such recent file".into()), }, "type.findFont" => { diff --git a/crates/ui-egui/src/opening/tests.rs b/crates/ui-egui/src/opening/tests.rs index b50e4a682..3b5d3a404 100644 --- a/crates/ui-egui/src/opening/tests.rs +++ b/crates/ui-egui/src/opening/tests.rs @@ -332,3 +332,19 @@ fn scope_dir() -> std::path::PathBuf { std::fs::create_dir_all(&dir).unwrap(); dir } + +/// A file asked for again while it is still being read is not read a second time (#3455). +#[test] +fn a_file_asked_for_again_while_it_is_read_opens_once() { + let mut app = VectorcraftApp::new(Session::new(), Services::default()); + let bytes = vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false); + start(&mut app, "/art/Poster.vectorcraft", bytes).unwrap(); + assert_eq!(crate::io::open_file(&mut app, "/art/Poster.vectorcraft", true).unwrap(), Value::Null); + assert_eq!(app.opening.pending.len(), 1); + let t0 = std::time::Instant::now(); + while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 { + poll(&mut app); + std::thread::yield_now(); + } + assert_eq!(app.session.documents().len(), 1); +} diff --git a/crates/ui-egui/src/tests/home.rs b/crates/ui-egui/src/tests/home.rs index 1b62ac94c..4f5fc2b3c 100644 --- a/crates/ui-egui/src/tests/home.rs +++ b/crates/ui-egui/src/tests/home.rs @@ -299,10 +299,11 @@ fn home_scrolls_in_a_short_window() { assert_eq!(app.session.documents().len(), 1, "the card opened its preset"); } -/// Home › Recent Files and File › Open Recent focus an open tab instead of opening a second copy -/// (#3455); `document.open` is unchanged. +/// A file that is already open is not opened again, however it is asked for: Home › Recent Files, +/// File › Open Recent, File › Open, the Finder and a drop all focus its tab, unsaved edits and all +/// (#3455). #[test] -fn home_recent_reopen_focuses_an_already_open_document() { +fn reopening_an_open_file_focuses_its_tab() { let services = crate::Services { read: Some(Box::new(|p: &str| std::fs::read(p).map_err(|e| e.to_string()))), write: Some(Box::new(|p: &str, b: &[u8]| std::fs::write(p, b).map_err(|e| e.to_string()))), @@ -316,13 +317,35 @@ fn home_recent_reopen_focuses_an_already_open_document() { app.run("file.new", json!({"width": 100, "height": 100})).unwrap(); app.run("shape.rectangle", json!({"x": 1, "y": 2, "width": 10, "height": 10})).unwrap(); app.run("file.saveAs", json!({"path": path})).unwrap(); - app.run("file.new", json!({})).unwrap(); - assert_eq!(app.session.documents().len(), 2); - assert_eq!(app.session.active_index(), Some(1)); - let r = crate::io::open_recent_reporting(&mut app, &path).unwrap(); + // An edit not saved yet. + app.run("shape.rectangle", json!({"x": 20, "y": 2, "width": 10, "height": 10})).unwrap(); + let back = |app: &mut VectorcraftApp| { + app.run("file.new", json!({})).unwrap(); + assert_eq!(app.session.documents().len(), 2); + }; + let focused = |app: &VectorcraftApp, how: &str| { + assert_eq!(app.session.documents().len(), 2, "{how}: no second copy"); + assert_eq!(app.session.active_index(), Some(0), "{how}"); + assert_eq!(app.session.doc().unwrap().doc.layers[0].children().unwrap().len(), 2, "{how}: the unsaved edit is kept"); + app.session.doc().unwrap().uid + }; + back(&mut app); + // Home › Recent Files, File › Open Recent, File › Open… and the Finder. + let r = crate::io::open_reporting(&mut app, &path).unwrap(); assert_eq!(r["activated"], true); - assert_eq!(app.session.documents().len(), 2); - assert_eq!(app.session.active_index(), Some(0)); - assert_eq!(app.session.doc().unwrap().doc.layers[0].children().unwrap().len(), 1); + let uid = focused(&app, "open_reporting"); + app.run("document.activate", json!({"index": 1})).unwrap(); + app.run("file.close", json!({})).unwrap(); + back(&mut app); + // File › Open with a path (an open dialog's answer, `app.open`), and a path spelled differently. + let other = d.join(".").join("doc.vectorcraft").to_string_lossy().to_string(); + assert_eq!(app.run("file.open", json!({"path": other})).unwrap()["activated"], true); + assert_eq!(focused(&app, "file.open"), uid); + app.run("document.activate", json!({"index": 1})).unwrap(); + app.run("file.close", json!({})).unwrap(); + back(&mut app); + // A drop. + crate::place::drop_files(&mut app, vec![(crate::place::DropTarget::Open, ("doc.vectorcraft".into(), Some(path.clone()), vec![]))]); + assert_eq!(focused(&app, "drop"), uid); let _ = std::fs::remove_dir_all(d); } diff --git a/crates/ui-egui/src/tests/save.rs b/crates/ui-egui/src/tests/save.rs index 472f564eb..ec8790f9f 100644 --- a/crates/ui-egui/src/tests/save.rs +++ b/crates/ui-egui/src/tests/save.rs @@ -232,6 +232,8 @@ fn a_document_reopens_at_its_saved_view() { // Moving around afterwards isn't an edit. app.view_mut().unwrap().zoom = 0.5; assert!(!app.session.active().unwrap().is_dirty()); + // Reopened: an open file would only come to the front (#3455). + app.run("file.close", json!({})).unwrap(); io::open_path(&mut app, &path).unwrap(); let v = *app.view().unwrap(); assert_eq!((v.zoom, v.center, v.rotation, v.fitted), (3.0, Point::new(70.0, 40.0), 30.0, true));