Fix #786: clear exact compound members and explicit targets

This commit is contained in:
DWAA1660
2026-10-09 11:33:23 -06:00
parent 04439bb01d
commit 636d9b24c8
5 changed files with 180 additions and 13 deletions
+1
View File
@@ -126,6 +126,7 @@ grade by behaviour against `plan/illustrator/`, not by whether a menu item exist
- **Shaper:** uneven squares and rectangles become live rectangles even when drawing begins mid-edge. Recognition uses uniformly spaced samples and closed-loop corners, fits rectangle edges/angles rather than requiring a nearly perfect bounding-box fill, and includes the pointer-release position. Rectangles/squares and ellipses snap to 45° steps, hexagons to 90°, and triangles point up or down; lines keep their free angle even with Shift. Shaper Groups retain the live originals, merge regions with the gesture-origin fill, punch fills independently from strokes, remove boundaries on exiting scribbles and trim protruding stroke pieces. Source moves/resizes re-evaluate the result in the same undo step; double-click enters construction editing. Native save/reopen, face recoloring, Release and Expand are covered. The construction-mode widget and finer gesture heuristics remain.
- **Selection:** Selection, Direct/Group Selection, Magic Wand, Lasso. A Shift-drag marquee toggles what it reaches: objects with the Selection tool, anchors with Direct and Group Selection (#483); the Lasso adds with Shift and subtracts with Alt. Dragging a path segment with Direct Selection bends it when it's curved (its anchors stay) and moves its two anchors when it's straight, one undo step; the fill, Shift or Alt still pick the whole path (Alt-drag copies it).
Agent selection commands validate every object id before changing the selection and report the resulting count and ids (#784).
Clear deletes individual selected compound-path members without deleting their siblings, and explicit object ids work without a selection; Cut still removes the same objects it copies (#786).
- **Saved selections:** Select ▸ Save Selection… asks for a name and keeps the selection in the document (saved with the file, undoable, up to 25); each one is listed at the bottom of the Select menu and a click selects its objects again; Edit Selection… lists them to rename or delete, applied together on OK. A file's saved selections are capped and checked against its objects when it opens. Not yet: Update Selection and the Layers panel's saved-selection menu.
- **Drawing:** Pen, Curvature, anchor tools, Pencil, Paintbrush, Blob Brush, Smooth, Path Eraser, Join. Direction handles dragged with Direct Selection or the Anchor Point tool snap to smart guides, and Shift keeps them at 45° steps from their anchor. Every point a drawing tool places snaps to Smart Guides through one snapper (#506): the Pen's and Curvature tool's anchors, the Pencil's ends, the shape tools' start and dragged corner or radius (Rectangle, Rounded Rectangle, Ellipse, Polygon, Star, Line Segment, Arc, Spiral, the grids), the Type tool's click and frame, a new artboard's corners.
- **Shapes:** all shape tools (including Flare) and the line, arc, spiral and grid tools. Space held while dragging out a shape, line, arc, spiral or grid moves it at its current size. While a grid is dragged out, Up/Down change its rows (concentric dividers) and Left/Right its columns (radial dividers).
+22 -12
View File
@@ -1,6 +1,6 @@
//! File (document-level) and Edit commands, plus document queries.
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
use std::sync::Arc;
use serde_json::{Value, json};
@@ -250,29 +250,39 @@ fn cut(s: &mut Session, p: &Value) -> Result<Value> {
if s.tool_id() == "artboard" {
return super::panelcmds::artboard_copy(s, &json!({}), true);
}
// Copy promotes a compound member to its compound. Cut must remove the same objects it
// copied, while Clear on its own deletes exactly the selected members.
let ids = selected_roots(s)?;
copy(s, p)?;
clear(s, &json!({}))
if s.doc()?.selection.anchors.is_empty() { clear(s, &json!({ "ids": ids })) } else { clear(s, &json!({})) }
}
fn clear(s: &mut Session, p: &Value) -> Result<Value> {
let explicit = if p.get("ids").is_some() { Some(checked_ids_param(s, p, "ids", "edit.clear")?) } else { None };
// Direct-selected anchors: delete those anchors instead of whole objects.
if ids_param(p, "ids").is_none() && !s.doc()?.selection.anchors.is_empty() {
if explicit.is_none() && !s.doc()?.selection.anchors.is_empty() {
return super::path::delete_anchors(s, p);
}
// Selected ruler guides (selected on their own).
if ids_param(p, "ids").is_none() && s.doc()?.selection.is_empty() {
if explicit.is_none() && s.doc()?.selection.is_empty() {
return super::docmenu::guide_remove(s, &json!({}));
}
let ids = match ids_param(p, "ids") {
Some(v) => v,
None => selected_roots(s)?,
};
let st = s.doc()?;
let targets = explicit.unwrap_or_else(|| st.selection.in_paint_order(&st.doc));
// Keep the selected ids themselves, without promoting compound members. Delete a selected
// ancestor just once; its selected descendants leave with it. Layers themselves are kept.
let targets: BTreeSet<_> = targets.into_iter().filter(|id| st.doc.node(*id).is_some_and(|n| !n.is_layer())).collect();
let ids: Vec<_> = targets
.iter()
.copied()
.filter(|id| !st.doc.ancestry(*id).unwrap_or_default().iter().rev().skip(1).any(|ancestor| targets.contains(ancestor)))
.collect();
if ids.is_empty() {
return ok();
}
s.edit("Clear", |d, sel| {
for id in &ids {
if d.node(*id).is_some_and(|n| n.is_layer()) {
continue;
}
let _ = d.remove(*id);
d.remove(*id)?;
}
sel.clear();
Ok(())
+4 -1
View File
@@ -1055,7 +1055,10 @@ impl Session {
/// Execute a command by id. This is THE entry point for every frontend.
pub fn execute(&mut self, id: &str, params: &Value) -> Result<Value> {
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?;
if let Err(why) = (spec.enabled)(self) {
// Clear's menu needs a selection; an agent supplying explicit targets only needs a
// document. The command validates those targets before making any edit.
let enabled = if id == "edit.clear" && params.get("ids").is_some() { cmd::has_doc(self) } else { (spec.enabled)(self) };
if let Err(why) = enabled {
return Err(EngineError::Disabled(id.to_string(), why));
}
// Only top-level commands are journaled (commands that call other commands would otherwise
+145
View File
@@ -0,0 +1,145 @@
//! Clear deletes its exact targets, including compound members (#786).
#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
use serde_json::json;
use vectorcraft_doc::NodeId;
use vectorcraft_engine::{EngineError, Session};
fn compound() -> (Session, NodeId, NodeId, NodeId) {
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
let mut rectangle =
|x| NodeId(s.execute("shape.rectangle", &json!({"x": x, "y": 10, "width": 30, "height": 30})).unwrap()["id"].as_u64().unwrap());
let a = rectangle(10);
let b = rectangle(50);
s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap();
let c = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap());
(s, a, b, c)
}
#[test]
fn clear_selected_compound_member_preserves_its_sibling_and_is_undoable() {
let (mut s, a, b, c) = compound();
s.execute("select.set", &json!({"ids": [a.0]})).unwrap();
s.execute("edit.clear", &json!({})).unwrap();
assert!(s.active().unwrap().doc.node(a).is_none());
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
assert!(s.active().unwrap().selection.is_empty());
assert_eq!(s.execute("edit.undo", &json!({})).unwrap()["undone"], "Clear");
assert_eq!(s.active().unwrap().doc.parent_of(a), Some(c));
assert_eq!(s.active().unwrap().selection.objects, vec![a]);
s.execute("edit.redo", &json!({})).unwrap();
assert!(s.active().unwrap().doc.node(a).is_none());
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
#[test]
fn clear_explicit_ids_works_without_a_selection_and_overrides_selected_anchors() {
let (mut s, a, b, c) = compound();
s.execute("select.none", &json!({})).unwrap();
let info = s.commands().into_iter().find(|info| info.id == "edit.clear").unwrap();
assert!(!info.enabled, "the menu stays disabled with no selection");
s.execute("edit.clear", &json!({"ids": [a.0]})).unwrap();
assert!(s.active().unwrap().doc.node(a).is_none());
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
s.execute("edit.undo", &json!({})).unwrap();
s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, 0]]})).unwrap();
let sibling = s.active().unwrap().doc.node(b).unwrap().clone();
s.execute("edit.clear", &json!({"ids": [a.0]})).unwrap();
assert!(s.active().unwrap().doc.node(a).is_none());
assert_eq!(s.active().unwrap().doc.node(b).unwrap(), &sibling);
}
#[test]
fn clear_explicit_targets_are_validated_atomically() {
let (mut s, a, _, _) = compound();
s.execute("select.none", &json!({})).unwrap();
for ids in [json!([a.0, 999]), json!([a.0, -1]), json!([a.0, "bad"]), json!([a.0, 2.5]), json!(null), json!(a.0)] {
let doc = s.active().unwrap().doc.clone();
let journal = s.journal.clone();
assert!(matches!(s.execute("edit.clear", &json!({"ids": ids})), Err(EngineError::BadParams { .. })), "{ids}");
assert_eq!(s.active().unwrap().doc, doc);
assert_eq!(s.journal, journal);
}
assert!(s.execute("edit.clear", &json!({})).is_err());
assert!(Session::new().execute("edit.clear", &json!({"ids": [a.0]})).is_err());
}
#[test]
fn clear_compound_and_member_together_deletes_the_compound() {
for explicit in [false, true] {
let (mut s, a, b, c) = compound();
s.execute("select.set", &json!({"ids": [a.0, c.0]})).unwrap();
s.execute("edit.clear", &if explicit { json!({"ids": [a.0, c.0, c.0]}) } else { json!({}) }).unwrap();
for id in [a, b, c] {
assert!(s.active().unwrap().doc.node(id).is_none());
}
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.active().unwrap().doc.parent_of(a), Some(c));
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
}
#[test]
fn cut_compound_member_keeps_copy_and_delete_targets_consistent() {
let (mut s, a, b, c) = compound();
s.execute("select.set", &json!({"ids": [a.0]})).unwrap();
s.execute("edit.cut", &json!({})).unwrap();
assert_eq!(s.clipboard.nodes.len(), 1);
assert_eq!(s.clipboard.nodes[0].id, c);
assert!(s.active().unwrap().doc.node(c).is_none());
s.execute("edit.pasteInPlace", &json!({})).unwrap();
let pasted = s.active().unwrap().selection.objects[0];
assert_eq!(s.active().unwrap().doc.node(pasted).unwrap().children().unwrap().len(), 2);
s.execute("edit.undo", &json!({})).unwrap();
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.active().unwrap().doc.parent_of(a), Some(c));
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
#[test]
fn clear_direct_selected_anchors_keeps_the_rest_of_the_path() {
let (mut s, a, b, c) = compound();
s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]]})).unwrap();
s.execute("edit.clear", &json!({})).unwrap();
assert_eq!(s.active().unwrap().doc.node(a).unwrap().path_data().unwrap().anchor_count(), 3);
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
#[test]
fn clear_guides_and_empty_explicit_targets_keep_their_separate_behavior() {
let (mut s, a, _, _) = compound();
s.execute("guide.add", &json!({"vertical": true, "pos": 10})).unwrap();
s.execute("guide.select", &json!({"indexes": [0]})).unwrap();
let history = s.active().unwrap().history.undo.len();
let selection = s.active().unwrap().selection.clone();
s.execute("edit.clear", &json!({"ids": []})).unwrap();
assert_eq!(s.active().unwrap().history.undo.len(), history);
assert_eq!(s.active().unwrap().selection, selection);
assert_eq!(s.active().unwrap().doc.guides.len(), 1);
s.execute("edit.clear", &json!({})).unwrap();
assert!(s.active().unwrap().doc.guides.is_empty());
assert!(s.active().unwrap().doc.node(a).is_some());
s.execute("edit.undo", &json!({})).unwrap();
assert_eq!(s.active().unwrap().doc.guides.len(), 1);
}
#[test]
fn clear_keeps_layers_but_deletes_an_explicitly_targeted_child() {
let (mut s, a, b, c) = compound();
let layer = s.active().unwrap().doc.layers[0].id;
s.execute("select.none", &json!({})).unwrap();
s.execute("edit.clear", &json!({"ids": [layer.0, a.0]})).unwrap();
assert!(s.active().unwrap().doc.node(layer).is_some());
assert!(s.active().unwrap().doc.node(a).is_none());
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
#[test]
fn cut_direct_selected_anchors_retains_anchor_deletion() {
let (mut s, a, b, c) = compound();
s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]]})).unwrap();
s.execute("edit.cut", &json!({})).unwrap();
assert_eq!(s.active().unwrap().doc.node(a).unwrap().path_data().unwrap().anchor_count(), 3);
assert_eq!(s.active().unwrap().doc.parent_of(b), Some(c));
}
+8
View File
@@ -621,6 +621,14 @@ A malformed or unknown id is an error naming that value; the entire selection is
All three commands return `{count, ids}` for the resulting selection, in selection order.
An empty array clears the selection for `select.set` and leaves it alone for Add and Toggle.
`edit.clear {ids?}` deletes the exact selected objects, including individual compound-path members;
their unselected siblings remain in the compound. Supplying `ids` works without a selection and
overrides selected anchors or guides. Every explicit id is validated before any deletion; a malformed
or unknown id leaves the document unchanged. An empty `ids` array does nothing. Layers themselves
are kept, and selecting an ancestor together with its descendants deletes that subtree once.
Without `ids`, direct-selected anchors or selected ruler guides retain their usual Clear behavior.
Cut still removes the objects it copied, including a whole compound when a member is selected.
## Saved selections
Select → Save Selection… keeps the selected objects under a name, in the document: `select.save {name?}` (default