diff --git a/crates/engine/src/cmd/docmenu.rs b/crates/engine/src/cmd/docmenu.rs index fd9dbaae7..2cbd4a3b9 100644 --- a/crates/engine/src/cmd/docmenu.rs +++ b/crates/engine/src/cmd/docmenu.rs @@ -153,7 +153,7 @@ pub fn specs() -> Vec { "Lock Guides", ["View", "Guides"], Some("Cmd+Alt+;"), - "{locked?: bool} toggle (or set) the session's guide lock → {locked}", + "{locked?: bool} toggle (or set) the session's guide lock; locking deselects the ruler guides and guide paths in every open document → {locked}", has_doc, lock_guides ), @@ -601,12 +601,20 @@ fn release_guides(s: &mut Session, p: &Value) -> Result { fn lock_guides(s: &mut Session, p: &Value) -> Result { let v = p.get("locked").and_then(Value::as_bool).unwrap_or(!s.menu.guides_locked); s.menu.guides_locked = v; - // Locked guides can't stay selected, ruler guides or guide paths ([`Session::select`] lets - // those go). + // Locked guides can't stay selected, ruler guides or guide paths, in any open document + // ([`Session::select`] lets those go in the active one). let st = s.doc()?; if v && (!st.selection.guides.is_empty() || st.selection.objects.iter().any(|id| crate::is_guide_path(&st.doc, *id))) { s.select(|_, sel| sel.guides.clear())?; } + if v { + let active = s.active; + for (i, st) in s.docs.iter_mut().enumerate() { + if Some(i) != active && crate::deselect_guides(&st.doc, &mut st.selection) { + st.revision += 1; + } + } + } Ok(json!({ "locked": v })) } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 9c5bbd559..eaa50e514 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -371,6 +371,7 @@ pub(crate) fn settle_interaction(s: &mut Session) -> Result<()> { fn undo(s: &mut Session, _: &Value) -> Result { settle_interaction(s)?; + let guides_locked = s.menu.guides_locked; let st = s.doc_mut()?; let e = st.history.undo.pop().ok_or_else(|| EngineError::Other("nothing to undo".into()))?; let label = e.label.clone(); @@ -383,12 +384,17 @@ fn undo(s: &mut Session, _: &Value) -> Result { st.history.trim(); st.revision += 1; st.selection.prune(&st.doc); + // Guides locked since the step was made stay deselected. + if guides_locked { + crate::deselect_guides(&st.doc, &mut st.selection); + } super::maskedit::follow_history(st, editing); Ok(json!({ "undone": label })) } fn redo(s: &mut Session, _: &Value) -> Result { settle_interaction(s)?; + let guides_locked = s.menu.guides_locked; let st = s.doc_mut()?; let e = st.history.redo.pop().ok_or_else(|| EngineError::Other("nothing to redo".into()))?; let label = e.label.clone(); @@ -401,6 +407,9 @@ fn redo(s: &mut Session, _: &Value) -> Result { st.history.trim(); st.revision += 1; st.selection.prune(&st.doc); + if guides_locked { + crate::deselect_guides(&st.doc, &mut st.selection); + } super::maskedit::follow_history(st, editing); Ok(json!({ "redone": label })) } diff --git a/crates/engine/src/cmd/fileio/batch.rs b/crates/engine/src/cmd/fileio/batch.rs index 9ff293dfb..02d8c7246 100644 --- a/crates/engine/src/cmd/fileio/batch.rs +++ b/crates/engine/src/cmd/fileio/batch.rs @@ -99,7 +99,7 @@ fn run_steps(s: &mut Session, label: &str, artboard: Option, cmds: &mut [Va /// ones close, closed or replaced ones (`stash`) come back, and each has its art, selection, /// history, file, format and saved state again (#1697: undoing the batch's interaction alone left /// a Save As step's new file and an Undo step's history in place). Files the steps wrote stay -/// written. +/// written. Lock Guides stays as the steps left it, and while it is on the guides are deselected. fn roll_back(s: &mut Session, start: Vec, stash: &[DocState]) { let open = std::mem::take(&mut s.docs); s.docs = start; @@ -107,5 +107,8 @@ fn roll_back(s: &mut Session, start: Vec, stash: &[DocState]) { // Past the newest revision the document reached, so the restored one redraws. let newest = open.iter().chain(stash).filter(|d| d.uid == st.uid).map(|d| d.revision).max().unwrap_or(st.revision); st.revision = newest.saturating_add(1); + if s.menu.guides_locked { + crate::deselect_guides(&st.doc, &mut st.selection); + } } } diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index bd71c073e..d806d9c80 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -1061,11 +1061,16 @@ fn isolate(s: &mut Session, p: &Value) -> Result { } fn exit_isolation(s: &mut Session, _: &Value) -> Result { + let guides_locked = s.menu.guides_locked; let st = s.doc_mut()?; if let Some(i) = st.isolation.take() { super::distortcmds::finish_edit_text(st, i); let target = st.doc.parent_of(i).filter(|p| st.doc.node(*p).is_some_and(|n| n.extras().shaper.is_some())).unwrap_or(i); st.selection.set([target]); + // A guide path isolated while Lock Guides is on isn't selected (#1534). + if guides_locked { + crate::deselect_guides(&st.doc, &mut st.selection); + } } st.revision += 1; ok() diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 526b915a8..27fc05e5b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -682,6 +682,18 @@ pub(crate) fn is_guide_path(d: &Document, id: NodeId) -> bool { d.node(id).is_some_and(|n| matches!(n.kind, NodeKind::Path { guide: true, .. })) } +/// Deselect the ruler guides and the paths made into guides: while View → Guides → Lock Guides is +/// on, they can't be selected (#1534), whatever selects them. → whether any was selected. +pub(crate) fn deselect_guides(d: &Document, sel: &mut Selection) -> bool { + let paths: Vec = sel.objects.iter().copied().filter(|id| is_guide_path(d, *id)).collect(); + let any = !paths.is_empty() || !sel.guides.is_empty(); + for id in paths { + sel.remove(id); + } + sel.guides.clear(); + any +} + /// The error of an edit that would nest objects deeper than [`vectorcraft_doc::MAX_DEPTH`]. pub const TOO_DEEP: &str = "objects can't be nested more than 32 deep"; const _: () = assert!(vectorcraft_doc::MAX_DEPTH == 32, "TOO_DEEP (and its catalog rows) names the cap"); @@ -1598,6 +1610,7 @@ impl Session { if !self.previewing && cmd::typing_in_progress(self) { cmd::settle_interaction(self)?; } + let guides_locked = self.menu.guides_locked; let st = self.doc_mut()?; let before = st.doc.clone(); let before_sel = st.selection.clone(); @@ -1645,6 +1658,10 @@ impl Session { match result { Ok(v) => { st.selection.prune(&st.doc); + // An edit that selects what it unlocks, shows or pastes leaves locked guides out. + if guides_locked { + deselect_guides(&st.doc, &mut st.selection); + } st.revision += 1; if st.interaction.is_none() { let selection = match st.history.undo.last() { @@ -1673,10 +1690,7 @@ impl Session { st.selection.prune(&st.doc); // Lock Guides locks the paths made into guides too (#1534), whatever selects them. if guides_locked { - let locked: Vec = st.selection.objects.iter().copied().filter(|id| is_guide_path(&st.doc, *id)).collect(); - for id in locked { - st.selection.remove(id); - } + deselect_guides(&st.doc, &mut st.selection); } // Selecting art makes its layer (or sublayer) the current one, as in the Layers panel of // the reference app: new art then goes beside it. @@ -1807,7 +1821,13 @@ impl Session { pub fn cancel_interaction(&mut self) -> Result<()> { self.pending_paint = None; - self.doc_mut()?.undo_interaction(); + let guides_locked = self.menu.guides_locked; + let st = self.doc_mut()?; + st.undo_interaction(); + // Guides locked during the interaction stay deselected. + if guides_locked { + deselect_guides(&st.doc, &mut st.selection); + } Ok(()) } @@ -1849,6 +1869,9 @@ impl Session { { st.doc = e.doc; st.selection = Arc::unwrap_or_clone(e.selection); + if self.menu.guides_locked { + deselect_guides(&st.doc, &mut st.selection); + } st.revision += 1; journal = Some(g.journal); } diff --git a/crates/engine/src/tests_menucmds.rs b/crates/engine/src/tests_menucmds.rs index 9af6d3c1e..4bc6f8f76 100644 --- a/crates/engine/src/tests_menucmds.rs +++ b/crates/engine/src/tests_menucmds.rs @@ -1860,6 +1860,139 @@ fn locked_guides_lock_guide_paths() { assert_eq!(selected(&s), vec![b]); } +/// A rectangle at (`x`, 0), 10 pt wide, made into a guide; nothing is selected after. +fn guide_path(s: &mut Session, x: f64) -> NodeId { + let g = rect(s, x, 0.0, 10.0, 10.0); + sel(s, &[g]); + s.execute("view.guides.make", &json!({})).unwrap(); + g +} + +/// Exit Isolation Mode selects the isolated art, except a guide path while Lock Guides is on, so +/// a move after it leaves the guide where it is. +#[test] +fn exit_isolation_leaves_a_locked_guide_path_deselected() { + let mut s = session(); + let g = guide_path(&mut s, 0.0); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.execute("object.isolate", &json!({"id": g.0})).unwrap(); + s.execute("object.exitIsolation", &json!({})).unwrap(); + assert!(selected(&s).is_empty()); + let _ = s.execute("object.move", &json!({"dx": 10, "dy": 0})); + assert_eq!(bounds(&s, g).x0, 0.0); + // Lock Guides off: the guide is selected. + s.execute("view.guides.lock", &json!({"locked": false})).unwrap(); + s.execute("object.isolate", &json!({"id": g.0})).unwrap(); + s.execute("object.exitIsolation", &json!({})).unwrap(); + assert_eq!(selected(&s), vec![g]); +} + +/// Undo and Redo bring back a selection without the guides, ruler guides or guide paths, that +/// Lock Guides has locked since, so a move after them leaves the guides where they are. +#[test] +fn undo_and_redo_leave_locked_guides_deselected() { + let mut s = session(); + let g = guide_path(&mut s, 0.0); + s.execute("guide.add", &json!({"vertical": true, "pos": 100})).unwrap(); + let select_both = |s: &mut Session| { + sel(s, &[g]); + s.execute("guide.select", &json!({"indexes": [0], "toggle": true})).unwrap(); + }; + select_both(&mut s); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + assert_eq!((bounds(&s, g).x0, guides(&s)), (10.0, vec![(true, 110.0)])); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!((selected(&s), selected_guides(&s)), (vec![], vec![]), "after Undo"); + let _ = s.execute("object.move", &json!({"dx": 10, "dy": 0})); + assert_eq!((bounds(&s, g).x0, guides(&s)), (0.0, vec![(true, 100.0)])); + // Undone with both selected, redone once they are locked. + s.execute("view.guides.lock", &json!({"locked": false})).unwrap(); + select_both(&mut s); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!((selected(&s), selected_guides(&s)), (vec![], vec![]), "after Redo"); + let _ = s.execute("object.move", &json!({"dx": 10, "dy": 0})); + assert_eq!((bounds(&s, g).x0, guides(&s)), (10.0, vec![(true, 110.0)])); +} + +/// Unlock All, Show All and Paste select the art they unlock, show or paste, except guide paths +/// while Lock Guides is on. +#[test] +fn unlock_all_show_all_and_paste_leave_locked_guide_paths_deselected() { + let mut s = session(); + let (g, h) = (guide_path(&mut s, 0.0), guide_path(&mut s, 20.0)); + sel(&mut s, &[g]); + s.execute("edit.copy", &json!({})).unwrap(); + s.execute("object.lock", &json!({})).unwrap(); + sel(&mut s, &[h]); + s.execute("object.hide", &json!({})).unwrap(); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + assert_eq!(s.execute("object.unlockAll", &json!({})).unwrap()["count"], 1); + assert!(selected(&s).is_empty(), "after Unlock All"); + assert_eq!(s.execute("object.showAll", &json!({})).unwrap()["count"], 1); + assert!(selected(&s).is_empty(), "after Show All"); + s.execute("edit.paste", &json!({})).unwrap(); + assert!(selected(&s).is_empty(), "after Paste"); + // Lock Guides off: the pasted guide is selected. + s.execute("view.guides.lock", &json!({"locked": false})).unwrap(); + s.execute("edit.paste", &json!({})).unwrap(); + let [copy] = selected(&s)[..] else { panic!("the pasted guide is selected") }; + assert!(crate::is_guide_path(&s.doc().unwrap().doc, copy)); +} + +/// Lock Guides deselects the guides of every open document, not only the active one's. +#[test] +fn lock_guides_deselects_the_guides_of_every_open_document() { + let mut s = session(); + let g = guide_path(&mut s, 0.0); + s.execute("guide.add", &json!({"vertical": true, "pos": 100})).unwrap(); + sel(&mut s, &[g]); + s.execute("guide.select", &json!({"indexes": [0], "toggle": true})).unwrap(); + s.execute("file.new", &json!({"width": 800, "height": 600})).unwrap(); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.execute("document.activate", &json!({"index": 0})).unwrap(); + assert_eq!((selected(&s), selected_guides(&s)), (vec![], vec![])); +} + +/// A failed batch and a canceled undo group bring back the selection from before them, without +/// the guide paths that Lock Guides locked inside them. +#[test] +fn a_failed_batch_and_a_canceled_undo_group_leave_locked_guides_deselected() { + let mut s = session(); + let g = guide_path(&mut s, 0.0); + sel(&mut s, &[g]); + let steps = json!([{"command": "view.guides.lock", "params": {"locked": true}}, {"command": "object.isolate", "params": {}}]); + assert!(s.execute("command.batch", &json!({"commands": steps})).is_err()); + assert!(s.guides_locked()); + assert!(selected(&s).is_empty(), "after the failed batch"); + s.execute("view.guides.lock", &json!({"locked": false})).unwrap(); + sel(&mut s, &[g]); + s.execute("edit.undoGroup.begin", &json!({})).unwrap(); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.execute("edit.undoGroup.end", &json!({"cancel": true})).unwrap(); + assert_eq!(bounds(&s, g).x0, 0.0); + assert!(selected(&s).is_empty(), "after the canceled undo group"); +} + +/// Canceling a drag or a dialog's preview brings back the selection from before it, without the +/// guide paths that Lock Guides locked during it. +#[test] +fn a_canceled_interaction_leaves_locked_guides_deselected() { + let mut s = session(); + let g = guide_path(&mut s, 0.0); + sel(&mut s, &[g]); + s.begin_interaction("Move").unwrap(); + s.preview("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + s.execute("view.guides.lock", &json!({"locked": true})).unwrap(); + s.cancel_interaction().unwrap(); + assert_eq!(bounds(&s, g).x0, 0.0); + assert!(selected(&s).is_empty()); +} + fn guides(s: &Session) -> Vec<(bool, f64)> { s.doc().unwrap().doc.guides.iter().map(|g| (g.vertical, g.pos)).collect() } diff --git a/docs/mcp.md b/docs/mcp.md index c48f2b99b..b2a23b9a0 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1078,7 +1078,9 @@ removes them) → `{selected}`. `guide.move {index, pos}` puts one guide somewhe moves the selected ones (vertical guides by `dx`, horizontal ones by `dy`; `copy` leaves them and selects the moved copies), and `guide.remove {index?}` deletes one guide or the selected ones → `{count}`; each is one undo step. With guides selected, `edit.clear` (Delete) deletes them and `object.nudge` (the arrow keys) nudges them. -View › Guides › Lock Guides (`view.guides.lock`) deselects them and refuses these commands until unlocked. +View › Guides › Lock Guides (`view.guides.lock`) deselects them and the paths made into guides, in every open +document, and refuses these commands until unlocked. While it is on, a command that selects art, such as `edit.undo`, +`edit.redo`, `object.exitIsolation`, `object.unlockAll`, `object.showAll` or `edit.paste`, leaves the guides out. `view.guides.release {index?, ids?}` (View › Guides › Release Guides) turns guides into paths → `{count}`. With `index` it releases that ruler guide, and with `ids` it releases those guide paths (and the guide paths inside them).