mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-08 11:52:10 +00:00
Merge pull request #5130 from dotCooCoo/fix/vars-p3
Variables: a command that changes nothing adds no undo step
This commit is contained in:
BFlat
@@ -87,16 +87,20 @@ pub(super) fn to_csv(g: &GraphSpec) -> String {
|
||||
}
|
||||
|
||||
/// Apply CSV graph data to the graph root `id` and rebuild its art (Variables data merge), as `graph.setData` with
|
||||
/// `csv` does.
|
||||
/// `csv` does. Data the graph already holds leaves the graph as it is.
|
||||
pub(crate) fn apply_csv(d: &mut Document, id: NodeId, csv: &str) -> Result<()> {
|
||||
let Some(node) = d.node(id) else {
|
||||
return Err(EngineError::NoNode(id));
|
||||
};
|
||||
let Some(mut spec) = node.extras().graph.as_deref().cloned() else {
|
||||
let Some(current) = node.extras().graph.as_deref() else {
|
||||
return Err(bad("graph.applyCsv", format!("node {} is not a graph", id.0)));
|
||||
};
|
||||
let mut spec = current.clone();
|
||||
apply_data(&mut spec, &json!({ "csv": csv }));
|
||||
keep_right_series(&mut spec);
|
||||
if spec == *current {
|
||||
return Ok(());
|
||||
}
|
||||
regenerate(d, id, spec)
|
||||
}
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
|
||||
use serde_json::{Value, json};
|
||||
use vectorcraft_doc::{DataSet, DataValue, NodeId, NodeKind, Variable, VariableKind, Variables};
|
||||
use vectorcraft_doc::{DataSet, DataValue, Document, NodeId, NodeKind, Selection, Variable, VariableKind, Variables};
|
||||
|
||||
use super::edit::selected_roots;
|
||||
use super::fileio::{absolute_path, read_file_head, write_file};
|
||||
@@ -44,6 +44,66 @@ fn selection_and_unlocked(s: &Session) -> std::result::Result<(), String> {
|
||||
variables_unlocked(s)
|
||||
}
|
||||
|
||||
/// Ends a typing session as [`Session::edit`] would, before a helper below compares the document,
|
||||
/// so the typing is its own undo step whether or not the command then changes anything.
|
||||
fn settle_typing(s: &mut Session) -> Result<()> {
|
||||
if !s.previewing && super::typing_in_progress(s) {
|
||||
super::settle_interaction(s)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// [`Session::edit`] for a command that changes only the variables and data sets and may change
|
||||
/// nothing, such as binding art to the variable it already has: `f` runs on a copy of them, and
|
||||
/// only a copy that differs becomes an undo step. When nothing changed, the undo and redo steps and
|
||||
/// the saved state stay as they were.
|
||||
fn edit_variables<T>(s: &mut Session, label: &str, f: impl FnOnce(&mut Variables) -> Result<T>) -> Result<T> {
|
||||
settle_typing(s)?;
|
||||
let st = s.doc()?;
|
||||
let mut vars = st.doc.variables.clone();
|
||||
let v = f(&mut vars)?;
|
||||
if vars != st.doc.variables {
|
||||
s.edit(label, |d, _| {
|
||||
d.variables = vars;
|
||||
Ok(())
|
||||
})?;
|
||||
}
|
||||
Ok(v)
|
||||
}
|
||||
|
||||
/// [`Session::edit`] for applying a data set, which may change nothing when the art already shows
|
||||
/// the active row: `f` runs on a copy of the document and the selection, and only a document that
|
||||
/// differs becomes an undo step, with the selection `f` left. When the document did not change, the
|
||||
/// undo and redo steps and the saved state stay as they were, and a selection `f` changed is set
|
||||
/// without an undo step. When the active row is applied again and the art already shows it,
|
||||
/// threaded type is compared after its story flows through its frames, as [`Session::edit`] flows
|
||||
/// it; any other change is an undo step without that extra flow.
|
||||
fn edit_if_changed<T>(s: &mut Session, label: &str, f: impl FnOnce(&mut Document, &mut Selection) -> Result<T>) -> Result<T> {
|
||||
settle_typing(s)?;
|
||||
let st = s.doc()?;
|
||||
let mut doc = (*st.doc).clone();
|
||||
let mut sel = st.selection.clone();
|
||||
let v = f(&mut doc, &mut sel)?;
|
||||
let vars = &st.doc.variables;
|
||||
if doc != *st.doc
|
||||
&& !doc.text_threads.is_empty()
|
||||
&& doc.variables == *vars
|
||||
&& vars.active_dataset.as_deref().and_then(|n| vars.dataset(n)).is_some_and(|ds| ds.matches(&st.doc.captured_values()))
|
||||
{
|
||||
super::threads::reflow(&st.doc, &mut doc);
|
||||
}
|
||||
if doc != *st.doc {
|
||||
s.edit(label, |d, selection| {
|
||||
*d = doc;
|
||||
*selection = sel;
|
||||
Ok(())
|
||||
})?;
|
||||
} else if sel != st.selection {
|
||||
s.select(|_, selection| *selection = sel)?;
|
||||
}
|
||||
Ok(v)
|
||||
}
|
||||
|
||||
fn var_name(p: &Value, cmd: &str) -> Result<String> {
|
||||
named(str_param(p, "name"), "name", cmd)
|
||||
}
|
||||
@@ -228,7 +288,7 @@ fn delete(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
// Without `names`, the highlighted variable: what the panel's bin and the menu item delete.
|
||||
let highlighted = s.doc()?.variables_highlight.clone();
|
||||
let names = names_param(p, "variable.delete", highlighted, "no variable highlighted")?;
|
||||
let n = s.edit("Delete Variables", |d, _| Ok(d.variables.prune(&names)))?;
|
||||
let n = edit_variables(s, "Delete Variables", |v| Ok(v.prune(&names)))?;
|
||||
// A highlight of a variable that is gone highlights nothing.
|
||||
let st = s.doc_mut()?;
|
||||
st.drop_stale_variables_highlight();
|
||||
@@ -277,7 +337,7 @@ fn bind(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
check_bindable(doc, &ids, kind, C)?;
|
||||
// Binding again is not a second binding, and says so rather than counting as a change.
|
||||
let already: Vec<u64> = ids.iter().filter(|id| doc.variables.objects_of(&name).contains(id)).map(|id| id.0).collect();
|
||||
let n = s.edit("Bind Variable", |d, _| Ok(ids.iter().filter(|id| d.variables.bind(**id, &name)).count()))?;
|
||||
let n = edit_variables(s, "Bind Variable", |v| Ok(ids.iter().filter(|id| v.bind(**id, &name)).count()))?;
|
||||
Ok(json!({"variable": name, "ids": ids.iter().map(|i| i.0).collect::<Vec<_>>(), "bound": n, "already": already}))
|
||||
}
|
||||
|
||||
@@ -285,13 +345,13 @@ fn unbind(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
const C: &str = "variable.unbind";
|
||||
let ids = checked_ids_or_selection(s, p, C)?;
|
||||
let name = defined_variable(s, p, C)?;
|
||||
let n = s.edit("Unbind Variable", |d, _| {
|
||||
let n = edit_variables(s, "Unbind Variable", |vars| {
|
||||
let mut n = 0;
|
||||
for id in &ids {
|
||||
// With `variable` only that one binding goes; without it, every one the object has.
|
||||
match &name {
|
||||
Some(v) => n += usize::from(d.variables.unbind(*id, v)),
|
||||
None => n += usize::from(d.variables.bindings.remove(id).is_some()),
|
||||
Some(v) => n += usize::from(vars.unbind(*id, v)),
|
||||
None => n += usize::from(vars.bindings.remove(id).is_some()),
|
||||
}
|
||||
}
|
||||
Ok(n)
|
||||
@@ -344,8 +404,8 @@ fn dataset_set(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
const C: &str = "dataset.set";
|
||||
let name = var_name(p, C)?;
|
||||
let values = values_param(&s.doc()?.doc.variables, p, C)?;
|
||||
let (set, removed) = s.edit("Edit Data Set", |d, _| {
|
||||
let ds = d.variables.datasets.iter_mut().find(|x| x.name == name).ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?;
|
||||
let (set, removed) = edit_variables(s, "Edit Data Set", |v| {
|
||||
let ds = v.datasets.iter_mut().find(|x| x.name == name).ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?;
|
||||
let removed: Vec<String> = ds.values.keys().filter(|k| !values.contains_key(*k)).cloned().collect();
|
||||
ds.values = values.clone();
|
||||
Ok((values.len(), removed))
|
||||
@@ -415,8 +475,8 @@ fn dataset_update(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
if values.is_empty() {
|
||||
return Err(bad(C, "bind something to a variable first"));
|
||||
}
|
||||
let (written, left) = s.edit("Update Data Set", |d, _| {
|
||||
let ds = d.variables.datasets.iter_mut().find(|d| d.name == name).ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?;
|
||||
let (written, left) = edit_variables(s, "Update Data Set", |vars| {
|
||||
let ds = vars.datasets.iter_mut().find(|d| d.name == name).ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?;
|
||||
let mut written = BTreeSet::new();
|
||||
for (var, value) in &values {
|
||||
if all || ds.values.contains_key(var) {
|
||||
@@ -424,7 +484,7 @@ fn dataset_update(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
written.insert(var.as_str());
|
||||
}
|
||||
}
|
||||
let left: Vec<String> = d.variables.variables.iter().filter(|v| !written.contains(v.name.as_str())).map(|v| v.name.clone()).collect();
|
||||
let left: Vec<String> = vars.variables.iter().filter(|v| !written.contains(v.name.as_str())).map(|v| v.name.clone()).collect();
|
||||
Ok((written.len(), left))
|
||||
})?;
|
||||
Ok(json!({"name": name, "values": written, "left": left}))
|
||||
@@ -434,14 +494,14 @@ fn dataset_delete(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
// Without `names`, the active dataset: what the panel's Delete Data Set deletes.
|
||||
let active = s.doc()?.doc.variables.active_dataset.clone();
|
||||
let names = names_param(p, "dataset.delete", active, "no active data set")?;
|
||||
let n = s.edit("Delete Data Sets", |d, _| {
|
||||
let before = d.variables.datasets.len();
|
||||
d.variables.datasets.retain(|d| !names.contains(&d.name));
|
||||
let n = edit_variables(s, "Delete Data Sets", |v| {
|
||||
let before = v.datasets.len();
|
||||
v.datasets.retain(|d| !names.contains(&d.name));
|
||||
// The active dataset's own name is the one that goes, not a variable's.
|
||||
if d.variables.active_dataset.as_ref().is_some_and(|a| names.contains(a)) {
|
||||
d.variables.active_dataset = None;
|
||||
if v.active_dataset.as_ref().is_some_and(|a| names.contains(a)) {
|
||||
v.active_dataset = None;
|
||||
}
|
||||
Ok(before - d.variables.datasets.len())
|
||||
Ok(before - v.datasets.len())
|
||||
})?;
|
||||
Ok(json!({"deleted": n}))
|
||||
}
|
||||
@@ -464,7 +524,7 @@ fn dataset_list(s: &mut Session, _p: &Value) -> Result<Value> {
|
||||
fn apply_dataset(s: &mut Session, name: &str) -> Result<Value> {
|
||||
const C: &str = "dataset.select";
|
||||
let doc_path = s.doc()?.path.clone();
|
||||
let (applied, skipped, mut typed, bound) = s.edit("Apply Data Set", |d, sel| {
|
||||
let (applied, skipped, mut typed, bound) = edit_if_changed(s, "Apply Data Set", |d, sel| {
|
||||
let ds = d.variables.dataset(name).cloned().ok_or_else(|| bad(C, format!("no dataset named `{name}`")))?;
|
||||
// One object may be driven by several variables; walk them in definition order, so two
|
||||
// text variables on the same object land the later one's value and both are counted.
|
||||
@@ -714,7 +774,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
"Select Data Set",
|
||||
["Window", "Variables"],
|
||||
None,
|
||||
"{name} apply a dataset in one undo step, deselecting every selected object left hidden or locked (the key object too); a text value bound to a frame of threaded text replaces the whole story → {applied, skipped, overflow?: [id…] (area type and type on a path whose text it rewrote and that no longer fits, the story's last frame for threaded text; left out when all fits), warning? (when no object is bound to a variable that the dataset has a value for, so nothing was applied)}",
|
||||
"{name} apply a dataset in one undo step (none when it leaves the document as it was), deselecting every selected object left hidden or locked (the key object too); a text value bound to a frame of threaded text replaces the whole story → {applied, skipped, overflow?: [id…] (area type and type on a path whose text it rewrote and that no longer fits, the story's last frame for threaded text; left out when all fits), warning? (when no object is bound to a variable that the dataset has a value for, so nothing was applied)}",
|
||||
has_doc,
|
||||
dataset_select
|
||||
),
|
||||
|
||||
@@ -1936,3 +1936,133 @@ fn graph_variable_applies_csv_and_captures() {
|
||||
let r = rect(&mut s);
|
||||
assert!(s.execute("variable.bind", &json!({"variable": name, "ids": [r.0]})).is_err());
|
||||
}
|
||||
|
||||
/// A Variables command that leaves the document as it was (binding art to the variable it already
|
||||
/// has, unbinding art that has none, deleting a variable or data set that does not exist, writing a
|
||||
/// row's own values back, applying the active data set again) records no undo step: the step to
|
||||
/// redo stays, and the document stays unmodified.
|
||||
#[test]
|
||||
fn a_variables_command_that_changes_nothing_records_no_undo_step() {
|
||||
let dir = Folder::new("vars-noop");
|
||||
let red_path = dir.file("red.png");
|
||||
write(&red_path, &png(8, 8, RED));
|
||||
let mut s = session();
|
||||
let t = text(&mut s, "Alice");
|
||||
let r = rect(&mut s);
|
||||
let img = place(&mut s, &red_path);
|
||||
let other = rect(&mut s);
|
||||
s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap();
|
||||
s.execute("variable.define", &json!({"name": "Show", "kind": "visibility"})).unwrap();
|
||||
s.execute("variable.define", &json!({"name": "Photo", "kind": "image"})).unwrap();
|
||||
s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap();
|
||||
s.execute("variable.bind", &json!({"variable": "Show", "ids": [r.0]})).unwrap();
|
||||
s.execute("variable.bind", &json!({"variable": "Photo", "ids": [img.0]})).unwrap();
|
||||
s.execute("dataset.new", &json!({"name": "A", "values": {"Name": "Bob", "Show": false, "Photo": red_path}})).unwrap();
|
||||
s.execute("dataset.select", &json!({"name": "A"})).unwrap();
|
||||
let a = json!({"name": "A", "values": {"Name": "Bob", "Show": false, "Photo": red_path}});
|
||||
let calls = [
|
||||
("variable.bind", json!({"variable": "Show", "ids": [r.0]})),
|
||||
("variable.unbind", json!({"ids": [other.0]})),
|
||||
("variable.unbind", json!({"variable": "Name", "ids": [r.0]})),
|
||||
("variable.delete", json!({"names": ["Nope"]})),
|
||||
("dataset.delete", json!({"names": ["Nope"]})),
|
||||
("dataset.set", a),
|
||||
("dataset.update", json!({})),
|
||||
("dataset.update", json!({"all": true})),
|
||||
("dataset.select", json!({"name": "A"})),
|
||||
("dataset.next", json!({})),
|
||||
("dataset.prev", json!({})),
|
||||
];
|
||||
for (cmd, p) in calls {
|
||||
super::select_with_a_step_to_redo(&mut s, &[other]);
|
||||
let before = super::history(&s);
|
||||
assert_eq!((before.1, before.2), (1, false), "a step to redo, and nothing unsaved");
|
||||
s.execute(cmd, &p).unwrap();
|
||||
assert_eq!(super::history(&s), before, "{cmd} {p}");
|
||||
}
|
||||
assert_eq!(plain(&s, t), "Bob");
|
||||
// Applying the row again still deselects the art it leaves hidden, without an undo step.
|
||||
s.execute("select.set", &json!({"ids": [r.0, other.0]})).unwrap();
|
||||
assert!(s.doc().unwrap().selection.objects.contains(&r), "the hidden rectangle is selected");
|
||||
let before = super::history(&s);
|
||||
s.execute("dataset.select", &json!({"name": "A"})).unwrap();
|
||||
assert_eq!((super::history(&s), s.doc().unwrap().selection.objects.clone()), (before, vec![other]));
|
||||
// A command that changes something is still one undo step, and it clears Redo.
|
||||
super::select_with_a_step_to_redo(&mut s, &[other]);
|
||||
let undo = super::history(&s).0;
|
||||
s.execute("variable.bind", &json!({"variable": "Show", "ids": [other.0]})).unwrap();
|
||||
assert_eq!(super::history(&s), (undo + 1, 0, true));
|
||||
s.execute("dataset.new", &json!({"name": "B", "values": {"Name": "Cy"}})).unwrap();
|
||||
s.doc_mut().unwrap().mark_saved();
|
||||
let undo = super::history(&s).0;
|
||||
s.execute("dataset.next", &json!({})).unwrap();
|
||||
assert_eq!((super::history(&s), plain(&s, t)), ((undo + 1, 0, true), "Cy".to_string()));
|
||||
}
|
||||
|
||||
/// A Variables command that changes nothing, run while the Type tool is typing, still ends the
|
||||
/// typing session as one that changes something does: the typing is its own undo step, and the
|
||||
/// next keystroke starts a new one.
|
||||
#[test]
|
||||
fn a_variables_command_that_changes_nothing_ends_the_typing() {
|
||||
use vectorcraft_tools::{PointerEvent, PointerKind};
|
||||
let mut s = session();
|
||||
let v = ViewInfo::default();
|
||||
let t = text(&mut s, "Hello");
|
||||
let r = s.execute("shape.rectangle", &json!({"x": 200, "y": 200, "width": 50, "height": 40})).unwrap();
|
||||
let r = NodeId(r["id"].as_u64().unwrap());
|
||||
s.execute("variable.define", &json!({"name": "Show", "kind": "visibility"})).unwrap();
|
||||
s.execute("variable.bind", &json!({"variable": "Show", "ids": [r.0]})).unwrap();
|
||||
s.execute("dataset.new", &json!({"name": "A", "values": {"Show": true}})).unwrap();
|
||||
s.execute("dataset.select", &json!({"name": "A"})).unwrap();
|
||||
for (cmd, p) in [("variable.bind", json!({"variable": "Show", "ids": [r.0]})), ("dataset.select", json!({"name": "A"}))] {
|
||||
s.select_tool("type", v).unwrap();
|
||||
let b = s.doc().unwrap().doc.bounds_of(&[t], false).unwrap();
|
||||
for kind in [PointerKind::Down, PointerKind::Up] {
|
||||
s.pointer(&PointerEvent::new(kind, b.x1 - 1.0, b.center().y), v).unwrap();
|
||||
}
|
||||
s.tool_text("!", v).unwrap();
|
||||
assert!(crate::cmd::typing_in_progress(&s), "{cmd}");
|
||||
let steps = s.doc().unwrap().history.undo.len();
|
||||
s.execute(cmd, &p).unwrap();
|
||||
assert!(!crate::cmd::typing_in_progress(&s), "{cmd}");
|
||||
let labels: Vec<String> = s.doc().unwrap().history.undo.iter().map(|e| e.label.clone()).collect();
|
||||
assert_eq!((labels.len(), labels.last().map(String::as_str)), (steps + 1, Some("Typing")), "{cmd}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Applying the active data set again to threaded type that already holds the row's story records
|
||||
/// no undo step.
|
||||
#[test]
|
||||
fn applying_a_story_row_again_records_no_undo_step() {
|
||||
let (mut s, a, b, _) = threaded(200);
|
||||
s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap();
|
||||
s.execute("variable.bind", &json!({"variable": "Name", "ids": [a.0]})).unwrap();
|
||||
s.execute("dataset.capture", &json!({"name": "Orig"})).unwrap();
|
||||
s.execute("dataset.select", &json!({"name": "Orig"})).unwrap();
|
||||
assert!(!plain(&s, b).is_empty());
|
||||
super::select_with_a_step_to_redo(&mut s, &[a]);
|
||||
let before = super::history(&s);
|
||||
s.execute("dataset.select", &json!({"name": "Orig"})).unwrap();
|
||||
assert_eq!(super::history(&s), before);
|
||||
}
|
||||
|
||||
/// Applying the active data set again to a graph whose data already matches the row records no
|
||||
/// undo step, and a row with other data still rebuilds the graph in one.
|
||||
#[test]
|
||||
fn applying_a_graph_row_again_records_no_undo_step() {
|
||||
let mut s = session();
|
||||
let graph = json!({"type": "column", "x": 10, "y": 10, "width": 120, "height": 80, "csv": ",Sales\nA,1\nB,2"});
|
||||
let id = s.execute("graph.create", &graph).unwrap()["id"].as_u64().unwrap();
|
||||
s.execute("select.set", &json!({"ids": [id]})).unwrap();
|
||||
s.execute("variable.makeGraphDynamic", &json!({})).unwrap();
|
||||
let name = s.execute("variable.list", &json!({})).unwrap()["variables"][0]["name"].as_str().unwrap().to_string();
|
||||
s.execute("dataset.new", &json!({"name": "A", "values": {name.clone(): ",Sales\nA,10\nB,20"}})).unwrap();
|
||||
s.execute("dataset.new", &json!({"name": "B", "values": {name: ",Sales\nA,3\nB,4"}})).unwrap();
|
||||
s.execute("dataset.select", &json!({"name": "A"})).unwrap();
|
||||
super::select_with_a_step_to_redo(&mut s, &[NodeId(id)]);
|
||||
let before = super::history(&s);
|
||||
let v = s.execute("dataset.select", &json!({"name": "A"})).unwrap();
|
||||
assert_eq!((v["applied"].as_u64(), super::history(&s)), (Some(1), before));
|
||||
s.execute("dataset.select", &json!({"name": "B"})).unwrap();
|
||||
assert_eq!(super::history(&s), (before.0 + 1, 0, true));
|
||||
}
|
||||
|
||||
@@ -1743,6 +1743,11 @@ Brushes panel's New Brush button, its menu's New Brush… and art dropped on the
|
||||
Brush Options for the new brush; for a scatter, art or pattern brush they preview the art from `brush.selectionArt`.
|
||||
Their OK runs `brush.new {type, name, params, ids?}`, and Cancel adds no brush.
|
||||
|
||||
`variable.bind`, `variable.unbind`, `variable.delete`, `dataset.set`, `dataset.update`, `dataset.delete`,
|
||||
`dataset.select`, `dataset.next` and `dataset.prev` add no undo step when they leave the document as it was, such as
|
||||
binding art to a variable it already has or applying the active data set again: the steps to redo stay, and a saved
|
||||
document stays unmodified.
|
||||
|
||||
`dataset.select`, `dataset.next` and `dataset.prev` reply `{name, applied, skipped}`. They add `overflow: [id…]`
|
||||
when area type or type on a path whose text they rewrote doesn't fit, as `text.setText` does (see
|
||||
[Fitting area type](#fitting-area-type-auto-size-shrink-text-to-fit-overflow)). They skip an object that is
|
||||
|
||||
Reference in New Issue
Block a user