mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 22:19:10 -06:00
M6: calculation checks: formulas by field object, whole AFSimple lists only, no all-clear beside a broken formula
- CalculatedField carries the field object: form_calculation_order and the Calculation order page find each row's formula and value by it, so two calculated fields sharing a name (a malformed form) each show their own. - An AFSimple_Calculate list counts as written out only when it ends the call: "row" + suffix and [...].concat(more) are names built at run time, and an array item counts only when it ends at a comma or the list's end, so ["row" + i] no longer reads, or renames, "row". - After a reorder, Undo is offered on the missing-field card instead of bringing back the all-clear beside it. From CodeRabbit's review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BfMGHQNgdq5yf5tKZHGR3z
This commit is contained in:
Grzegorz Martens
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
feefeb8c95
commit
6a5020760e
@@ -667,11 +667,9 @@ impl Automation {
|
||||
CalculationIssue::NotInOrder => json!({ "issue": "not_in_order" }),
|
||||
CalculationIssue::BuiltNames => json!({ "issue": "built_names" }),
|
||||
};
|
||||
// By field object, not name: two calculated fields can share a name in a malformed form.
|
||||
let doc = self.doc(a)?;
|
||||
let mut by_name: std::collections::HashMap<&str, &FormField> = std::collections::HashMap::with_capacity(doc.form.len());
|
||||
for f in doc.form.iter() {
|
||||
by_name.entry(f.name.as_str()).or_insert(f);
|
||||
}
|
||||
let by_obj: std::collections::HashMap<_, &FormField> = doc.form.iter().map(|f| (f.obj, f)).collect();
|
||||
out["calculation_order"] = json!(order.fields.iter().map(|f| f.name.as_str()).collect::<Vec<_>>());
|
||||
out["fields"] = order
|
||||
.fields
|
||||
@@ -679,7 +677,7 @@ impl Automation {
|
||||
.map(|f| {
|
||||
json!({
|
||||
"name": f.name,
|
||||
"formula": by_name.get(f.name.as_str()).and_then(|x| calculate_json(x)),
|
||||
"formula": by_obj.get(&f.obj).and_then(|x| calculate_json(x)),
|
||||
"reads": f.reads,
|
||||
"listed": f.listed,
|
||||
"issues": f.issues.iter().map(issue).collect::<Vec<_>>(),
|
||||
|
||||
@@ -5058,6 +5058,43 @@ fn the_calculation_order_is_read_fixed_and_set() {
|
||||
assert_eq!((value(&mut a, "total"), value(&mut a, "tax")), (json!("0"), json!("0")), "undone with the values it recalculated");
|
||||
}
|
||||
|
||||
/// A malformed form with two calculated fields named `dup`, one summing `src`, the other `x`.
|
||||
fn two_calculations_with_one_name() -> Vec<u8> {
|
||||
let widget = |name: &str, y: u32, reads: &str| {
|
||||
format!(
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T ({name}) /Rect [50 {y} 250 {}] /P 3 0 R /AA << /C << /S /JavaScript /JS (AFSimple_Calculate\\(\"SUM\", \"{reads}\"\\);) >> >> >>",
|
||||
y + 20
|
||||
)
|
||||
};
|
||||
pdf_of(&[
|
||||
"<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>",
|
||||
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>",
|
||||
"<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R 7 0 R 8 0 R] >>",
|
||||
"<< /Fields [5 0 R 6 0 R 7 0 R 8 0 R] /DA (/Helv 0 Tf 0 g) /CO [6 0 R 7 0 R 5 0 R] >>",
|
||||
&widget("dup", 700, "x"),
|
||||
&widget("dup", 660, "src"),
|
||||
&widget("src", 620, "x"),
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (x) /Rect [50 580 250 600] /P 3 0 R /V (3) >>",
|
||||
])
|
||||
}
|
||||
|
||||
/// Each row of the calculation order gets its own field's formula, even where two fields share a
|
||||
/// name (a malformed form): the first dup sums src, the second x.
|
||||
#[test]
|
||||
fn the_calculation_order_gives_each_field_its_own_formula() {
|
||||
let dir = workdir("calcdup");
|
||||
std::fs::write(dir.join("dup.pdf"), two_calculations_with_one_name()).unwrap();
|
||||
let mut a = auto(&dir);
|
||||
let doc = ok(&mut a, "doc_open", json!({ "path": "dup.pdf" }))["doc"].as_u64().unwrap();
|
||||
let r = ok(&mut a, "form_calculation_order", json!({ "doc": doc }));
|
||||
assert_eq!(r["calculation_order"], json!(["dup", "src", "dup"]));
|
||||
let formulas: Vec<Value> = r["fields"].as_array().unwrap().iter().map(|f| f["formula"].clone()).collect();
|
||||
assert_eq!(
|
||||
formulas,
|
||||
[json!({ "op": "sum", "fields": ["src"] }), json!({ "op": "sum", "fields": ["x"] }), json!({ "op": "sum", "fields": ["x"] })]
|
||||
);
|
||||
}
|
||||
|
||||
/// The calculation order through the tool shows each formula, in form_set_props's shape or as a
|
||||
/// script, and flags a read of a field the form doesn't have and a script that builds the names
|
||||
/// it reads; form_fields shows a custom script too.
|
||||
|
||||
@@ -87,7 +87,8 @@ let cells: Vec<table::Cell> = grid.cells_for(names)?; // "Table1_1", "Table
|
||||
left alone. `calculations_changed` compares two readings of a form to say which formulas an
|
||||
edit rewrote.
|
||||
- The calculation order (`/CO`, §12.7.3): `calculation_order` lists the calculated fields in the
|
||||
order `recalculate` runs them, with the fields each formula reads (`Calculate::reads`,
|
||||
order `recalculate` runs them, each with its field object (two can share a name in a malformed
|
||||
form), with the fields each formula reads (`Calculate::reads`,
|
||||
`af::calculation_reads`; names built at run time aren't known) and what is wrong with the
|
||||
order: a field reading one that calculates later, a loop, a field `/CO` leaves out, and `/CO`
|
||||
entries that calculate nothing. Two issues are about the formula itself: it reads a field the
|
||||
|
||||
@@ -277,12 +277,18 @@ fn calculation_sites(js: &str) -> Vec<(Range<usize>, Site)> {
|
||||
sites.extend(built(k));
|
||||
continue;
|
||||
}
|
||||
// One string of comma-separated names, when it is the whole list (not `"row" + suffix`).
|
||||
if let Some((range, list)) = string(k + 4) {
|
||||
sites.push((range, Site::Commas(list)));
|
||||
if punct(k + 5, ')') {
|
||||
sites.push((range, Site::Commas(list)));
|
||||
} else {
|
||||
sites.extend(built(k));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
// `new Array("a", "b")` or `["a", "b"]`: its items, up to the first that isn't a string,
|
||||
// which has to close the list for the list to be written out. Nothing else is read, so no
|
||||
// `new Array("a", "b")` or `["a", "b"]`: each item that is one string, and the list has
|
||||
// to end the call. An item that is more than a string (`"row" + i`), or anything after
|
||||
// the list (`.concat(more)`), is a name built at run time. Nothing else is read, so no
|
||||
// token is read for two calls.
|
||||
let (first, close) = if punct(k + 4, '[') {
|
||||
(k + 5, ']')
|
||||
@@ -292,11 +298,21 @@ fn calculation_sites(js: &str) -> Vec<(Range<usize>, Site)> {
|
||||
sites.extend(built(k));
|
||||
continue;
|
||||
};
|
||||
for (range, token) in tokens.iter().skip(first) {
|
||||
match token {
|
||||
Js::Str(name) => sites.push((range.clone(), Site::Literal(name.clone()))),
|
||||
Js::Punct(',') => {}
|
||||
Js::Punct(c) if *c == close => break,
|
||||
let mut j = first;
|
||||
loop {
|
||||
match tokens.get(j) {
|
||||
// An item ends at a comma or the list's end, or where a script cut short ends.
|
||||
Some((range, Js::Str(name))) if punct(j + 1, ',') || punct(j + 1, close) || tokens.get(j + 1).is_none() => {
|
||||
sites.push((range.clone(), Site::Literal(name.clone())));
|
||||
j += 1;
|
||||
}
|
||||
Some((_, Js::Punct(','))) => j += 1,
|
||||
Some((_, Js::Punct(c))) if *c == close => {
|
||||
if !punct(j + 1, ')') {
|
||||
sites.extend(built(k));
|
||||
}
|
||||
break;
|
||||
}
|
||||
_ => {
|
||||
sites.extend(built(k));
|
||||
break;
|
||||
|
||||
@@ -168,9 +168,23 @@ fn names_built_at_run_time_are_noticed() {
|
||||
"AFSimple_Calculate(\"SUM\", [\"a\", prefix + \"b\"]);",
|
||||
"AFSimple_Calculate(\"SUM\", new Array(\"a\", \"b\" + c));",
|
||||
"AFSimple_Calculate(op, new Array(\"a\"));",
|
||||
"AFSimple_Calculate(\"SUM\", \"row\" + suffix);",
|
||||
"AFSimple_Calculate(\"SUM\", [\"a\"].concat(more));",
|
||||
"AFSimple_Calculate(\"SUM\", [\"row\" + i]);",
|
||||
] {
|
||||
assert!(reads_built_names(js), "{js}");
|
||||
}
|
||||
// A string that is only the start of a name or a list isn't read, or renamed, as one.
|
||||
for (js, reads) in [
|
||||
("AFSimple_Calculate(\"SUM\", \"row\" + suffix);", vec![]),
|
||||
("AFSimple_Calculate(\"SUM\", [\"row\" + i]);", vec![]),
|
||||
("AFSimple_Calculate(\"SUM\", [\"a\", \"row\" + i]);", vec!["a"]),
|
||||
("AFSimple_Calculate(\"SUM\", [\"a\", \"b\"].concat(more));", vec!["a", "b"]),
|
||||
] {
|
||||
assert_eq!(parse_calculate(js), None, "{js} is a script");
|
||||
assert_eq!(calculation_reads(js), reads, "{js}");
|
||||
assert_eq!(rename_in_calculation(js, "row", "line"), None, "{js}");
|
||||
}
|
||||
for js in [
|
||||
"event.value = getField(\"a\").value + getField('b').value;",
|
||||
"AFSimple_Calculate(\"SUM\", new Array(\"a\", \"b\"));",
|
||||
|
||||
@@ -17,6 +17,9 @@ const MAX_LINKS: usize = 1 << 20;
|
||||
pub struct CalculatedField {
|
||||
/// The field's full name.
|
||||
pub name: String,
|
||||
/// The field object. Two calculated fields can share a name in a malformed form; this tells
|
||||
/// them apart.
|
||||
pub obj: ObjRef,
|
||||
/// The fields its formula reads, each once, as the formula names them: a name also stands for
|
||||
/// the fields under it. A script's names built at run time (`getField("row" + i)`) aren't here.
|
||||
pub reads: Vec<String>,
|
||||
@@ -184,7 +187,7 @@ pub fn calculation_order(doc: &Document, fields: &[Field]) -> CalculationOrder {
|
||||
if f.actions.scripts.calculate.as_deref().is_some_and(crate::af::reads_built_names) {
|
||||
issues.push(CalculationIssue::BuiltNames);
|
||||
}
|
||||
out.push(CalculatedField { name: f.name.clone(), reads, listed: *listed, issues });
|
||||
out.push(CalculatedField { name: f.name.clone(), obj: f.obj, reads, listed: *listed, issues });
|
||||
}
|
||||
let fixed = fixed_order(&links, &component);
|
||||
CalculationOrder { fields: out, extra, fixed }
|
||||
|
||||
@@ -1814,6 +1814,12 @@ fn two_fields_with_one_name_keep_their_places_in_the_order() {
|
||||
};
|
||||
let row = |name: &str, reads: &str, ok: bool| (name.to_owned(), vec![reads.to_owned()], ok);
|
||||
assert_eq!(rows(&doc), [row("dup", "src", false), row("src", "x", true), row("dup", "x", true)]);
|
||||
// Each row is its own field object, so a caller can find the right formula for each dup.
|
||||
let all = fields(&doc);
|
||||
for c in calculation_order(&doc, &all).fields {
|
||||
let f = all.iter().find(|f| f.obj == c.obj).unwrap();
|
||||
assert_eq!((&f.name, f.actions.calculate.reads()), (&c.name, c.reads.clone()));
|
||||
}
|
||||
let current: Vec<String> = ["dup", "src", "dup"].map(String::from).into();
|
||||
set_calculation_order(&mut doc, ¤t).unwrap();
|
||||
assert_eq!(rows(&doc), [row("dup", "src", false), row("src", "x", true), row("dup", "x", true)], "the current order sets back unchanged");
|
||||
|
||||
@@ -139,16 +139,14 @@ fn one_line(text: &str) -> String {
|
||||
text.split_whitespace().collect::<Vec<_>>().join(" ").chars().take(200).collect()
|
||||
}
|
||||
|
||||
/// Each calculated field's value, formula and standing, in the order's order.
|
||||
/// Each calculated field's value, formula and standing, in the order's order. Fields are found by
|
||||
/// their object, not their name: two calculated fields can share a name in a malformed form.
|
||||
fn rows(calculated: &[CalculatedField], form: &[FormField]) -> Vec<Row> {
|
||||
let mut by_name: HashMap<&str, &FormField> = HashMap::with_capacity(form.len());
|
||||
for f in form {
|
||||
by_name.entry(f.name.as_str()).or_insert(f);
|
||||
}
|
||||
let by_obj: HashMap<_, &FormField> = form.iter().map(|f| (f.obj, f)).collect();
|
||||
calculated
|
||||
.iter()
|
||||
.map(|c| {
|
||||
let field = by_name.get(c.name.as_str());
|
||||
let field = by_obj.get(&c.obj);
|
||||
let value = match field {
|
||||
Some(f) if f.kind == FormFieldKind::Text => f.value.first().map(|v| format_value(&f.actions.format, v)).unwrap_or_default(),
|
||||
Some(f) => f.display_value(),
|
||||
@@ -451,6 +449,11 @@ fn cards(ui: &mut egui::Ui, t: &Tokens, pal: &Palette, order: &CalculationOrder,
|
||||
{
|
||||
act = Some(Act::EditFormula((*first).to_owned()));
|
||||
}
|
||||
// The all-clear, which carries Undo after a reorder, stays hidden while a formula is
|
||||
// broken, so Undo is offered here.
|
||||
if undo && text_button(ui, tl!("Undo"), t) {
|
||||
act = Some(Act::Undo);
|
||||
}
|
||||
});
|
||||
ui.add_space(6.0);
|
||||
}
|
||||
@@ -503,7 +506,7 @@ fn cards(ui: &mut egui::Ui, t: &Tokens, pal: &Palette, order: &CalculationOrder,
|
||||
ui.add(egui::Label::new(egui::RichText::new(tl!("Moves only what must move")).font(theme::regular(11.5)).color(t.text_muted)).wrap());
|
||||
});
|
||||
});
|
||||
} else if missing.is_empty() || undo {
|
||||
} else if missing.is_empty() {
|
||||
let (fill, edge, colour) = pal.good;
|
||||
card(ui, fill, edge, |ui| {
|
||||
let said = if loops.is_empty() {
|
||||
@@ -965,7 +968,8 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
fn field(name: &str, reads: &[&str], issues: Vec<CalculationIssue>) -> CalculatedField {
|
||||
CalculatedField { name: name.into(), reads: reads.iter().map(|r| (*r).to_owned()).collect(), listed: true, issues }
|
||||
let obj = pdfcraft_cos::ObjRef { num: 0, generation: 0 };
|
||||
CalculatedField { name: name.into(), obj, reads: reads.iter().map(|r| (*r).to_owned()).collect(), listed: true, issues }
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1354,6 +1354,56 @@ fn deleting_two_radio_buttons_of_a_group_at_once() {
|
||||
assert!(!names(&h).iter().any(|n| n == "shirt"));
|
||||
}
|
||||
|
||||
/// Two calculated fields that share a name (a malformed form) each show their own formula: the
|
||||
/// first dup sums src, the second x.
|
||||
#[test]
|
||||
fn fields_sharing_a_name_each_show_their_own_formula() {
|
||||
let widget = |name: &str, y: u32, reads: &str| {
|
||||
format!(
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T ({name}) /Rect [50 {y} 250 {}] /P 3 0 R /AA << /C << /S /JavaScript /JS (AFSimple_Calculate\\(\"SUM\", \"{reads}\"\\);) >> >> >>",
|
||||
y + 20
|
||||
)
|
||||
};
|
||||
let objs: Vec<String> = vec![
|
||||
"<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),
|
||||
"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),
|
||||
"<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R 7 0 R 8 0 R] >>".into(),
|
||||
"<< /Fields [5 0 R 6 0 R 7 0 R 8 0 R] /DA (/Helv 0 Tf 0 g) /CO [6 0 R 7 0 R 5 0 R] >>".into(),
|
||||
widget("dup", 700, "x"),
|
||||
widget("dup", 660, "src"),
|
||||
widget("src", 620, "x"),
|
||||
"<< /Type /Annot /Subtype /Widget /FT /Tx /T (x) /Rect [50 580 250 600] /P 3 0 R /V (3) >>".into(),
|
||||
];
|
||||
let mut bytes = b"%PDF-1.7\n".to_vec();
|
||||
let mut offsets = Vec::new();
|
||||
for (i, o) in objs.iter().enumerate() {
|
||||
offsets.push(bytes.len());
|
||||
bytes.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes());
|
||||
}
|
||||
let xref = bytes.len();
|
||||
bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes());
|
||||
for o in offsets {
|
||||
bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes());
|
||||
}
|
||||
bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
|
||||
let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_| {
|
||||
let mut app = PdfCraftApp::new();
|
||||
app.set_option("language", "en").unwrap();
|
||||
app.open_bytes("dup.pdf", None, bytes.clone()).unwrap();
|
||||
app
|
||||
});
|
||||
h.run_steps(4);
|
||||
h.state_mut().execute("form.prepare");
|
||||
h.run_steps(3);
|
||||
h.get_by_label("Calculation order").click();
|
||||
h.run_steps(3);
|
||||
for (row, formula) in [("1. dup", "= sum(src)"), ("3. dup", "= sum(x)")] {
|
||||
h.get_by_label_contains(row).hover();
|
||||
h.run_steps(30);
|
||||
assert!(h.query_by_label_contains(formula).is_some(), "{row} shows {formula}");
|
||||
}
|
||||
}
|
||||
|
||||
/// A formula that reads a field the form doesn't have gets a card of its own, in place of "every
|
||||
/// field calculates after the fields it reads", and its link opens that formula. A script that
|
||||
/// builds the names it reads gets a note.
|
||||
@@ -1383,6 +1433,17 @@ fn a_formula_reading_a_missing_field_has_its_own_card() {
|
||||
assert!(h.query_by_label("1 formula reads a field the form doesn't have").is_some());
|
||||
assert!(h.query_by_label("sub reads discount, which the form doesn't have.").is_some());
|
||||
assert!(h.query_by_label("Every field calculates after the fields it reads.").is_none(), "no all-clear while a formula is broken");
|
||||
// Reordered, the order can be undone from the missing-field card; the all-clear still stays away.
|
||||
h.state_mut().apply_edit(Edit::SetCalculationOrder { names: vec!["rows".into(), "sub".into()] });
|
||||
h.run_steps(3);
|
||||
assert!(h.query_by_label("Every field calculates after the fields it reads.").is_none(), "still no all-clear");
|
||||
h.get_by_label("Undo").click();
|
||||
h.run_steps(3);
|
||||
let order: Vec<String> = {
|
||||
let s = h.state();
|
||||
s.session.get(s.views[0].id).unwrap().calculation_order().fields.into_iter().map(|f| f.name).collect()
|
||||
};
|
||||
assert_eq!(order, ["sub", "rows"], "Undo put the order back");
|
||||
h.get_by_label("Edit sub's formula").click();
|
||||
h.run_steps(3);
|
||||
let draft = h.state().field_props.as_ref().map(|d| (d.field.clone(), d.tab));
|
||||
|
||||
Reference in New Issue
Block a user