Review: guides on hidden or locked layers, and through layer commands

- Guides on a hidden or locked layer leave the selection (Selection::prune,
  run after every edit and selection change), their Layers rows neither
  select nor drag them, and guide.move / guide.remove / guide.setLayer
  refuse them; guide.setLayer refuses a locked target layer, as dragged
  rows are refused there.
- A new guide goes where new art goes (the current layer, else the top
  shown and unlocked one; refused when there is none), so it never lands
  on a hidden layer and vanishes.
- Layer commands keep guides with their layers: Duplicate (and Alt-drag
  copies of rows) copy a layer's guides onto the copy, Merge Selected and
  Flatten Artwork put the merged layers' guides on the merged layer and
  Flatten discards a hidden layer's. One shared doc predicate for "the
  layer a guide is on"; the deleted-layer cleanup moves into the doc.
- Tests: engine tests for the above and their undo, a doc check that an
  old guide reads with no layer and none is written, and a headless
  Layers panel test of guide rows (select, drag to another layer, locked).
- Translations for the new message; docs and a ROADMAP progress line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bflatastic
2026-10-11 06:03:48 +07:00
co-authored by Claude Opus 5.5
parent 2d2e0c6992
commit c183c47df3
17 changed files with 206 additions and 35 deletions
+27 -9
View File
@@ -516,10 +516,12 @@ fn guide_add(s: &mut Session, p: &Value) -> Result<Value> {
}
None => None,
};
let st = s.doc()?;
let layer = match id_param(p, "layer") {
Some(l) if s.doc()?.doc.node(l).is_some_and(Node::is_layer) => Some(l),
Some(l) if st.doc.node(l).is_some_and(Node::is_layer) => Some(l),
Some(_) => return Err(bad("guide.add", "layer must name a layer or sublayer")),
None => s.doc()?.current_layer(),
// Where new art goes (refused when no layer is shown and unlocked), or that group's layer.
None => st.target_parent()?.and_then(|p| if st.doc.node(p).is_some_and(Node::is_layer) { Some(p) } else { st.doc.layer_containing(p) }),
};
let i = s.edit("New Guide", |d, _| {
d.guides.push(Guide { artboard, layer, ..Guide::new(vertical, pos) });
@@ -554,6 +556,16 @@ fn guide_index(s: &Session, v: Option<&Value>, cmd: &str) -> Result<usize> {
Ok(i)
}
/// [`guide_index`] of a guide that can be moved or deleted: not on a hidden or locked layer.
fn editable_guide_index(s: &Session, v: Option<&Value>, cmd: &str) -> Result<usize> {
let i = guide_index(s, v, cmd)?;
let d = &s.doc()?.doc;
if !d.guides.get(i).is_some_and(|g| d.guide_editable(g)) {
return Err(EngineError::Other("the guide is on a hidden or locked layer".into()));
}
Ok(i)
}
fn guide_select(s: &mut Session, p: &Value) -> Result<Value> {
let list = p.get("indexes").and_then(Value::as_array).ok_or_else(|| bad("guide.select", "missing indexes"))?;
let picked = list.iter().map(|v| guide_index(s, Some(v), "guide.select")).collect::<Result<Vec<_>>>()?;
@@ -573,7 +585,7 @@ fn guide_select(s: &mut Session, p: &Value) -> Result<Value> {
/// Delete ruler guide `index`, else the selected guides, in one undo step → {count}.
pub(crate) fn guide_remove(s: &mut Session, p: &Value) -> Result<Value> {
let gone: Vec<usize> = match p.get("index") {
Some(v) => vec![guide_index(s, Some(v), "guide.remove")?],
Some(v) => vec![editable_guide_index(s, Some(v), "guide.remove")?],
None => s.doc()?.selection.guides.clone(),
};
if gone.is_empty() {
@@ -589,15 +601,21 @@ pub(crate) fn guide_remove(s: &mut Session, p: &Value) -> Result<Value> {
/// Put ruler guide `index`, else the selected guides, on layer `layer`, in one undo step.
fn guide_set_layer(s: &mut Session, p: &Value) -> Result<Value> {
let layer = id_param(p, "layer").filter(|l| s.doc().is_ok_and(|st| st.doc.node(*l).is_some_and(Node::is_layer)));
let layer = layer.ok_or_else(|| bad("guide.setLayer", "layer must name a layer or sublayer"))?;
const C: &str = "guide.setLayer";
let d = &s.doc()?.doc;
let layer =
id_param(p, "layer").filter(|l| d.node(*l).is_some_and(Node::is_layer)).ok_or_else(|| bad(C, "layer must name a layer or sublayer"))?;
// As objects dragged in the Layers panel: never onto a locked layer.
if super::layer::locked_within(d, layer) {
return Err(EngineError::Other("the layer or group is locked".into()));
}
let moving: Vec<usize> = match (p.get("index"), p.get("indexes").and_then(Value::as_array)) {
(Some(v), _) => vec![guide_index(s, Some(v), "guide.setLayer")?],
(None, Some(list)) => list.iter().take(10_000).map(|v| guide_index(s, Some(v), "guide.setLayer")).collect::<Result<Vec<_>>>()?,
(Some(v), _) => vec![editable_guide_index(s, Some(v), C)?],
(None, Some(list)) => list.iter().take(10_000).map(|v| editable_guide_index(s, Some(v), C)).collect::<Result<Vec<_>>>()?,
(None, None) => s.doc()?.selection.guides.clone(),
};
if moving.is_empty() {
return Err(bad("guide.setLayer", "no guide selected (or give an index)"));
return Err(bad(C, "no guide selected (or give an index)"));
}
s.edit("Move Guide to Layer", |d, _| {
for i in &moving {
@@ -613,7 +631,7 @@ fn guide_set_layer(s: &mut Session, p: &Value) -> Result<Value> {
/// Put ruler guide `index` at `pos`, else move (or copy) the selected guides, in one undo step.
pub(crate) fn guide_move(s: &mut Session, p: &Value) -> Result<Value> {
if let Some(v) = p.get("index") {
let i = guide_index(s, Some(v), "guide.move")?;
let i = editable_guide_index(s, Some(v), "guide.move")?;
let pos = f64_req(p, "pos", "guide.move")?;
s.edit("Move Guide", |d, _| {
if let Some(g) = d.guides.get_mut(i) {
+8 -4
View File
@@ -221,7 +221,7 @@ pub(crate) fn accepts(d: &Document, parent: Option<NodeId>, n: &Node) -> bool {
}
/// Whether `id` or a container around it is locked.
fn locked_within(d: &Document, id: NodeId) -> bool {
pub(crate) fn locked_within(d: &Document, id: NodeId) -> bool {
d.ancestry(id).unwrap_or_default().iter().any(|a| d.node(*a).is_some_and(|n| n.locked))
}
@@ -463,8 +463,7 @@ fn delete_rows(s: &mut Session, p: &Value) -> Result<Value> {
d.remove(*id)?;
}
// The guides on the deleted layers go with them.
let orphans: Vec<usize> = d.guides.iter().enumerate().filter(|(_, g)| g.layer.is_some_and(|l| d.node(l).is_none())).map(|(i, _)| i).collect();
d.retain_guides(sel, |i, _| !orphans.contains(&i));
d.drop_guides_of_deleted_layers(sel);
sel.prune(d);
Ok(())
})?;
@@ -490,6 +489,7 @@ fn duplicate_rows(s: &mut Session, p: &Value) -> Result<Value> {
let mut c = d.reid(&n);
if n.is_layer() {
c.name = Some(format!("{} copy", n.display_name()));
d.copy_layer_guides(&n, &c);
}
out.push(d.insert(par, idx + 1, c)?);
}
@@ -682,7 +682,11 @@ fn move_rows(s: &mut Session, p: &Value) -> Result<Value> {
for id in &ids {
nodes.push(if copy {
let n = d.node(*id).cloned().ok_or(EngineError::NoNode(*id))?;
d.reid(&n)
let c = d.reid(&n);
if n.is_layer() {
d.copy_layer_guides(&n, &c);
}
c
} else {
Arc::unwrap_or_clone(d.remove(*id)?)
});
+4
View File
@@ -263,6 +263,8 @@ fn merge_into(d: &mut Document, into: NodeId, others: &[NodeId]) -> Result<()> {
all.extend(taken);
all.sort_by(|a, b| keys.get(&a.id).cmp(&keys.get(&b.id)));
*ch = all;
// The merged layers' guides go to it too.
d.rehome_guides(into);
Ok(())
}
@@ -311,6 +313,8 @@ fn flatten(s: &mut Session, p: &Value) -> Result<Value> {
for h in &hidden {
d.remove(*h)?;
}
// Hidden layers are discarded with their guides.
d.drop_guides_of_deleted_layers(sel);
merge_into(d, into, &others)?;
sel.prune(d);
Ok(())