Merge remote-tracking branch 'origin/main' into feat/996-inspect-fields

# Conflicts:
#	docs/roadmap.md

crates/engine/src/auto_layout_move_tests.rs

@@ -0,0 +1,59 @@
+//! Moving an auto layout frame (#1571): a frame that only moves keeps the layers inside it where
+//! they are, so a drag doesn't lay its contents out again on every pointer move; a change that can
+//! move or resize them still does, and its parent's layout still follows it.
+
+use figcraft_doc::NodeId;
+use serde_json::json;
+
+use crate::Session;
+use crate::params::id_of;
+
+/// Two rectangles wrapped in an auto layout frame: the session, the frame and the rectangles.
+fn stack() -> (Session, NodeId, [NodeId; 2]) {
+    let mut s = Session::new();
+    let mut make = |y| id_of(&s.execute("shape.rectangle", &json!({"x": 0, "y": y, "width": 100, "height": 40})).unwrap()["id"]).unwrap();
+    let kids = [make(0), make(60)];
+    s.execute("select.set", &json!({"ids": kids})).unwrap();
+    s.execute("layout.add", &json!({})).unwrap();
+    let frame = s.doc().unwrap().node(kids[0]).and_then(|n| n.parent).unwrap();
+    (s, frame, kids)
+}
+
+fn local_y(s: &Session, id: NodeId) -> f64 {
+    s.doc().unwrap().node(id).unwrap().transform.translation().y
+}
+
+#[test]
+fn moving_an_auto_layout_frame_leaves_its_contents_alone() {
+    let (mut s, frame, [_, second]) = stack();
+    assert!(s.doc().unwrap().node(frame).unwrap().layout.is_some());
+    let laid_out = local_y(&s, second);
+    // Put the second rectangle out of place without anything noticing (as if the layout had
+    // never run on it), then move the frame: its contents aren't laid out again.
+    {
+        let doc = s.ds_mut().unwrap().doc_mut();
+        doc.node_mut(second).unwrap().transform = figcraft_doc::geom::Affine::translate((0.0, laid_out + 500.0));
+        doc.take_dirty();
+    }
+    s.execute("node.set", &json!({"ids": [frame.0], "props": {"x": 300, "y": 200}})).unwrap();
+    assert_eq!(local_y(&s, second), laid_out + 500.0, "a move doesn't lay the frame out");
+    // A change that can move its contents does.
+    s.execute("node.set", &json!({"ids": [frame.0], "props": {"width": 160}})).unwrap();
+    assert_eq!(local_y(&s, second), laid_out, "a resize lays it out");
+}
+
+#[test]
+fn a_frame_moved_inside_auto_layout_still_moves_its_siblings() {
+    // A stack and a rectangle side by side in an outer auto layout frame: the outer layout still
+    // follows when the stack grows.
+    let (mut s, inner, [first, _]) = stack();
+    let other = id_of(&s.execute("shape.rectangle", &json!({"x": 300, "y": 0, "width": 100, "height": 40})).unwrap()["id"]).unwrap();
+    s.execute("select.set", &json!({"ids": [inner.0, other.0]})).unwrap();
+    s.execute("layout.add", &json!({})).unwrap();
+    let outer = s.doc().unwrap().node(inner).and_then(|n| n.parent).unwrap();
+    assert!(s.doc().unwrap().node(outer).unwrap().layout.is_some());
+    let before = s.doc().unwrap().world_bounds(other).unwrap();
+    s.execute("node.set", &json!({"ids": [first.0], "props": {"width": 200}})).unwrap();
+    let after = s.doc().unwrap().world_bounds(other).unwrap();
+    assert!(after.x0 > before.x0, "the outer layout followed the inner frame's growth: {before:?} {after:?}");
+}

crates/engine/src/boolean_cap_tests.rs

@@ -0,0 +1,39 @@
+//! A boolean group past the path operations' caps: it stays a boolean group, flattening it is
+//! refused with the reason, and the document is unchanged.
+
+use figcraft_doc::{NodeKind, VectorNetwork};
+use figcraft_geom::kurbo::{BezPath, Rect, Shape};
+use serde_json::json;
+
+use crate::Session;
+use crate::params::id_of;
+
+/// `n` long thin bars, across or down: 750 of each cross 2.25 million times, past the caps.
+fn bars(n: usize, across: bool) -> BezPath {
+    let mut p = BezPath::new();
+    for i in 0..n {
+        let (at, len) = (i as f64 * 4.0, n as f64 * 4.0);
+        let r = if across { Rect::new(0.0, at, len, at + 2.0) } else { Rect::new(at, 0.0, at + 2.0, len) };
+        p.extend(r.path_elements(0.1));
+    }
+    p
+}
+
+#[test]
+fn flattening_a_boolean_past_the_caps_is_refused() {
+    let mut s = Session::new();
+    let page = s.page().unwrap();
+    let ids: Vec<_> = [true, false]
+        .into_iter()
+        .map(|across| {
+            let network = VectorNetwork::from_path(&bars(750, across));
+            s.doc_mut().unwrap().create(page, NodeKind::Vector { network }, "Bars", 0.0, 0.0, 3000.0, 3000.0).unwrap()
+        })
+        .collect();
+    let g = id_of(&s.execute("bool.union", &json!({"ids": ids})).unwrap()["id"]).unwrap();
+    let e = s.execute("vector.flatten", &json!({"ids": [g]})).unwrap_err().to_string();
+    assert!(e.contains("too complex to combine"), "{e}");
+    let doc = s.doc().unwrap();
+    assert!(matches!(doc.node(g).map(|n| &n.kind), Some(NodeKind::BooleanOp { .. })), "the group is kept");
+    assert_eq!(doc.node(g).map(|n| n.children.len()), Some(2));
+}

crates/engine/src/canvas/tools.rs

@@ -557,6 +557,9 @@ pub struct ToolState {
     pub dev_mode: bool,
     /// Pen snap targets, kept while hovering (see `vectoredit::point_snap`).
     pub(crate) pen_targets: Option<crate::vectoredit::point_snap::PenTargets>,
+    /// Vector edit mode's pen: where the last press that started something new (no path being
+    /// drawn) was, and the undo depth before it, so a double-click there takes that edit back.
+    pub(crate) pen_press: Option<(Point, usize)>,
     /// The Scale tool's anchor: column and row (0 start, 1 middle, 2 end) of the point of the
     /// selection box that stays put (`tool.scaleAnchor`).
     pub scale_anchor: (u8, u8),

crates/engine/src/canvas/tools/click.rs

@@ -26,7 +26,7 @@ impl Session {
         let ds = self.ds()?;
         let (doc, sel, page) = (ds.doc.clone(), ds.selection.clone(), ds.page);
         if self.tool.tool == ToolId::Pen {
-            return self.pen_finish();
+            return self.pen_double_click(ev);
         }
         let edits = !self.tool.dev_mode;
         // A frame's or section's name label: rename it there (the canvas puts a field over it).

crates/engine/src/canvas/tools/keys.rs

@@ -8,6 +8,11 @@ impl Session {
         if self.tool.dev_mode {
             return Ok(false);
         }
+        // Enter while the pen draws ends the path and edits it with the Move tool (#1430).
+        if name == "Enter" && !alt && !cmd && self.tool.pen.is_some() {
+            self.pen_enter()?;
+            return Ok(true);
+        }
         if self.ds().is_ok_and(|d| d.vector_edit.is_some()) {
             return crate::vectoredit::key(self, name, shift, alt, cmd);
         }

crates/engine/src/canvas/tools/pen.rs

@@ -3,6 +3,7 @@ use super::{
     Affine, Document, Gesture, Guide, Node, NodeId, NodeKind, PenPath, Point, PointerEvent, Result, Session, StrokeAlign, ToolId, Vec2,
     VectorNetwork, at_45_degrees, frame_at, json, pen_handles,
 };
+use crate::vectoredit::VTool;
 
 /// Pen pressure along drawn samples as width profile points `[t, scale]`: `t` by length, at most
 /// 32 points, medium pressure (0.5) at the stroke weight, from a tenth to twice it. `None` when no
@@ -84,7 +85,7 @@ impl Session {
         if self.tool.pen.is_none()
             && let Some((id, v)) = selected_end_at(&doc, &self.selection(), ev.p(), 8.0 / ev.zoom.max(1e-6))
         {
-            return self.pen_edit(id, Some(v));
+            return self.edit_vector_with(id, VTool::Pen, Some(v));
         }
         let pen = self.tool.pen.get_or_insert_with(PenPath::default);
         // Clicking the first anchor closes the path.
@@ -103,6 +104,10 @@ impl Session {
             self.pen_update()?;
             return self.pen_finish();
         }
+        // A press on the last anchor (a double-click's second press) adds no point on top of it.
+        if pen.points.last().is_some_and(|(q, _, _)| near(q)) {
+            return Ok(());
+        }
         if pen.parent.is_none() {
             pen.parent = Some(frame_at(&doc, page, p, &[]).unwrap_or(page));
         }
@@ -200,29 +205,59 @@ impl Session {
         Ok(())
     }
 
-    /// Edit `id` as a vector with the pen, going on from point `anchor` (or, without one, starting
-    /// a new path in the same layer at the next click).
-    fn pen_edit(&mut self, id: NodeId, anchor: Option<usize>) -> Result<()> {
+    /// Edit `id` as a vector with `tool` (the pen going on from point `anchor`, or, without one,
+    /// starting a new path in the same layer at the next click).
+    fn edit_vector_with(&mut self, id: NodeId, tool: VTool, anchor: Option<usize>) -> Result<()> {
         self.set_tool(ToolId::Move);
         crate::vectoredit::begin(self, id)?;
-        crate::vectoredit::set_tool(self, crate::vectoredit::VTool::Pen)?;
+        crate::vectoredit::set_tool(self, tool)?;
         crate::vectoredit::set_state(self, |e| {
             e.anchor = anchor;
             e.vertices = anchor.into_iter().collect();
         })
     }
 
-    /// Enter/Escape while the pen draws: the path ends but its layer stays in edit mode with the
-    /// pen, so the next path joins the same vector (another Escape leaves edit mode).
+    /// Escape while the pen draws: the path ends but its layer stays in edit mode with the pen, so
+    /// the next path joins the same vector (another Escape leaves edit mode).
     pub fn pen_end(&mut self) -> Result<()> {
+        self.end_pen_path(VTool::Pen)
+    }
+
+    /// Enter (or a double-click) while the pen draws: the path ends and its layer is edited with
+    /// the Move tool, its points and handles selectable.
+    pub fn pen_enter(&mut self) -> Result<()> {
+        self.end_pen_path(VTool::Move)
+    }
+
+    /// End the pen's path and edit its layer with `tool` (nothing to edit before a segment).
+    fn end_pen_path(&mut self, tool: VTool) -> Result<()> {
         let id = self.tool.pen.as_ref().and_then(|p| p.id);
         self.pen_finish()?;
         match id {
-            Some(id) if self.doc()?.contains(id) => self.pen_edit(id, None),
+            Some(id) if self.doc()?.contains(id) => self.edit_vector_with(id, tool, None),
             _ => Ok(()),
         }
     }
 
+    /// A double-click with the pen (#1430). While a path is drawn it ends there, as Enter does.
+    /// Otherwise the double-click only started a path (its second press adds nothing): that is
+    /// dropped, and the layer under the pointer is edited with the Move tool, as a double-click on
+    /// it with the Move tool would.
+    pub(super) fn pen_double_click(&mut self, ev: &PointerEvent) -> Result<()> {
+        if self.tool.pen.as_ref().is_some_and(|p| p.id.is_some()) {
+            return self.pen_enter();
+        }
+        self.pen_finish()?;
+        let (doc, page) = {
+            let ds = self.ds()?;
+            (ds.doc.clone(), ds.page)
+        };
+        match self.hit(&doc, page, ev.p(), ev.tol() * 2.0).filter(|id| crate::vectoredit::enter_targets(&doc, &[*id]).is_some()) {
+            Some(id) => self.edit_vector_with(id, VTool::Move, None),
+            None => Ok(()),
+        }
+    }
+
     /// Finish the pen path (and keep the pen tool: double-click, Delete, a tool switch).
     pub fn pen_finish(&mut self) -> Result<()> {
         let pen = self.tool.pen.take();

crates/engine/src/canvas/vectoredit.rs

@@ -741,13 +741,10 @@ pub fn key(s: &mut Session, name: &str, shift: bool, alt: bool, cmd: bool) -> Re
     }
     match (name, shift, alt, cmd) {
         ("Escape", ..) => escape(s)?,
-        ("Enter", ..) => {
-            if ve.anchor.is_some() {
-                escape(s)?;
-            } else {
-                end(s)?;
-            }
-        }
+        // With the pen, Enter ends its path and goes on editing with the Move tool (#1430); the next
+        // Enter leaves edit mode.
+        ("Enter", ..) if ve.tool == VTool::Pen => set_tool(s, VTool::Move)?,
+        ("Enter", ..) => end(s)?,
         ("Backspace" | "Delete", false, false, false) if ve.tool == VTool::Width => {
             // The selected width point (nothing else is selected with the Variable width tool).
             if let Some(i) = ve.width_point {

crates/engine/src/canvas/vectoredit/pointer.rs

@@ -205,6 +205,7 @@ impl Session {
             PointerKind::Down => self.vector_down(&ve, ev),
             PointerKind::Move => self.vector_drag(&ve, ev).map(|()| true),
             PointerKind::Up => self.vector_up(&ve, ev).map(|()| true),
+            PointerKind::DoubleClick if ve.tool == VTool::Pen => self.pen_double_click_editing(ve.id, ev).map(|()| true),
             PointerKind::DoubleClick => {
                 let tol = 6.0 / ev.zoom.max(1e-6);
                 let doc = self.doc()?.clone();
@@ -218,6 +219,21 @@ impl Session {
         }
     }
 
+    /// A double-click with the pen in edit mode (#1430): the path ends and the layer `id` goes on
+    /// being edited with the Move tool. When the double-click began something new (a point split
+    /// into a segment, or a new path's first point), that edit of its first press is taken back.
+    fn pen_double_click_editing(&mut self, id: NodeId, ev: &PointerEvent) -> Result<()> {
+        if let Some((at, depth)) = self.tool.pen_press.take()
+            && (at - ev.p()).hypot() * ev.zoom < 8.0
+        {
+            while self.undo_depth().0 > depth && self.undo()? {}
+        }
+        if editing(self).is_err() {
+            super::begin(self, id)?;
+        }
+        super::set_tool(self, VTool::Move)
+    }
+
     fn vgesture(&mut self, g: VGesture) {
         self.tool.gesture = Gesture::Vector(g);
     }
@@ -743,6 +759,7 @@ impl Session {
             VTool::Pen => {
                 let at = if h.is_none() && !ev.shift { self.snap_pen(ev, Some(id), &[])?.0 } else { ev.p() };
                 let Some(plan) = self.pen_plan(ve, h, w.inverse() * at, ev.shift) else { return Ok(true) };
+                self.tool.pen_press = ve.anchor.is_none().then(|| (ev.p(), self.undo_depth().0));
                 self.begin_interaction("Pen")?;
                 let out = ve.pen_out;
                 let mut v = None;

crates/engine/src/cmd.rs

@@ -65,6 +65,9 @@ struct Registry {
     index: HashMap<&'static str, usize>,
     /// The parameter names each command's doc lists (`list` order; see [`doc_param_names`]).
     names: Vec<Option<Vec<&'static str>>>,
+    /// Whether each command's doc lists a parameter it can't run without (`list` order; see
+    /// [`doc_needs_params`]).
+    needs: Vec<bool>,
 }
 
 fn registry() -> &'static Registry {
@@ -127,7 +130,8 @@ fn registry() -> &'static Registry {
             }
         }
         let names = list.iter().map(|s| doc_param_names(s.params)).collect();
-        Registry { list, index, names }
+        let needs = list.iter().map(|s| doc_needs_params(s.params)).collect();
+        Registry { list, index, names, needs }
     })
 }
 
@@ -147,6 +151,25 @@ pub fn param_names(id: &str) -> Option<&'static [&'static str]> {
     r.index.get(id).and_then(|i| r.names.get(*i)).and_then(|n| n.as_deref())
 }
 
+/// Whether command `id` can't run without parameters: its doc lists one that isn't optional (no
+/// `?`), so a call with none is refused (`image.place` needs `data` or `path`). A UI that runs
+/// commands without parameters (the Actions palette) leaves these out. `false` for an unknown id
+/// or a doc that isn't a `{…}` list.
+pub fn needs_params(id: &str) -> bool {
+    let r = registry();
+    r.index.get(id).and_then(|i| r.needs.get(*i)).is_some_and(|n| *n)
+}
+
+/// Whether a `{…}` params doc lists an item whose name has no `?` (`{data (base64) | path, x?}`:
+/// yes; `{ids?, direction? (h|v)}`, `{}` or a doc of another form: no).
+fn doc_needs_params(doc: &'static str) -> bool {
+    let required = |item: &str| {
+        let first = item.trim().split(|c: char| c.is_whitespace() || c == '(').next().unwrap_or_default();
+        first.starts_with(|c: char| c.is_ascii_alphabetic() || c == '_') && !first.contains('?')
+    };
+    doc_items(doc).is_some_and(|items| items.into_iter().any(required))
+}
+
 /// Refuse params naming something the command doesn't take: a misspelt name would otherwise be
 /// ignored and the command answer as if it hadn't been given. `ids` and `id` are always taken:
 /// the layers a command acts on (`params::targets` reads either).

crates/engine/src/cmd/comment.rs

@@ -332,17 +332,79 @@ fn mentionable<'a>(people: &'a [String], user: &'a str) -> Vec<&'a str> {
     people.iter().map(String::as_str).chain([user]).collect()
 }
 
-/// A message as JSON, with the names it @mentions: `{name, known, you}` each, resolved against
-/// `people` (`mentionable`); `known: false` is a name nobody in the file has.
-fn message_json(m: &Message, people: &[&str], user: &str) -> Value {
-    let mut named: Vec<Value> = Vec::new();
-    for (r, who) in mentions(&m.text, people) {
-        let name = who.or_else(|| m.text.get(r.start + 1..r.end)).unwrap_or_default();
-        let v = json!({"name": name, "known": who.is_some(), "you": who.is_some_and(|w| w.eq_ignore_ascii_case(user))});
+/// The names `text` @mentions by the rule the thread view highlights them with
+/// ([`figcraft_doc::mentions`]), and whether each is one of `people` (else it is the letters and
+/// digits after the `@`).
+fn mentioned<'a>(text: &'a str, people: &[&'a str]) -> impl Iterator<Item = (&'a str, bool)> {
+    mentions(text, people).into_iter().map(move |(r, who)| (who.or_else(|| text.get(r.start + 1..r.end)).unwrap_or_default(), who.is_some()))
+}
+
+/// Whether a message of `c` @mentions `name` (any case), as `comment.list` reports mentions;
+/// `name` resolves like one of `people`, so a name nobody in the file has can be several words.
+fn thread_mentions(c: &Comment, people: &[&str], name: &str) -> bool {
+    let people: Vec<&str> = people.iter().copied().chain([name]).collect();
+    c.messages().any(|m| mentioned(&m.text, &people).any(|(n, _)| n.eq_ignore_ascii_case(name)))
+}
+
+/// A list's `mentions` filter: a name (a leading `@` is fine), or none.
+fn mentions_param(cmd: &str, p: &Value) -> crate::Result<Option<String>> {
+    match get(p, "mentions") {
+        None | Some(Value::Null) => Ok(None),
+        Some(Value::String(s)) if !s.trim().trim_start_matches('@').trim().is_empty() => {
+            Ok(Some(s.trim().trim_start_matches('@').trim().to_string()))
+        }
+        Some(_) => Err(bad(cmd, "`mentions` must be a name")),
+    }
+}
+
+/// The threads a list's params pick (`list_params`, `frame`, `mentions`), in list order, as
+/// `who` sees them; mentions resolve against `people` (`mentionable`).
+fn listed<'s>(cmd: &str, s: &'s Session, p: &Value, who: &str, people: &[&str]) -> crate::Result<Vec<&'s Comment>> {
+    let (page, filter, sort, text) = list_params(cmd, s, p)?;
+    let want = mentions_param(cmd, p)?;
+    let q = Query { page, frame: id(p, "frame"), filter, sort, text: &text };
+    Ok(query(s.doc()?, &q, who).into_iter().filter(|c| want.as_deref().is_none_or(|w| thread_mentions(c, people, w))).collect())
+}
+
+/// Add the names `text` @mentions to `named` as `{name, known, you}`, each once.
+fn push_mentions(named: &mut Vec<Value>, text: &str, people: &[&str], user: &str) {
+    for (name, known) in mentioned(text, people) {
+        let v = json!({"name": name, "known": known, "you": known && name.eq_ignore_ascii_case(user)});
         if !named.contains(&v) {
             named.push(v);
         }
     }
+}
+
+/// The most mentions `comment.prompt` lists for one thread (its list stays bounded).
+const PROMPT_MAX_MENTIONS: usize = 32;
+
+/// `comment.people`: everyone who wrote or reacted in a comment, and every name a message
+/// @mentions (as `comment.list` reports it), once each in any case, sorted as
+/// `Document::comment_people` sorts.
+fn people_and_mentioned(doc: &Document, you: &str) -> Vec<String> {
+    let people = doc.comment_people();
+    let resolve = mentionable(&people, you);
+    let mut seen = std::collections::HashSet::new();
+    let mut out: Vec<String> = Vec::new();
+    let names = people
+        .iter()
+        .map(String::as_str)
+        .chain(doc.comments.iter().flat_map(Comment::messages).flat_map(|m| mentioned(&m.text, &resolve).map(|(n, _)| n)));
+    for name in names {
+        if seen.insert(name.to_lowercase()) {
+            out.push(name.to_string());
+        }
+    }
+    out.sort_by(|a, b| a.to_lowercase().cmp(&b.to_lowercase()).then_with(|| a.cmp(b)));
+    out
+}
+
+/// A message as JSON, with the names it @mentions: `{name, known, you}` each, resolved against
+/// `people` (`mentionable`); `known: false` is a name nobody in the file has.
+fn message_json(m: &Message, people: &[&str], user: &str) -> Value {
+    let mut named: Vec<Value> = Vec::new();
+    push_mentions(&mut named, &m.text, people, user);
     json!({
         "author": m.author, "agent": m.agent, "text": m.text, "created": m.created, "edited": m.edited,
         "reactions": m.reactions.iter().map(|r| json!({"emoji": r.emoji, "users": r.users})).collect::<Vec<_>>(),
@@ -371,9 +433,23 @@ pub fn comment_json(doc: &Document, c: &Comment, user: &str, people: &[&str]) ->
 
 /// Bounded metadata for `comment.prompt`'s `comments` list. Full messages and replies stay in
 /// `comment.list`; a prompt over thousands of threads must not repeat all of them here.
-fn comment_brief_json(doc: &Document, c: &Comment, user: &str) -> Value {
+fn comment_brief_json(doc: &Document, c: &Comment, user: &str, people: &[&str]) -> Value {
     let (x, y) = c.position(doc);
     let frame = frame_of(doc, c);
+    // The names the whole thread @mentions, as `comment.list` reports them per message.
+    let mut named = Vec::new();
+    for m in c.messages() {
+        push_mentions(&mut named, &m.text, people, user);
+        if named.len() >= PROMPT_MAX_MENTIONS {
+            break;
+        }
+    }
+    named.truncate(PROMPT_MAX_MENTIONS);
+    for v in &mut named {
+        if let Some(name) = v.get("name").and_then(Value::as_str).map(|n| clip_chars(n, 80)) {
+            v["name"] = json!(name);
+        }
+    }
     json!({
         "id": c.id, "page": c.page,
         "pageName": doc.node(c.page).map(|n| clip_chars(&n.name, 80)),
@@ -381,7 +457,7 @@ fn comment_brief_json(doc: &Document, c: &Comment, user: &str) -> Value {
         "frame": frame, "frameName": frame.and_then(|f| doc.node(f)).map(|n| clip_chars(&n.name, 80)),
         "resolved": c.resolved, "unread": c.is_unread(user), "link": link(c.id),
         "author": clip_chars(&c.first.author, 80), "agent": c.first.agent, "preview": clip_chars(&c.first.text, 160),
-        "replies": c.replies.len(),
+        "replies": c.replies.len(), "mentions": named,
     })
 }
 
@@ -760,15 +836,13 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "comment.next",
             "Next comment",
-            "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query?, frame?, resolved? (the list, as comment.list), author?}",
+            "{id? (the thread to step from; omitted: the first or last), dir? (1 next, -1 previous; default 1), page?, filter?, sort?, query?, frame?, resolved?, mentions? (the list, as comment.list), author?}  → the thread, as comment.list lists it",
             false,
             |s, p| {
                 let dir = num(p, "dir").unwrap_or(1.0);
-                let doc = s.doc()?;
                 let who = author(s, p);
-                let (page, filter, sort, text) = list_params("comment.next", s, p)?;
-                let ids: Vec<u64> =
-                    query(doc, &Query { page, frame: id(p, "frame"), filter, sort, text: &text }, &who).iter().map(|c| c.id).collect();
+                let people = s.doc()?.comment_people();
+                let ids: Vec<u64> = listed("comment.next", s, p, &who, &mentionable(&people, &who))?.iter().map(|c| c.id).collect();
                 let at = id(p, "id").and_then(|i| ids.iter().position(|x| *x == i.0));
                 let next = match at {
                     Some(i) if dir < 0.0 => ids.get(i.saturating_sub(1)),
@@ -801,22 +875,28 @@ pub fn specs() -> Vec<CommandSpec> {
             let id = req_comment("comment.link", s, p)?;
             Ok(json!({"link": link(id)}))
         }),
-        spec!("comment.people", "People in comments", "{}", false, |s, _| {
-            Ok(json!({"people": s.doc()?.comment_people(), "you": acting_name(s)}))
-        }),
+        spec!(
+            "comment.people",
+            "People in comments",
+            "{}  → {people (everyone who wrote or reacted in a comment, and every name a message @mentions), you}",
+            false,
+            |s, _| {
+                let you = acting_name(s);
+                Ok(json!({"people": people_and_mentioned(s.doc()?, &you), "you": you}))
+            }
+        ),
         spec!(
             "comment.list",
             "List comments",
-            "{page? (default: current; \"all\" for every page), filter? (all|open|resolved|mine|unread; default all), sort? (date|page; default date), query? (search text), frame? (a top-level frame id), resolved? (false hides resolved ones), author? (whose threads and read state)}",
+            "{page? (default: current; \"all\" for every page), filter? (all|open|resolved|mine|unread; default all), sort? (date|page; default date), query? (search text), frame? (a top-level frame id), resolved? (false hides resolved ones), author? (whose threads and read state), mentions? (a name, any case: only threads where a message @mentions it, as each message's `mentions` reports)}",
             false,
             |s, p| {
                 let doc = s.doc()?;
-                let (page, filter, sort, text) = list_params("comment.list", s, p)?;
                 let who = author(s, p);
-                let q = Query { page, frame: id(p, "frame"), filter, sort, text: &text };
                 let people = doc.comment_people();
                 let people = mentionable(&people, &who);
-                let list: Vec<Value> = query(doc, &q, &who).into_iter().map(|c| comment_json(doc, c, &who, &people)).collect();
+                let list: Vec<Value> =
+                    listed("comment.list", s, p, &who, &people)?.into_iter().map(|c| comment_json(doc, c, &who, &people)).collect();
                 Ok(json!({"comments": list}))
             }
         ),
@@ -877,7 +957,9 @@ pub fn specs() -> Vec<CommandSpec> {
                 };
                 let count = selected.len();
                 let (text, truncated) = prompt_text(doc, path.as_deref(), &selected);
-                let comments: Vec<Value> = selected.iter().take(PROMPT_MAX_THREADS).map(|c| comment_brief_json(doc, c, &who)).collect();
+                let people = doc.comment_people();
+                let people = mentionable(&people, &who);
+                let comments: Vec<Value> = selected.iter().take(PROMPT_MAX_THREADS).map(|c| comment_brief_json(doc, c, &who, &people)).collect();
                 Ok(json!({"count": count, "truncated": truncated, "text": text, "comments": comments}))
             }
         ),

crates/engine/src/cmd/file.rs

@@ -33,7 +33,7 @@ use load::font_report;
 #[cfg(test)]
 pub(crate) use load::{installed_family, sketch_line};
 pub use load::{load, load_bytes, load_path};
-pub(crate) use load::{load_fig, read_capped, recovered, variables_param, write};
+pub(crate) use load::{load_fig, read_capped, recovered, recovered_from, variables_param, write};
 pub use open::MAX_RECENT;
 #[cfg(test)]
 use open::append_pages;
@@ -181,11 +181,16 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "file.recover",
             "Recover file",
-            "{path}  open a recovery copy (written while the app runs, read back after a crash) as a document with unsaved changes and no path: saving asks where → {index}",
+            "{path | data (base64: the copy's bytes, as the browser keeps them)}  open a recovery copy (written while the app runs, read back after a crash) as a document with unsaved changes and no path: saving asks where → {index}",
             false,
             |s, p| {
-                let path = req_string("file.recover", p, "path")?;
-                s.docs.push(recovered(&path)?);
+                let ds = match string(p, "data") {
+                    Some(d) => recovered_from(
+                        &base64::engine::general_purpose::STANDARD.decode(d.trim()).map_err(|e| bad("file.recover", format!("`data`: {e}")))?,
+                    )?,
+                    None => recovered(&req_string("file.recover", p, "path")?)?,
+                };
+                s.docs.push(ds);
                 s.active = s.docs.len() - 1;
                 Ok(json!({"index": s.active}))
             }

crates/engine/src/cmd/file/load.rs

@@ -4,7 +4,13 @@ use super::{DocState, Document, EngineError, LineHeight, NodeId, NodeKind, Resul
 
 /// The recovery copy at `path` as a document with unsaved changes and no path.
 pub(crate) fn recovered(path: &str) -> Result<DocState> {
-    let loaded = read(path).and_then(|b| figcraft_format::load_bytes_with_versions(&b).map_err(|e| EngineError::Other(e.to_string())))?;
+    recovered_from(&read(path)?)
+}
+
+/// A recovery copy's bytes as a document with unsaved changes and no path (the browser keeps its
+/// copies in the page's storage, not in files).
+pub(crate) fn recovered_from(bytes: &[u8]) -> Result<DocState> {
+    let loaded = figcraft_format::load_bytes_with_versions(bytes).map_err(|e| EngineError::Other(e.to_string()))?;
     Ok(DocState::new(loaded.doc, None).with_versions(loaded.versions).with_branches(loaded.branches).unsaved())
 }
 
@@ -693,6 +699,10 @@ pub(crate) fn variables_param(cmd: &str, p: &Value) -> Result<Option<Value>> {
         None | Some(Value::Null) => return Ok(None),
         Some(v @ Value::Object(_)) => v.clone(),
         Some(Value::String(s)) if s.trim_start().starts_with('{') => parse(s.as_bytes())?,
+        // Capped like the sibling file; the browser has no file system, which `read` says.
+        #[cfg(not(target_arch = "wasm32"))]
+        Some(Value::String(path)) => parse(&read_capped(path, MAX_VARIABLES_FILE)?)?,
+        #[cfg(target_arch = "wasm32")]
         Some(Value::String(path)) => parse(&read(path)?)?,
         Some(_) => return Err(bad(cmd, "variables: the local variables JSON (an object, JSON text or a path)")),
     };

crates/engine/src/cmd/file/open.rs

@@ -183,7 +183,7 @@ pub(super) fn import(s: &mut Session, p: &Value) -> Result<Value> {
             }
             Ok(out)
         }
-        "figcraft" | "json" | "fig" | "sketch" | "penpot" => {
+        "figcraft" | "json" | "fig" | "jam" | "sketch" | "penpot" => {
             let resolver = path.as_deref().map(rest_images);
             let images: figcraft_format::rest::ImageResolver = match &resolver {
                 Some(r) => r,

crates/engine/src/cmd/structure.rs

@@ -475,13 +475,7 @@ fn collect_flatten_parts(
             part
         }
         _ => {
-            let Some((path, rule)) = figcraft_render::node_outline(doc, node) else { return Ok(()) };
-            let mut part = VectorNetwork::from_path(&path);
-            for region in &mut part.regions {
-                if rule == figcraft_render::FillRule::EvenOdd {
-                    region.winding = WindingRule::EvenOdd;
-                }
-            }
+            let Some(part) = outline_part(doc, node)? else { return Ok(()) };
             part
         }
     };
@@ -490,6 +484,25 @@ fn collect_flatten_parts(
     Ok(())
 }
 
+/// What a layer other than a vector or a container flattens into: its outline (`None`: it has
+/// none). A boolean group has no outline only past the path operations' caps; flattening it would
+/// drop it, so that is refused.
+fn outline_part(doc: &Document, node: &Node) -> Result<Option<VectorNetwork>> {
+    let Some((path, rule)) = figcraft_render::node_outline(doc, node) else {
+        if matches!(node.kind, NodeKind::BooleanOp { .. }) {
+            return Err(bad("vector.flatten", figcraft_pathops::PathOpsError::TooComplex.to_string()));
+        }
+        return Ok(None);
+    };
+    let mut part = VectorNetwork::from_path(&path);
+    for region in &mut part.regions {
+        if rule == figcraft_render::FillRule::EvenOdd {
+            region.winding = WindingRule::EvenOdd;
+        }
+    }
+    Ok(Some(part))
+}
+
 fn append_flatten_part(
     node: &Node,
     fallback_fills: Option<&[Paint]>,

crates/engine/src/cmd/variables_io.rs

@@ -25,7 +25,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "variable.import",
             "Import variables",
-            "{variables (the local variables JSON, as variable.export writes it: an object, JSON text or a path; `variableCollections` and `variables` under `meta` or at the top level)}  collections join this file's collection of the same name (values by mode name; new modes and variables are added, a variable of another type is skipped), others are added  → {collections (added), variables (added), updated, warnings (also when it holds no variables, or fewer than its collections list)}",
+            "{variables (the local variables JSON, as variable.export writes it: an object, JSON text or a path to a regular file of at most 256 MiB; `variableCollections` and `variables` under `meta` or at the top level)}  collections join this file's collection of the same name (values by mode name; new modes and variables are added, a variable of another type is skipped), others are added  → {collections (added), variables (added), updated, warnings (also when it holds no variables, or fewer than its collections list)}",
             true,
             import
         ),

crates/engine/src/cmd/view.rs

@@ -201,8 +201,9 @@ fn pref(s: &mut Session, p: &Value, f: fn(&mut crate::Prefs) -> &mut bool) -> Re
 }
 
 pub(crate) fn tool(s: &mut Session, t: ToolId) -> Result<Value> {
-    // A file you can't edit now (view only, or on its way to a server): moving around only.
-    if let Some(why) = s.active_read_only().filter(|_| !matches!(t, ToolId::Move | ToolId::Hand)) {
+    // A file you can't edit now (view only, or on its way to a server): moving around only, and
+    // commenting with a comment link.
+    if let Some(why) = s.active_read_only().filter(|why| !crate::live::tool_left(why, t)) {
         return Err(EngineError::Disabled(format!("tool.{}", t.id()), why.into()));
     }
     s.set_tool(t);
@@ -369,6 +370,13 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!("pref.renameDuplicates", "Rename duplicated layers", "{on?}", false, |s, p| pref(s, p, |x| &mut x.rename_duplicates)),
         spec!("pref.showDimensions", "Show dimensions on objects", "{on?}", false, |s, p| pref(s, p, |x| &mut x.show_dimensions)),
         spec!("pref.numberKeysOpacity", "Use number keys for opacity", "{on?}", false, |s, p| pref(s, p, |x| &mut x.number_keys_opacity)),
+        spec!(
+            "pref.scrollWheelZoom",
+            "Scroll wheel zoom",
+            "{on?}  a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls it (Shift+wheel still scrolls sideways; trackpads are unchanged) → {on}",
+            false,
+            |s, p| pref(s, p, |x| &mut x.scroll_wheel_zoom)
+        ),
         spec!("pref.reduceMotion", "Reduce motion", "{on?}", false, |s, p| pref(s, p, |x| &mut x.reduce_motion)),
         spec!(
             "pref.uiScale",

crates/engine/src/comment_link_tests.rs

@@ -0,0 +1,91 @@
+//! A server file joined with a comment link, against a scripted link: comments (new threads,
+//! replies, resolving, reactions) are made and sync through the server; every other edit, the
+//! editing tools and gestures are refused as with a view link, and a change made outside a command
+//! is put back unless it only touches comment threads.
+
+use figcraft_collab::message::ClientMsg;
+use figcraft_doc::{Document, NodeKind};
+use serde_json::json;
+
+use crate::live::{Access, COMMENT_ONLY, Event, Role, tool_left};
+use crate::live_tests::{info, push, relay, serve_as, session};
+use crate::tools::ToolId;
+use crate::{EngineError, Session};
+
+fn refused(s: &mut Session, cmd: &str, params: serde_json::Value) {
+    match s.execute(cmd, &params) {
+        Err(EngineError::Disabled(_, reason)) => assert_eq!(reason, COMMENT_ONLY, "{cmd}"),
+        other => panic!("{cmd}: expected a refusal, got {other:?}"),
+    }
+}
+
+#[test]
+fn a_comment_link_lets_you_comment_and_nothing_else() {
+    let (mut s, script) = session();
+    s.execute("tool.set", &json!({"tool": "rectangle"})).unwrap();
+    s.execute("collab.join", &json!({"link": "figcraft://join?host=a:1&file=f&token=t"})).unwrap();
+    let mut doc = Document::new("Shared");
+    let page = doc.first_page().unwrap();
+    let card = doc.create(page, NodeKind::Rectangle, "Card", 0.0, 0.0, 10.0, 10.0).unwrap();
+    let (mut server, id, welcome) = serve_as(&doc, Access::Comment);
+    let mut me = info(Role::Guest, id);
+    me.access = Access::Comment;
+    push(&script, vec![Event::Welcome { info: me, peers: vec![] }, welcome]);
+    s.live_poll();
+    assert!(s.live().syncing());
+    assert_eq!(s.active_read_only(), Some(COMMENT_ONLY));
+    let summary = s.live_summary();
+    assert_eq!((summary["access"].as_str(), summary["readOnly"].as_bool()), (Some("comment"), Some(true)));
+    assert_eq!(s.tool.tool, ToolId::Move, "the editing tool was put away");
+    // The comment tool is left, and commenting syncs through the server.
+    s.execute("tool.set", &json!({"tool": "comment"})).unwrap();
+    let thread = s.execute("comment.add", &json!({"x": 5, "y": 5, "text": "Bigger logo?"})).unwrap()["id"].as_u64().unwrap();
+    s.execute("comment.reply", &json!({"id": thread, "text": "Or darker"})).unwrap();
+    s.execute("comment.react", &json!({"id": thread, "emoji": "👍"})).unwrap();
+    s.execute("comment.resolve", &json!({"id": thread})).unwrap();
+    let answers = relay(&script, &mut server, id);
+    let on_server = server.document().comment(thread).expect("the thread is on the server");
+    assert!(on_server.resolved && on_server.replies.len() == 1 && !on_server.first.reactions.is_empty());
+    push(&script, answers);
+    s.live_poll();
+    // Everything else is refused before anything is sent, as with a view link.
+    let before = s.doc().unwrap().clone();
+    refused(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 5, "height": 5}));
+    refused(&mut s, "node.set", json!({"ids": [card.0], "props": {"name": "Mine"}}));
+    refused(&mut s, "edit.delete", json!({"ids": [card.0]}));
+    refused(&mut s, "tool.set", json!({"tool": "text"}));
+    assert!(matches!(s.begin_interaction("Move"), Err(EngineError::Disabled(..))));
+    assert_eq!(*s.doc().unwrap(), before);
+    assert!(script.lock().unwrap().msgs.iter().all(|m| !matches!(m, ClientMsg::Changes { .. })), "nothing sent");
+    // A change made outside a command: put back, unless it only touches comment threads.
+    s.ds_mut().unwrap().doc_mut().node_mut(card).unwrap().name = "Outside".into();
+    s.live_sync();
+    assert_eq!(s.doc().unwrap().node(card).unwrap().name, "Card");
+    s.ds_mut().unwrap().doc_mut().comment_mut(thread).unwrap().resolved = false;
+    s.live_sync();
+    relay(&script, &mut server, id);
+    assert!(!server.document().comment(thread).unwrap().resolved, "sent");
+    // Your other tabs stay yours.
+    s.active = 0;
+    assert_eq!(s.active_read_only(), None);
+    assert!(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 5, "height": 5})).is_ok());
+}
+
+#[test]
+fn only_a_comment_link_leaves_the_comment_tool() {
+    for t in [ToolId::Move, ToolId::Hand, ToolId::Comment] {
+        assert!(tool_left(COMMENT_ONLY, t), "{t:?}");
+    }
+    assert!(!tool_left(COMMENT_ONLY, ToolId::Rectangle));
+    assert!(!tool_left(crate::live::VIEW_ONLY, ToolId::Comment) && tool_left(crate::live::VIEW_ONLY, ToolId::Hand));
+    assert!(!tool_left(crate::live::PUBLISHING, ToolId::Comment));
+}
+
+#[test]
+fn a_local_share_has_no_comment_code() {
+    let (mut s, _) = session();
+    let e = s.execute("collab.share", &json!({"via": "local", "access": "comment"})).unwrap_err().to_string();
+    assert!(e.contains("can edit it"), "{e}");
+    let e = s.execute("collab.share", &json!({"access": "owner"})).unwrap_err().to_string();
+    assert!(e.contains("\"comment\""), "{e}");
+}

crates/engine/src/comment_mentions_tests.rs

@@ -1,5 +1,6 @@
 //! `comment.list` reports the names each message @mentions, by the rule the thread view highlights
 //! them with: resolved against the file's people where it can be, and whether the name is you.
+//! Its `mentions` param lists only the threads where a message mentions a name.
 
 use serde_json::{Value, json};
 
@@ -42,3 +43,101 @@ fn the_mention_rule_matches_the_highlighting_rule() {
     assert_eq!(figcraft_doc::mention_ranges("@Sam Lee, @sam, x@Sam, @Zoe!", &people), vec![0..8, 10..14]);
     assert!(figcraft_doc::mentions("@ and a@b.com", &people).is_empty());
 }
+
+/// The ids of the threads `comment.list` returns for `p`.
+fn listed(s: &mut Session, p: Value) -> Vec<u64> {
+    let l = s.execute("comment.list", &p).unwrap();
+    l["comments"].as_array().unwrap().iter().map(|c| c["id"].as_u64().unwrap()).collect()
+}
+
+fn add(s: &mut Session, text: &str, author: &str) -> u64 {
+    s.execute("comment.add", &json!({"x": 0, "y": 0, "text": text, "author": author})).unwrap()["id"].as_u64().unwrap()
+}
+
+#[test]
+fn the_mentions_param_lists_only_the_threads_that_mention_a_name() {
+    let mut s = Session::new();
+    let claude = add(&mut s, "@Claude make this bigger", "Sam");
+    let in_reply = add(&mut s, "Spacing looks off", "Sam");
+    s.execute("comment.reply", &json!({"id": in_reply, "text": "agreed, @claude: tighten it", "author": "Ada Lovelace"})).unwrap();
+    let other = add(&mut s, "@Ada Lovelace and @Zoe Smith, mail claude@example.com", "Sam");
+    add(&mut s, "No mentions here", "Sam");
+    let mut both = listed(&mut s, json!({"mentions": "CLAUDE"}));
+    both.sort_unstable();
+    assert_eq!(both, vec![claude, in_reply], "any case, in a reply too, never an email address");
+    assert_eq!(listed(&mut s, json!({"mentions": " @Claude "})), listed(&mut s, json!({"mentions": "claude"})), "a leading @ is fine");
+    assert_eq!(listed(&mut s, json!({"mentions": "ada lovelace"})), vec![other]);
+    assert!(listed(&mut s, json!({"mentions": "Ada"})).is_empty(), "@Ada Lovelace names Ada Lovelace, not Ada");
+    assert_eq!(listed(&mut s, json!({"mentions": "Zoe Smith"})), vec![other], "a name nobody in the file has, several words");
+    assert_eq!(listed(&mut s, json!({"mentions": null})).len(), 4);
+    // Combined with the other filters: resolved threads drop out.
+    s.execute("comment.resolve", &json!({"id": claude})).unwrap();
+    assert_eq!(listed(&mut s, json!({"mentions": "Claude", "filter": "open"})), vec![in_reply]);
+    for junk in [json!(""), json!("@"), json!(5), json!(["Claude"])] {
+        assert!(s.execute("comment.list", &json!({"mentions": junk})).is_err(), "{junk}");
+    }
+}
+
+#[test]
+fn people_lists_the_names_only_mentioned_too() {
+    let mut s = Session::new();
+    let id = add(&mut s, "@ada lovelace, @Zoe and @zoe: see me@example.com", "Sam");
+    s.execute("comment.reply", &json!({"id": id, "text": "On it", "author": "Ada Lovelace"})).unwrap();
+    s.execute("comment.react", &json!({"id": id, "emoji": "👍", "author": "Lin"})).unwrap();
+    let p = s.execute("comment.people", &json!({})).unwrap();
+    // Ada once, spelt as the file spells her; Zoe once in any case; no email address.
+    assert_eq!(p["people"], json!(["Ada Lovelace", "Lin", "Sam", "Zoe"]));
+    assert_eq!(p["you"], "You");
+}
+
+#[test]
+fn people_who_write_in_two_cases_are_listed_once() {
+    let mut s = Session::new();
+    let id = add(&mut s, "Hi", "Ada");
+    s.execute("comment.reply", &json!({"id": id, "text": "Hi @ADA", "author": "ada"})).unwrap();
+    let p = s.execute("comment.people", &json!({})).unwrap();
+    assert_eq!(p["people"].as_array().map(Vec::len), Some(1), "{}", p["people"]);
+}
+
+#[test]
+fn next_and_prompt_carry_the_mentions_too() {
+    let mut s = Session::new();
+    let first = add(&mut s, "@Claude make this bigger", "Sam");
+    let second = add(&mut s, "Spacing looks off", "Sam");
+    s.execute("comment.reply", &json!({"id": second, "text": "@Claude and @Bob, tighten it", "author": "Ada"})).unwrap();
+    let plain = add(&mut s, "No mentions here", "Sam");
+    let claude = json!({"name": "Claude", "known": false, "you": false});
+    // comment.next returns the thread as comment.list lists it, mentions and all.
+    let list = s.execute("comment.list", &json!({})).unwrap();
+    let ids: Vec<u64> = list["comments"].as_array().unwrap().iter().map(|c| c["id"].as_u64().unwrap()).collect();
+    let next = s.execute("comment.next", &json!({"id": ids[0]})).unwrap();
+    let listed = list["comments"].as_array().unwrap().iter().find(|c| c["id"] == next["id"]).unwrap();
+    assert_eq!(next["mentions"], listed["mentions"]);
+    assert_eq!(next["replies"], listed["replies"]);
+    // Stepping through the threads that mention Claude skips the others.
+    let n = s.execute("comment.next", &json!({"mentions": "claude"})).unwrap();
+    let m = s.execute("comment.next", &json!({"mentions": "claude", "id": n["id"]})).unwrap();
+    let mut both = vec![n["id"].as_u64().unwrap(), m["id"].as_u64().unwrap()];
+    both.sort_unstable();
+    assert_eq!(both, vec![first, second]);
+    assert!(s.execute("comment.next", &json!({"mentions": 5})).is_err());
+    // comment.prompt: each thread's mentions, the first message's and the replies' together.
+    let prompt = s.execute("comment.prompt", &json!({})).unwrap();
+    let mentions = |id: u64| prompt["comments"].as_array().unwrap().iter().find(|c| c["id"] == id).unwrap()["mentions"].clone();
+    assert_eq!(mentions(first), json!([claude]));
+    assert_eq!(mentions(second), json!([claude, {"name": "Bob", "known": false, "you": false}]));
+    assert_eq!(mentions(plain), json!([]));
+}
+
+#[test]
+fn a_prompt_lists_a_bounded_number_of_short_mentions() {
+    let mut s = Session::new();
+    let many: String = (0..100).map(|i| format!("@n{i} ")).collect();
+    let id = add(&mut s, &format!("{many} @{}", "x".repeat(500)), "Sam");
+    let prompt = s.execute("comment.prompt", &json!({"ids": [id]})).unwrap();
+    let mentions = prompt["comments"][0]["mentions"].as_array().unwrap().clone();
+    assert_eq!(mentions.len(), 32);
+    let id = add(&mut s, &format!("@{}", "y".repeat(500)), "Sam");
+    let long = s.execute("comment.prompt", &json!({"ids": [id]})).unwrap();
+    assert!(long["comments"][0]["mentions"][0]["name"].as_str().unwrap().chars().count() <= 81);
+}

crates/engine/src/comments_polish_tests.rs

@@ -137,7 +137,8 @@ fn mentions_and_people() {
     let c = add(&mut s, 0.0, 0.0, "Ask @Ada", "Sam");
     s.execute("comment.react", &json!({"id": c, "emoji": "👀", "author": "Lin"})).unwrap();
     let p = s.execute("comment.people", &json!({})).unwrap();
-    assert_eq!(p["people"], json!(["Lin", "Sam"]));
+    // Ada was only @mentioned, and is listed too (#1415).
+    assert_eq!(p["people"], json!(["Ada", "Lin", "Sam"]));
     assert_eq!(p["you"], "You");
     // A thread you're mentioned in is yours.
     assert!(comment(&s, c).involves("Ada"));

crates/engine/src/derived.rs

@@ -128,9 +128,10 @@ pub fn normalize(doc: &mut Document, before: &Document) {
         if !doc.contains(id) {
             continue;
         }
-        let mut chain = vec![id];
-        chain.extend(doc.ancestors(id));
-        for c in chain {
+        // A frame that only moved (or turned) keeps its contents where they are: its own layout
+        // runs again only for a change that can move or resize them. Its parent's still runs.
+        let own = (!moved_only(before, doc, id)).then_some(id);
+        for c in own.into_iter().chain(doc.ancestors(id)) {
             if doc.node(c).is_some_and(|n| n.layout.is_some()) {
                 frames.insert(c);
             }
@@ -177,9 +178,19 @@ fn keeps_right_edge(doc: &Document, before: &Document, n: &Node) -> bool {
 /// layout settings.
 fn affects_layout(before: &Document, doc: &Document, id: NodeId) -> bool {
     let (Some(a), Some(b)) = (before.node(id), doc.node(id)) else { return true };
+    a.transform != b.transform || reshaped(a, b)
+}
+
+/// Whether layer `id` only moved (or turned) since `before`: its transform is all that changed of
+/// what [`affects_layout`] looks at, so the layers inside it stay where they are.
+fn moved_only(before: &Document, doc: &Document, id: NodeId) -> bool {
+    before.node(id).zip(doc.node(id)).is_some_and(|(a, b)| !reshaped(a, b))
+}
+
+/// What [`affects_layout`] looks at, but where the layer sits in its parent.
+fn reshaped(a: &Node, b: &Node) -> bool {
     a.width != b.width
         || a.height != b.height
-        || a.transform != b.transform
         || a.visible != b.visible
         || a.parent != b.parent
         || a.children != b.children

crates/engine/src/guard.rs

@@ -1,5 +1,9 @@
 //! The last line of defence: a panic inside a command becomes an [`EngineError::Internal`] and
 //! the caller rolls the document back. It exists for bugs and doesn't replace `Result`.
+//!
+//! The browser build can't catch a panic (wasm32 always aborts, and stable Rust has no unwinding
+//! std for it), so there a panicking command stops the page: its panic hook keeps the files with
+//! unsaved changes for the next page instead (`apps/figcraft-web/src/recovery.rs`).
 
 use crate::{EngineError, Result};
 

crates/engine/src/import_export_tests.rs

@@ -94,6 +94,63 @@ fn open_reads_variables_from_param_or_next_to_the_file() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// Every document, the active tab and the recent files, compared before and after a refused command.
+fn state(s: &Session) -> (String, usize, Vec<String>) {
+    (format!("{:?}", s.docs), s.active, s.prefs.recent_files.clone())
+}
+
+#[test]
+fn an_explicit_variables_path_is_capped_like_the_sibling_file() {
+    let dir = temp_dir("restvars-cap");
+    let path = dir.join("proto.json");
+    std::fs::write(&path, serde_json::to_vec(&file()).unwrap()).unwrap();
+    let path = path.to_string_lossy().to_string();
+    let vars = dir.join("theme.json");
+    std::fs::write(&vars, serde_json::to_vec(&variables()).unwrap()).unwrap();
+    let vars = vars.to_string_lossy().to_string();
+
+    // A small file opens and imports as before.
+    let mut s = Session::new();
+    s.execute("file.open", &json!({"path": path, "variables": vars})).unwrap();
+    assert_eq!(named(&s, "Button").bound.get("fill").map(String::as_str), Some(BRAND));
+
+    // One byte over the limit (sparse, so nothing is written), a folder, and a device are refused
+    // before the document changes, by the same check the sibling file goes through.
+    let big = dir.join("big.json");
+    std::fs::File::create(&big).unwrap().set_len(256 * 1024 * 1024 + 1).unwrap();
+    let mut refused = vec![big.to_string_lossy().to_string(), dir.to_string_lossy().to_string()];
+    if cfg!(unix) {
+        refused.push("/dev/null".to_string());
+    }
+    s.prefs.recent_files = vec![path.clone()];
+    s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 20, "height": 10})).unwrap();
+    for bad in &refused {
+        for cmd in ["file.open", "file.import"] {
+            let before = state(&s);
+            let e = s.execute(cmd, &json!({"path": path, "variables": bad})).unwrap_err().to_string();
+            assert!(e.contains("not a file of at most 268435456 bytes"), "{cmd} {bad}: {e}");
+            assert_eq!(state(&s), before, "{cmd} {bad}");
+        }
+    }
+
+    // Access roots still apply: inside them the file reads, outside it is rejected.
+    let inside = temp_dir("restvars-cap-root");
+    std::fs::copy(&vars, inside.join("theme.json")).unwrap();
+    std::fs::copy(&path, inside.join("proto.json")).unwrap();
+    let roots = std::sync::Arc::new(crate::file_access::Roots::new(&[inside.to_string_lossy().to_string()], &[]).unwrap());
+    let inside_vars = inside.join("theme.json").to_string_lossy().to_string();
+    let inside_path = inside.join("proto.json").to_string_lossy().to_string();
+    crate::file_access::confined(Some(&roots), || {
+        s.execute("file.import", &json!({"path": inside_path, "variables": inside_vars})).unwrap();
+        let before = state(&s);
+        let e = s.execute("file.import", &json!({"path": inside_path, "variables": vars})).unwrap_err().to_string();
+        assert!(e.contains("automation path rejected"), "{e}");
+        assert_eq!(state(&s), before);
+    });
+    let _ = std::fs::remove_dir_all(&dir);
+    let _ = std::fs::remove_dir_all(&inside);
+}
+
 #[test]
 fn import_appends_flows_interactions_and_variables() {
     let mut s = Session::new();

crates/engine/src/jam_open_tests.rs

@@ -0,0 +1,55 @@
+//! FigJam `.jam` files (#1342) open and import like `.fig` files: File ▸ Open (`file.open`), the
+//! command line (the same command) and File ▸ Import / drag and drop (`file.import`). The file is
+//! made here from our own `.fig` export with the canvas header FigJam writes (`fig-jam.`).
+
+use std::io::{Read, Write};
+
+use serde_json::json;
+
+use crate::Session;
+
+/// A `.jam` holding one page with a rectangle named `Card`, written to a temporary file.
+fn jam_file(tag: &str) -> std::path::PathBuf {
+    let mut doc = figcraft_doc::Document::new("Synthetic board");
+    let page = doc.first_page().unwrap();
+    doc.create(page, figcraft_doc::NodeKind::Rectangle, "Card", 10.0, 10.0, 80.0, 40.0).unwrap();
+    let (fig, _) = figcraft_format::fig::export(&doc, None).unwrap();
+    let mut from = zip::ZipArchive::new(std::io::Cursor::new(fig)).unwrap();
+    let mut to = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
+    for i in 0..from.len() {
+        let mut member = from.by_index(i).unwrap();
+        let mut bytes = Vec::new();
+        member.read_to_end(&mut bytes).unwrap();
+        if member.name() == "canvas.fig" {
+            assert!(bytes.starts_with(b"fig-kiwi"));
+            bytes.splice(..8, *b"fig-jam.");
+        }
+        to.start_file(member.name(), zip::write::SimpleFileOptions::default()).unwrap();
+        to.write_all(&bytes).unwrap();
+    }
+    let path = std::env::temp_dir().join(format!("figcraft-jam-{tag}-{}.jam", std::process::id()));
+    std::fs::write(&path, to.finish().unwrap().into_inner()).unwrap();
+    path
+}
+
+fn cards(s: &Session) -> usize {
+    s.doc().unwrap().nodes().filter(|n| n.name == "Card").count()
+}
+
+#[test]
+fn a_jam_file_opens_and_imports_like_a_fig() {
+    let path = jam_file("open");
+    let mut s = Session::new();
+    s.execute("file.open", &json!({"path": path})).unwrap();
+    assert_eq!(cards(&s), 1, "opened");
+    assert!(crate::board::is_board(s.doc().unwrap()), "a whiteboard opens as a board");
+    // Imported into a design file, it adds its pages to it (not an image of its bytes), which
+    // stays a design file.
+    s.execute("doc.switch", &json!({"index": 0})).unwrap();
+    let before = s.doc().unwrap().pages.len();
+    s.execute("file.import", &json!({"path": path})).unwrap();
+    assert_eq!(s.doc().unwrap().pages.len(), before + 1);
+    assert_eq!(cards(&s), 1);
+    assert!(!crate::board::is_board(s.doc().unwrap()));
+    std::fs::remove_file(path).unwrap();
+}

crates/engine/src/lib.rs

@@ -187,6 +187,8 @@ mod arc_fields_tests;
 #[cfg(test)]
 mod arc_handles_tests;
 #[cfg(test)]
+mod auto_layout_move_tests;
+#[cfg(test)]
 mod autosave_tests;
 #[cfg(test)]
 mod batch_continue_events_tests;
@@ -207,6 +209,8 @@ mod board_tests;
 #[cfg(test)]
 mod board_tools_tests;
 #[cfg(test)]
+mod boolean_cap_tests;
+#[cfg(test)]
 mod booleans_tests;
 #[cfg(test)]
 mod bound_alpha_tests;
@@ -233,6 +237,8 @@ mod collab_undo_tests;
 #[cfg(test)]
 mod comment_agent_tests;
 #[cfg(test)]
+mod comment_link_tests;
+#[cfg(test)]
 mod comment_mentions_tests;
 #[cfg(test)]
 mod comments_polish_tests;
@@ -349,6 +355,8 @@ mod interaction_rollback_tests;
 #[cfg(test)]
 mod invite_links_tests;
 #[cfg(test)]
+mod jam_open_tests;
+#[cfg(test)]
 mod keymap_tests;
 #[cfg(test)]
 mod library_path_tests;
@@ -411,6 +419,8 @@ mod pen_end_tests;
 #[cfg(test)]
 mod pen_overlay_tests;
 #[cfg(test)]
+mod pen_vector_edit_tests;
+#[cfg(test)]
 mod penpot_export_tests;
 #[cfg(test)]
 mod pixel_preview_tests;

crates/engine/src/live_share_tests.rs

@@ -118,7 +118,7 @@ fn links(n: usize, codes: Option<(&str, &str)>) -> Shared {
         link: format!("figcraft://join?server=wss://share.example:443/&file=f1&token=Share{what}Share{what}0{n}"),
         code: code.map(str::to_string),
     };
-    Shared { edit: link("edit", codes.map(|c| c.0)), view: Some(link("view", codes.map(|c| c.1))) }
+    Shared { edit: link("edit", codes.map(|c| c.0)), view: Some(link("view", codes.map(|c| c.1))), comment: None }
 }
 
 /// The file is on the server and joined: its owner's links, and what others join with (codes
@@ -264,7 +264,7 @@ fn sharing_online_publishes_to_the_server_in_preferences_and_shows_its_code() {
     assert_eq!((&r["status"], &r["syncing"].is_boolean(), &r["code"]), (&json!("live"), &true, &Value::Null));
     assert_eq!(
         r["share"],
-        json!({"via": "online", "access": "edit", "shared": false, "editCode": null, "viewCode": null, "editLink": null, "viewLink": null})
+        json!({"via": "online", "access": "edit", "shared": false, "editCode": null, "viewCode": null, "commentCode": null, "editLink": null, "viewLink": null, "commentLink": null})
     );
     assert!(r["published"]["owner"].is_string(), "still the owner's file on the server");
     assert!(s.message.as_deref().unwrap_or_default().contains("stays on the server"));

crates/engine/src/online_tests.rs

@@ -108,7 +108,7 @@ fn links(file: &str, n: usize) -> Shared {
         link: format!("figcraft://join?server={SERVER}&file={file}&token=Shared{what}Shared{what}0{n}"),
         code: Some(code),
     };
-    Shared { edit: link("edit", format!("NEWED1T0{n}")), view: Some(link("view", format!("NEWV1EW0{n}"))) }
+    Shared { edit: link("edit", format!("NEWED1T0{n}")), view: Some(link("view", format!("NEWV1EW0{n}"))), comment: None }
 }
 
 fn session() -> (Session, Arc<Mutex<Script>>) {

crates/engine/src/pen_end_tests.rs

@@ -1,5 +1,6 @@
-//! Ending a pen path with Escape or Enter (#760): the path's vector stays in edit mode with the pen,
-//! so the next path is drawn into the same layer; another Escape leaves edit mode.
+//! Ending a pen path with Escape (#760): the path's vector stays in edit mode with the pen, so the
+//! next path is drawn into the same layer; another Escape leaves edit mode. (Enter goes on with the
+//! Move tool instead: `pen_vector_edit_tests`.)
 
 use crate::Session;
 use crate::tools::{PointerEvent, PointerKind, ToolId};
@@ -41,8 +42,8 @@ fn drawn(end: fn(&mut Session)) -> (Session, figcraft_doc::NodeId) {
 }
 
 #[test]
-fn escape_and_enter_keep_the_vector_in_edit_mode_with_the_pen() {
-    // Escape, and Enter (its key runs `pen_end`).
+fn escape_keeps_the_vector_in_edit_mode_with_the_pen() {
+    // Escape, and `pen_end` that it runs.
     let ends: [fn(&mut Session); 2] = [|s| s.escape().unwrap(), |s| s.pen_end().unwrap()];
     for end in ends {
         let (mut s, id) = drawn(end);

crates/engine/src/pen_vector_edit_tests.rs

@@ -0,0 +1,121 @@
+//! From the pen into vector edit mode (#1430): a double-click or Enter while the pen is out ends
+//! the path and edits the vector with the Move tool's point handles, instead of drawing on (a
+//! double-click used to leave a zero-length segment, or a stray two-point vector on the layer it
+//! was meant to edit). Escape still keeps the pen (#760).
+
+use serde_json::json;
+
+use crate::Session;
+use crate::tools::{PointerEvent, PointerKind, ToolId};
+use crate::vectoredit::VTool;
+use figcraft_doc::{NodeId, NodeKind};
+
+fn press(s: &mut Session, kinds: &[PointerKind], x: f64, y: f64) {
+    for kind in kinds {
+        s.pointer(&PointerEvent::at(*kind, x, y, 1.0)).unwrap();
+    }
+}
+
+fn click(s: &mut Session, x: f64, y: f64) {
+    press(s, &[PointerKind::Down, PointerKind::Up], x, y);
+}
+
+/// A double-click as the canvas sends it: two clicks, then the double-click.
+fn double_click(s: &mut Session, x: f64, y: f64) {
+    press(s, &[PointerKind::Down, PointerKind::Up, PointerKind::Down, PointerKind::Up, PointerKind::DoubleClick], x, y);
+}
+
+fn layers(s: &Session) -> Vec<NodeId> {
+    s.doc().unwrap().node(s.page().unwrap()).unwrap().children.clone()
+}
+
+/// (points, segments) of vector `id`.
+fn shape(s: &Session, id: NodeId) -> (usize, usize) {
+    match &s.doc().unwrap().node(id).unwrap().kind {
+        NodeKind::Vector { network } => (network.vertices.len(), network.segments.len()),
+        k => panic!("not a vector: {k:?}"),
+    }
+}
+
+/// The layer and tool vector edit mode has (with no pen path open).
+fn editing(s: &Session) -> Option<(NodeId, VTool)> {
+    s.ds().unwrap().vector_edit.as_ref().filter(|v| v.anchor.is_none()).map(|v| (v.id, v.tool))
+}
+
+/// A pen path from (100, 100) to (200, 100), still being drawn; its layer.
+fn drawing() -> (Session, NodeId) {
+    let mut s = Session::new();
+    s.set_tool(ToolId::Pen);
+    click(&mut s, 100.0, 100.0);
+    click(&mut s, 200.0, 100.0);
+    let id = s.tool.pen.as_ref().and_then(|p| p.id).unwrap();
+    (s, id)
+}
+
+#[test]
+fn a_double_click_with_the_pen_on_a_layer_edits_it_instead_of_drawing() {
+    let mut s = Session::new();
+    s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 100, "height": 100})).unwrap();
+    let [rect] = <[NodeId; 1]>::try_from(layers(&s)).unwrap();
+    s.set_tool(ToolId::Pen);
+    double_click(&mut s, 150.0, 100.0);
+    assert_eq!(layers(&s), vec![rect], "no stray vector drawn");
+    assert_eq!(editing(&s), Some((rect, VTool::Move)), "its points are edited with the Move tool");
+    assert!(s.tool.pen.is_none());
+    assert_eq!(s.tool.tool, ToolId::Move);
+    // Off any layer the double-click draws nothing either.
+    let mut s = Session::new();
+    s.set_tool(ToolId::Pen);
+    double_click(&mut s, 400.0, 400.0);
+    assert!(layers(&s).is_empty() && s.tool.pen.is_none() && editing(&s).is_none());
+}
+
+#[test]
+fn a_double_click_while_drawing_ends_the_path_there_and_edits_it() {
+    let (mut s, id) = drawing();
+    let undo = s.undo_depth().0;
+    double_click(&mut s, 200.0, 200.0);
+    assert_eq!(shape(&s, id), (3, 2), "the double-click's point once, no zero-length segment");
+    assert!(s.tool.pen.is_none());
+    assert_eq!(editing(&s), Some((id, VTool::Move)));
+    assert_eq!(s.undo_depth().0, undo + 1, "the path is one undo step");
+}
+
+#[test]
+fn enter_while_drawing_ends_the_path_and_edits_it_with_the_move_tool() {
+    let (mut s, id) = drawing();
+    assert!(s.tool_key("Enter", false, false, false).unwrap());
+    assert!(s.tool.pen.is_none());
+    assert_eq!(editing(&s), Some((id, VTool::Move)));
+    assert_eq!(shape(&s, id), (2, 1));
+    // The next Enter leaves edit mode.
+    assert!(s.tool_key("Enter", false, false, false).unwrap());
+    assert!(s.ds().unwrap().vector_edit.is_none());
+    assert_eq!(s.selection(), vec![id]);
+}
+
+#[test]
+fn in_edit_mode_the_pen_goes_to_the_move_tool_on_a_double_click_or_enter() {
+    // Escape keeps the pen in edit mode (#760); a double-click on the path then edits it rather
+    // than adding a point where it landed.
+    let (mut s, id) = drawing();
+    s.escape().unwrap();
+    assert_eq!(editing(&s), Some((id, VTool::Pen)));
+    double_click(&mut s, 150.0, 100.0);
+    assert_eq!(editing(&s), Some((id, VTool::Move)));
+    assert_eq!(shape(&s, id), (2, 1), "no point split into the segment");
+    // Off the path: no lone point left behind.
+    let (mut s, id) = drawing();
+    s.escape().unwrap();
+    double_click(&mut s, 150.0, 300.0);
+    assert_eq!((editing(&s), shape(&s, id)), (Some((id, VTool::Move)), (2, 1)));
+    // Enter ends the pen's path in edit mode and takes the Move tool; the next one leaves.
+    let (mut s, id) = drawing();
+    s.escape().unwrap();
+    click(&mut s, 150.0, 300.0);
+    click(&mut s, 250.0, 300.0);
+    assert!(s.tool_key("Enter", false, false, false).unwrap());
+    assert_eq!((editing(&s), shape(&s, id)), (Some((id, VTool::Move)), (4, 2)));
+    assert!(s.tool_key("Enter", false, false, false).unwrap());
+    assert!(s.ds().unwrap().vector_edit.is_none());
+}

crates/engine/src/prefs.rs

@@ -29,6 +29,9 @@ pub struct Prefs {
     pub highlight_on_hover: bool,
     pub number_keys_opacity: bool,
     pub show_dimensions: bool,
+    /// A mouse wheel zooms the canvas about the pointer, and Cmd/Ctrl+wheel scrolls it (off: the
+    /// other way round). Trackpads are unchanged.
+    pub scroll_wheel_zoom: bool,
     /// "system" by default; an explicit "light" or "dark" choice stays saved.
     pub theme: String,
     /// The widths of the main window's left and right panels (the interface keeps them within its
@@ -122,6 +125,7 @@ impl Default for Prefs {
             highlight_on_hover: true,
             number_keys_opacity: true,
             show_dimensions: true,
+            scroll_wheel_zoom: false,
             theme: "system".into(),
             panel_widths: (240.0, 240.0),
             pages_height: None,

crates/engine/src/session/exec.rs

@@ -186,12 +186,15 @@ impl Session {
     }
 
     /// A live session's document you may only view (or that is on its way to a server) is
-    /// read-only: refuse an edit (it is rolled back).
+    /// read-only: refuse an edit (it is rolled back). With a comment link, one that changes only
+    /// comment threads goes through.
     fn check_live_access(&self, id: &str, before: &Option<(Arc<Document>, Vec<NodeId>, NodeId, u64)>) -> Result<()> {
         let Some((bdoc, _, _, uid)) = before else { return Ok(()) };
         let Some(ds) = self.docs.iter().find(|d| d.uid == *uid) else { return Ok(()) };
         match self.live_read_only(*uid) {
-            Some(why) if !Arc::ptr_eq(&ds.doc, bdoc) => Err(EngineError::Disabled(id.to_string(), why.into())),
+            Some(why) if !Arc::ptr_eq(&ds.doc, bdoc) && !self.live_comment_edit(*uid, &ds.doc) => {
+                Err(EngineError::Disabled(id.to_string(), why.into()))
+            }
             _ => Ok(()),
         }
     }

crates/engine/src/sharing/branches.rs

@@ -180,6 +180,31 @@ pub fn current(s: &Session, uid: u64) -> Vec<Branch> {
     branches
 }
 
+/// The tabs showing branches of the file in tab `uid`.
+pub fn tabs(s: &Session, uid: u64) -> impl Iterator<Item = &DocState> {
+    s.docs.iter().filter(move |t| t.branch_of.as_ref().is_some_and(|(main, _)| *main == uid))
+}
+
+/// A file whose changes would be lost, so it needs a recovery copy: in its document, its history
+/// or one of its branch tabs (a branch tab's edits are saved with its main file, so the tab has no
+/// copy of its own).
+pub fn needs_copy(s: &Session, d: &DocState) -> bool {
+    d.branch_of.is_none() && (d.is_dirty() || tabs(s, d.uid).any(DocState::is_dirty))
+}
+
+/// What a recovery copy of `d` holds: the document itself (not a version or live text preview
+/// shown in its place: a crash during a preview must bring the file back, not the version),
+/// its version history and its branches as they are now ([`current`]).
+pub fn recovery_copy(s: &Session, d: &DocState) -> (Arc<Document>, Vec<figcraft_format::Version>, Vec<Branch>) {
+    (d.document_itself().clone(), d.versions.clone(), current(s, d.uid))
+}
+
+/// The revisions a recovery copy of `d` is of: its own and its branch tabs' (a branch tab's edits
+/// change only its own revision). A new copy is due when they change.
+pub fn stamp(s: &Session, d: &DocState) -> Vec<u64> {
+    std::iter::once(d.revision).chain(tabs(s, d.uid).map(|t| t.revision)).collect()
+}
+
 /// The tab a save writes: the main file's for a branch tab, else the active tab.
 pub fn file_tab(s: &Session) -> usize {
     s.ds().ok().and_then(|d| d.branch_of.as_ref()).and_then(|(m, _)| s.docs.iter().position(|d| d.uid == *m)).unwrap_or(s.active)

crates/engine/src/sharing/live.rs

@@ -386,6 +386,24 @@ pub const NO_TEAM_LIBRARIES: &str = "team libraries on a server need the desktop
 
 /// Why an edit of a server file you joined with a view link is refused.
 pub const VIEW_ONLY: &str = "View only: the link you joined with lets you see this file, not change it";
+/// Why an edit other than to comment threads of a server file you joined with a comment link is
+/// refused.
+pub const COMMENT_ONLY: &str = "Can comment: the link you joined with lets you see this file and comment on it, not change it";
+
+/// Why a session document can't be edited with `access` (`None`: it can).
+pub(crate) fn limited_by(access: Access) -> Option<&'static str> {
+    match access {
+        Access::Edit => None,
+        Access::View => Some(VIEW_ONLY),
+        Access::Comment => Some(COMMENT_ONLY),
+    }
+}
+
+/// Whether `tool` is one a file you can't edit now leaves you (`why`: what
+/// [`Session::live_read_only`] says): moving around and, with a comment link, commenting.
+pub fn tool_left(why: &str, tool: ToolId) -> bool {
+    matches!(tool, ToolId::Move | ToolId::Hand) || (why == COMMENT_ONLY && tool == ToolId::Comment)
+}
 /// Why an edit of a file on its way to a server is refused.
 pub const PUBLISHING: &str = "The file is being published: you can edit it again once it is on the server";
 

crates/engine/src/sharing/live/events.rs

@@ -2,7 +2,7 @@ use super::host::{clean_hosting, clean_shared};
 use super::types::{Event, PageInfo, Role, SessionId, Status};
 use super::{
     Access, Arc, Client, ClientMsg, DocState, Document, EngineError, Live, NodeId, PUBLISHING, Peer, Presence, Result, ServerMsg, Session, Signal,
-    ToolId, VIEW_ONLY, display_name, editing_hint, editing_of, sanitize,
+    ToolId, display_name, editing_hint, editing_of, limited_by, sanitize, tool_left,
 };
 
 /// What a live session's document holding components that contain each other's instances shows
@@ -410,8 +410,11 @@ impl Session {
             }
             _ => return,
         };
-        // A view link: the editing tools are off.
-        if client.access() == Access::View && self.tab() == Some(uid) && !matches!(self.tool.tool, ToolId::Move | ToolId::Hand) {
+        // A view or comment link: the editing tools are off.
+        if let Some(why) = limited_by(client.access())
+            && self.tab() == Some(uid)
+            && !tool_left(why, self.tool.tool)
+        {
             self.set_tool(ToolId::Move);
         }
         let Some(live) = self.live.sessions.get_mut(&id) else { return };
@@ -439,18 +442,25 @@ impl Session {
         self.message = Some(reason);
     }
 
-    /// Why document `uid` can't be edited now, if it can't: you may only view it in its session,
-    /// or it is on its way to a server.
+    /// Why document `uid` can't be edited now, if it can't: you may only view it (or view and
+    /// comment on it: see [`live_comment_edit`](Self::live_comment_edit)) in its session, or it is
+    /// on its way to a server.
     pub fn live_read_only(&self, uid: u64) -> Option<&'static str> {
         let live = self.live.doc(uid)?;
         match &live.client {
-            Some(c) if c.access() == Access::View => Some(VIEW_ONLY),
-            Some(_) => None,
+            Some(c) => limited_by(c.access()),
             None if live.status.active() && live.role() != Some(Role::Host) => Some(PUBLISHING),
             None => None,
         }
     }
 
+    /// Whether `doc` is session document `uid` with nothing but comment threads changed, and the
+    /// link it was joined with lets you comment: such an edit goes through where others are refused.
+    pub(crate) fn live_comment_edit(&self, uid: u64, doc: &Document) -> bool {
+        let client = self.live.doc(uid).and_then(|l| l.client.as_ref());
+        client.is_some_and(|c| c.access() == Access::Comment && c.changes_only_comments(doc))
+    }
+
     /// The active document can't be edited now (see [`live_read_only`](Self::live_read_only)).
     pub fn active_read_only(&self) -> Option<&'static str> {
         self.tab().and_then(|uid| self.live_read_only(uid))

crates/engine/src/sharing/live/host.rs

@@ -82,8 +82,17 @@ fn start_publish(s: &mut Session, cmd: &str, server: &str, p: &Value, share: boo
     Ok(id)
 }
 
-/// Why a local network share has no view-only code.
-const LOCAL_EDITS: &str = "everyone who joins a file shared on the local network can edit it: share it online for a view-only code";
+/// Why a local network share has no view-only or comment code.
+const LOCAL_EDITS: &str = "everyone who joins a file shared on the local network can edit it: share it online for a view-only or comment code";
+
+/// Why a share has no comment code.
+const NO_COMMENT_LINK: &str =
+    "this share has no comment code (its server is older than comment links, or it was shared before them): stop sharing and share again";
+
+/// The `access` parameter of `cmd`, if given.
+fn access_param(cmd: &str, p: &Value) -> Result<Option<Access>> {
+    string(p, "access").map(|a| Access::parse(&a).ok_or_else(|| bad(cmd, "`access` must be \"edit\", \"comment\" or \"view\""))).transpose()
+}
 
 pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
     const CMD: &str = "collab.share";
@@ -92,10 +101,7 @@ pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
         Some("local") => Via::Local,
         Some(_) => return Err(bad(CMD, "`via` must be \"online\" or \"local\"")),
     };
-    let access = match string(p, "access") {
-        None => None,
-        Some(a) => Some(Access::parse(&a).ok_or_else(|| bad(CMD, "`access` must be \"edit\" or \"view\""))?),
-    };
+    let access = access_param(CMD, p)?;
     // A share of this file is there already: only which code is shown changes, or (the file is
     // on the server, not shared at the moment) it gets new links and codes. Nothing is published
     // twice.
@@ -106,7 +112,7 @@ pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
                 "this file is shared the other way already: stop sharing it first (collab.stop), or leave its session (collab.leave)",
             ));
         }
-        if access == Some(Access::View) && shared == Via::Local {
+        if access.is_some_and(|a| a != Access::Edit) && shared == Via::Local {
             return Err(bad(CMD, LOCAL_EDITS));
         }
         if let Some(req) = share_request(s, Vec::new(), true).filter(|_| s.live().share.as_ref().is_some_and(|sh| sh.links.is_none())) {
@@ -114,6 +120,9 @@ pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
             shared_now(s, Some(clean_shared(links)));
         }
         if let (Some(a), Some(sh)) = (access, s.live_mut().and_then(|l| l.share.as_mut())) {
+            if sh.links.as_ref().is_some_and(|l| l.of(a).is_none()) {
+                return Err(bad(CMD, NO_COMMENT_LINK));
+            }
             sh.access = a;
         }
         return Ok(s.live_summary());
@@ -121,7 +130,7 @@ pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
     let access = access.unwrap_or_default();
     let mut server = String::new();
     let id = match via {
-        Via::Local if access == Access::View => return Err(bad(CMD, LOCAL_EDITS)),
+        Via::Local if access != Access::Edit => return Err(bad(CMD, LOCAL_EDITS)),
         Via::Local => start_host(s, CMD, p, Bind::Lan, true)?,
         Via::Online => {
             server = string(p, "server").unwrap_or_else(|| s.prefs.publish_server.clone()).trim().to_string();
@@ -136,7 +145,7 @@ pub(super) fn share(s: &mut Session, p: &Value) -> Result<Value> {
         // what the link says: where the session is and its secret. (Online they arrive with the file.)
         let links = live.info.as_ref().filter(|_| via == Via::Local).map(|i| {
             let code = code::Local::from_link(&i.link).map(|l| l.code());
-            Shared { edit: SharedLink { id: String::new(), link: i.link.clone(), code }, view: None }
+            Shared { edit: SharedLink { id: String::new(), link: i.link.clone(), code }, ..Shared::default() }
         });
         live.share = Some(Share { via, server, access, links });
     }
@@ -155,6 +164,7 @@ pub(crate) fn clean_shared(mut links: Shared) -> Shared {
     let clean = |l: &mut SharedLink| l.code = l.code.take().and_then(|c| code::normalize(&c).ok()).filter(|c| c.len() == code::ONLINE_LEN);
     clean(&mut links.edit);
     links.view.as_mut().map(clean);
+    links.comment.as_mut().map(clean);
     links
 }
 
@@ -217,11 +227,7 @@ pub(super) fn manage_file(s: &mut Session, p: &Value) -> Result<Value> {
         "unshare" => return crate::online::set_shared(s, CMD, &server, &file, &owner, false),
         "links" => Admin::Links { file: file.clone() },
         "createLink" => {
-            let access = match string(p, "access").as_deref().map(str::trim) {
-                None | Some("view") => Access::View,
-                Some("edit") => Access::Edit,
-                Some(_) => return Err(bad(CMD, "`access` must be \"edit\" or \"view\"")),
-            };
+            let access = access_param(CMD, p)?.unwrap_or(Access::View);
             Admin::CreateLink { file: file.clone(), access, label: string(p, "label").unwrap_or_default() }
         }
         "revoke" => Admin::Revoke { file: file.clone(), link: req_string(CMD, p, "link")?.trim().to_string() },

crates/engine/src/sharing/live/join.rs

@@ -1,5 +1,5 @@
 use super::host::{host, hosting_request, manage_file, publish, report, server_info, share, share_answer, share_request, shared_now, unavailable};
-use super::types::{Published, Role, SessionId, Share, Status, join_target};
+use super::types::{Published, Role, SessionId, Share, Shared, Status, join_target};
 use super::{
     Admin, AdminReply, CommandSpec, EngineError, Live, Result, Resume, Session, Value, bad, display_name, get, json, num, req_string, spec, string,
 };
@@ -53,7 +53,7 @@ fn stop(s: &mut Session, p: &Value) -> Result<Value> {
     // their codes) away, which disconnects whoever joined with them, and the owner stays in the
     // file. Anything else (a session this app hosts, one it joined, a file not shared now) ends.
     let shared = s.live().share.as_ref().and_then(|sh| sh.links.as_ref());
-    let ids: Vec<String> = shared.into_iter().flat_map(|l| [Some(&l.edit), l.view.as_ref()]).flatten().map(|l| l.id.clone()).collect();
+    let ids: Vec<String> = shared.into_iter().flat_map(Shared::all).map(|l| l.id.clone()).collect();
     let Some(req) = share_request(s, ids, false).filter(|_| shared.is_some()) else { return leave(s, p) };
     share_answer(s, req).map_err(|e| EngineError::Other(format!("couldn't stop sharing (the others can still get in): {e}")))?;
     shared_now(s, None);
@@ -174,7 +174,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.share",
             "Share with a join code",
-            "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), key? (online: the server's key, when the address has none), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, editLink, viewLink}: links made for others (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing",
+            "{via?: \"online\" (default: the open file is kept on the server in Preferences, `pref.publishServer`, and the server hands out the codes) | \"local\" (this app hosts it for people on the same network; no server), access?: \"edit\" (default) | \"comment\" (online only: view and comment, not edit) | \"view\" (online only: which code `code` is), server? (online: another server's address, with ?key=… when it needs its key), key? (online: the server's key, when the address has none), name? (online: the file's name there), port?, bind?: \"lan\" (default) | \"local\" (this machine only) | an IP address (local)} → the open file's session as `collab.peers` gives it: `code` is what others type to join (`collab.join {code}`, the Join field), `link` an invite link. Local: at once, the code carries the host's IPv4 address, port and secret (K7M2Q-…, 20 symbols). Online: status \"connecting\" first; once live `code` is the server's (K7M-2QX-9TD) and `share` is {via, access, shared, editCode, viewCode, commentCode, editLink, viewLink, commentLink}: links made for others (commentCode and commentLink null from a server older than comment links) (the file's own links in `published` stay yours); `code` stays null when the server is older than join codes (give `share.editLink` / `share.viewLink` instead). Called again while shared: only switches which code `code` is (`access`). Called after `collab.stop` on a file that is still on the server: new links and codes for that same file (nothing is published twice). `collab.stop` stops sharing",
             false,
             share
         ),
@@ -216,7 +216,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.manageFile",
             "Manage published file",
-            "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink | revoke | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename | remove (the file is deleted from the server and leaves your online files), access? (createLink: view (default) | edit), label? (createLink), link? (revoke: a link's id), name? (rename)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, editLink, viewLink} | {done}",
+            "{file (its id), server?, owner? (the server's address and the owner token you got when you published the file without the key; each defaults to what is recorded for one of your online files, see `collab.files`), action: links | createLink | revoke | share (others get new links and join codes) | unshare (the links and codes others got are revoked; the file stays yours on the server) | rename | remove (the file is deleted from the server and leaves your online files), access? (createLink: view (default) | comment | edit), label? (createLink), link? (revoke: a link's id), name? (rename)} → {links: [{id, access, label, created}]} | {id, access, link (a new share link, shown once)} | {shared, code, viewCode, commentCode, editLink, viewLink, commentLink} | {done}",
             false,
             manage_file
         ),
@@ -230,7 +230,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.peers",
             "People in this file",
-            "{} → the open file's session (each open file has its own or none): {available, canHost (false in the browser: it can only join), session (its id; null: this file isn't in a session), status: off|connecting|live|reconnecting|failed, reason?, role, me, name, color, fileName, pages, link, url, address, network, access: edit|view (view: a server's view link; edits are refused), readOnly (the open file can't be edited now: view only, or being published), published (a file you published: {server, file, name, edit, view}), code (the join code to give others, when the file was shared with `collab.share`; null otherwise or while it is on its way), share ({via: online|local, access, shared (false: a file of yours on a server that nobody else gets into now), editCode, viewCode, editLink, viewLink (what others join with; null when not shared)} or null), follow, syncing, document (index of the synced open document), editing (the text layer you type in: {node, layer, caret, selection}), peers: [{id, name, color, editing (theirs, same shape, or null), presence: {cursor: [page, x, y], selection, viewport: [page, x, y, w, h], editing_text}}], sessions: [{session, document (index of its open document; null for a join on its way), status, reason, role, fileName, link, peers (how many others)}] (every session of the app)}",
+            "{} → the open file's session (each open file has its own or none): {available, canHost (false in the browser: it can only join), session (its id; null: this file isn't in a session), status: off|connecting|live|reconnecting|failed, reason?, role, me, name, color, fileName, pages, link, url, address, network, access: edit|view|comment (view: a server's view link, edits are refused; comment: its comment link, edits other than to comment threads are refused), readOnly (the open file can't be edited now: view only, comment only, or being published), published (a file you published: {server, file, name, edit, view}), code (the join code to give others, when the file was shared with `collab.share`; null otherwise or while it is on its way), share ({via: online|local, access, shared (false: a file of yours on a server that nobody else gets into now), editCode, viewCode, commentCode, editLink, viewLink, commentLink (what others join with; null when not shared)} or null), follow, syncing, document (index of the synced open document), editing (the text layer you type in: {node, layer, caret, selection}), peers: [{id, name, color, editing (theirs, same shape, or null), presence: {cursor: [page, x, y], selection, viewport: [page, x, y, w, h], editing_text}}], sessions: [{session, document (index of its open document; null for a join on its way), status, reason, role, fileName, link, peers (how many others)}] (every session of the app)}",
             false,
             |s, _| Ok(s.live_summary())
         ),

crates/engine/src/sharing/live/sync.rs

@@ -1,7 +1,7 @@
 use super::host::hosting_json;
 use super::types::{NO_SESSION, SessionId, Share, SharedLink, Status};
 use super::{
-    Arc, Client, EditingText, EngineError, EngineRecompute, FollowedView, Live, Result, Session, Value, ViewState, bad, busy, close, code,
+    Access, Arc, Client, EditingText, EngineError, EngineRecompute, FollowedView, Live, Result, Session, Value, ViewState, bad, busy, close, code,
     editing_of, json,
 };
 
@@ -45,7 +45,7 @@ impl Session {
             return;
         }
         let (id, doc, synced) = (live.id, ds.doc.clone(), live.synced.clone());
-        if self.live_read_only(uid).is_some() {
+        if self.live_read_only(uid).is_some() && !self.live_comment_edit(uid, &doc) {
             // Nothing goes out: what changed outside a command is put back.
             if let Some(s) = synced {
                 self.apply_remote(uid, s);
@@ -322,7 +322,8 @@ impl Session {
             "published": l.published,
             "code": l.share.as_ref().and_then(Share::code),
             "share": l.share.as_ref().map(|sh| {
-                let (edit, view) = (sh.links.as_ref().map(|s| &s.edit), sh.links.as_ref().and_then(|s| s.view.as_ref()));
+                let of = |a| sh.links.as_ref().and_then(|s| s.of(a));
+                let (edit, view, comment) = (of(Access::Edit), of(Access::View), of(Access::Comment));
                 let code = |l: Option<&SharedLink>| l.and_then(|l| l.code.as_deref()).map(code::grouped);
                 json!({
                     "via": sh.via,
@@ -330,8 +331,10 @@ impl Session {
                     "shared": sh.links.is_some(),
                     "editCode": code(edit),
                     "viewCode": code(view),
+                    "commentCode": code(comment),
                     "editLink": edit.map(|l| l.link.as_str()),
                     "viewLink": view.map(|l| l.link.as_str()),
+                    "commentLink": comment.map(|l| l.link.as_str()),
                 })
             }),
             "hosting": l.hosting.as_ref().map(hosting_json),

crates/engine/src/sharing/live/types.rs

@@ -117,7 +117,8 @@ pub struct SessionInfo {
     pub address: String,
     /// Reachable from other machines (the host listens on a network address, not loopback).
     pub network: bool,
-    /// What you may do: edit, or only view (a server's view link).
+    /// What you may do: edit, only view (a server's view link), or view and comment (its comment
+    /// link).
     pub access: Access,
 }
 
@@ -164,12 +165,31 @@ pub struct SharedLink {
     pub code: Option<String>,
 }
 
-/// What others join a shared file with: to edit it and, on a server, to only view it.
+/// What others join a shared file with: to edit it and, on a server, to only view it or to view
+/// and comment on it.
 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, serde::Deserialize)]
 #[serde(default)]
 pub struct Shared {
     pub edit: SharedLink,
     pub view: Option<SharedLink>,
+    /// `None`: a server older than comment links, or a share made before them.
+    pub comment: Option<SharedLink>,
+}
+
+impl Shared {
+    /// The way in that gives `access`, if there is one.
+    pub fn of(&self, access: Access) -> Option<&SharedLink> {
+        match access {
+            Access::Edit => Some(&self.edit),
+            Access::View => self.view.as_ref(),
+            Access::Comment => self.comment.as_ref(),
+        }
+    }
+
+    /// Every way in (what stopping revokes).
+    pub fn all(&self) -> impl Iterator<Item = &SharedLink> {
+        std::iter::once(&self.edit).chain(&self.view).chain(&self.comment)
+    }
 }
 
 /// A file shared with a join code (`collab.share`). Online, the file is its owner's on the
@@ -190,8 +210,7 @@ pub struct Share {
 impl Share {
     /// The way in to show: the one for [`access`](Self::access).
     fn shown(&self) -> Option<&SharedLink> {
-        let links = self.links.as_ref()?;
-        if self.access == Access::View { links.view.as_ref() } else { Some(&links.edit) }
+        self.links.as_ref()?.of(self.access)
     }
 
     /// The code to show, grouped (`K7M-2QX-9TD`).

crates/engine/src/sharing/online.rs

@@ -161,6 +161,7 @@ impl OnlineFile {
         let usable = |l: &SharedLink| link(&l.link) && l.id.len() <= 64 && !l.id.contains(char::is_control);
         self.shared = self.shared.take().filter(|s| usable(&s.edit)).map(|mut s| {
             s.view = s.view.take().filter(usable);
+            s.comment = s.comment.take().filter(usable);
             clean_shared(s)
         });
         Some(self)
@@ -384,9 +385,7 @@ impl Session {
         let Some(f) = self.prefs.online_files.get_mut(i) else { return };
         let before = (f.name.clone(), f.shared.is_some());
         // Links revoked from elsewhere no longer let anyone in.
-        let live = |f: &OnlineFile, o: &OwnedFile| {
-            f.shared.as_ref().is_some_and(|s| o.links.contains(&s.edit.id) && s.view.as_ref().is_none_or(|v| o.links.contains(&v.id)))
-        };
+        let live = |f: &OnlineFile, o: &OwnedFile| f.shared.as_ref().is_some_and(|s| s.all().all(|l| o.links.contains(&l.id)));
         let shared = match answer {
             Some(o) => {
                 f.state = FileState::Online;
@@ -429,6 +428,7 @@ impl Session {
                     "shared": f.shared.is_some(),
                     "code": code(f.shared.as_ref().map(|s| &s.edit)),
                     "viewCode": code(f.shared.as_ref().and_then(|s| s.view.as_ref())),
+                    "commentCode": code(f.shared.as_ref().and_then(|s| s.comment.as_ref())),
                     "state": f.state,
                     "expires": f.expires,
                     "session": session.map(|l| l.id),
@@ -590,14 +590,16 @@ pub(crate) fn removed(s: &mut Session, server: &str, file: &str) {
 pub(crate) fn set_shared(s: &mut Session, cmd: &str, server: &str, file: &str, owner: &str, on: bool) -> Result<Value> {
     let shared = s.online_index(server, file).and_then(|i| s.prefs.online_files.get(i)).and_then(|f| f.shared.clone());
     let result = |links: Option<&Shared>| {
-        let (edit, view) = (links.map(|l| &l.edit), links.and_then(|l| l.view.as_ref()));
+        let (edit, view, comment) = (links.map(|l| &l.edit), links.and_then(|l| l.view.as_ref()), links.and_then(|l| l.comment.as_ref()));
         let code = |l: Option<&SharedLink>| l.and_then(|l| l.code.as_deref()).map(code::grouped);
         json!({
             "shared": links.is_some(),
             "code": code(edit),
             "viewCode": code(view),
+            "commentCode": code(comment),
             "editLink": edit.map(|l| l.link.as_str()),
             "viewLink": view.map(|l| l.link.as_str()),
+            "commentLink": comment.map(|l| l.link.as_str()),
         })
     };
     // As asked already: nothing to tell the server.
@@ -607,7 +609,7 @@ pub(crate) fn set_shared(s: &mut Session, cmd: &str, server: &str, file: &str, o
     if !on && shared.is_none() {
         return Err(bad(cmd, "the links others got aren't recorded on this computer: revoke them by id (action: links, then revoke)"));
     }
-    let revoke: Vec<String> = shared.iter().flat_map(|l| [Some(&l.edit), l.view.as_ref()]).flatten().map(|l| l.id.clone()).collect();
+    let revoke: Vec<String> = shared.iter().flat_map(Shared::all).map(|l| l.id.clone()).collect();
     let req = NetRequest::Share { server: server.to_string(), file: file.to_string(), owner: owner.to_string(), revoke, create: on };
     let links = crate::live::share_answer(s, req)?.map(clean_shared);
     if on && links.is_none() {
@@ -622,7 +624,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "collab.files",
             "Your online files",
-            "{refresh? (true: ask each server once what became of its files; default false: no network)} → {files: [{server, host, file (its id), name, created, opened (seconds since 1970), shared (others get in with a code now), code, viewCode (the join codes, while shared), state: unknown | online | gone (the server no longer has it: removed or expired), expires, session, document (the session and open document it is in now, or null), hasOwnerToken: true}], problems: [{server, host, state: offline | unsupported (a server older than this request), reason}], max}: the files you published or shared without a server key, most recently opened first, as recorded on this computer with the preferences. Owner tokens and your own links are never in the result",
+            "{refresh? (true: ask each server once what became of its files; default false: no network)} → {files: [{server, host, file (its id), name, created, opened (seconds since 1970), shared (others get in with a code now), code, viewCode, commentCode (the join codes, while shared; commentCode null from a server older than comment links), state: unknown | online | gone (the server no longer has it: removed or expired), expires, session, document (the session and open document it is in now, or null), hasOwnerToken: true}], problems: [{server, host, state: offline | unsupported (a server older than this request), reason}], max}: the files you published or shared without a server key, most recently opened first, as recorded on this computer with the preferences. Owner tokens and your own links are never in the result",
             false,
             files
         ),

crates/engine/src/unknown_params_tests.rs

@@ -89,3 +89,21 @@ fn batch_steps_refuse_unknown_keys_and_parameters() {
         err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "comment.list", "params": {"filterr": "open"}}]}));
     assert!(e.contains("comment.list") && e.contains("step 1: unknown parameter `filterr`"), "{e}");
 }
+
+#[test]
+fn commands_that_cant_run_without_parameters_say_so() {
+    // A parameter without `?` (or one of alternatives) is needed; `?` ones and none aren't.
+    // (`file.placeImage` too: its menu item asks for the file, a call has to name it.)
+    for id in ["image.place", "file.placeImage", "comment.add", "comment.reply", "tool.set"] {
+        assert!(crate::cmd::needs_params(id), "{id}");
+    }
+    for id in ["view.zoomIn", "edit.delete", "edit.undo", "no.such.command"] {
+        assert!(!crate::cmd::needs_params(id), "{id}");
+    }
+    // What it says holds: such a command refuses no parameters.
+    let mut s = Session::new();
+    s.execute("file.new", &json!({})).unwrap();
+    for id in ["image.place", "comment.add", "tool.set"] {
+        assert!(s.execute(id, &json!({})).is_err(), "{id} ran without its parameters");
+    }
+}

crates/engine/src/variables_io_tests.rs

@@ -270,6 +270,36 @@ fn variables_import_checks_what_it_reads() {
     healthy(&s);
 }
 
+#[test]
+fn variables_import_caps_the_file_it_reads() {
+    let dir = std::env::temp_dir().join(format!("figcraft-vars-io-cap-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let mut s = Session::new();
+    theme(&mut s);
+    let small = dir.join("theme.json").to_string_lossy().into_owned();
+    ex(&mut s, "variable.export", json!({"path": small}));
+    let mut t = Session::new();
+    ex(&mut t, "variable.createCollection", json!({"name": "Other"}));
+    // One byte over the limit (sparse, so nothing is written), a folder, and a device: refused,
+    // nothing changes.
+    let big = dir.join("big.json");
+    std::fs::File::create(&big).unwrap().set_len(256 * 1024 * 1024 + 1).unwrap();
+    let mut refused = vec![big.to_string_lossy().into_owned(), dir.to_string_lossy().into_owned()];
+    if cfg!(unix) {
+        refused.push("/dev/null".to_string());
+    }
+    for bad in &refused {
+        let before = (format!("{:?}", t.docs), t.active);
+        let e = t.execute("variable.import", &json!({"variables": bad})).unwrap_err().to_string();
+        assert!(e.contains("not a file of at most 268435456 bytes"), "{bad}: {e}");
+        assert_eq!((format!("{:?}", t.docs), t.active), before, "{bad}");
+    }
+    // A file under the limit still imports.
+    ex(&mut t, "variable.import", json!({"variables": small}));
+    assert_eq!(t.doc().unwrap().variables.collections.len(), 2);
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 /// A rectangle filled with a three-stop linear gradient, and colour variables `brand` and `ink`.
 fn gradient_rect(s: &mut Session) -> NodeId {
     let r = idv(&ex(s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40})));