diff --git a/crates/calc/src/eval/call.rs b/crates/calc/src/eval/call.rs index 848762ab9..fe185e700 100644 --- a/crates/calc/src/eval/call.rs +++ b/crates/calc/src/eval/call.rs @@ -52,13 +52,16 @@ impl<'h> Evaluator<'h> { "ISFORMULA" | "FORMULATEXT" => { let Some(Ev::R(a)) = args.first().map(|a| self.eval(a)) else { return err(CellError::NA) }; let Some(a) = a.first() else { return err(CellError::NA) }; - let f = self.host.workbook().sheet(a.sheet).and_then(|s| s.cell(a.range.start)).and_then(|c| c.formula.clone()); + let wb = self.host.workbook(); + let f = wb.sheet(a.sheet).and_then(|s| s.cell(a.range.start)).and_then(|c| c.formula.clone()); + // A formula formatted Hidden on a protected sheet stays hidden from FORMULATEXT too. + let hidden = wb.sheet(a.sheet).is_some_and(|s| s.is_protected() && wb.styles.get(s.style_id(a.range.start)).protection.hidden); if name == "ISFORMULA" { Ev::V(Value::Bool(f.is_some())) } else { match f { - Some(f) => Ev::V(Value::text(format!("={}", f.text))), - None => err(CellError::NA), + Some(f) if !hidden => Ev::V(Value::text(format!("={}", f.text))), + _ => err(CellError::NA), } } } diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index da130fe2a..a59bf9b2a 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -366,6 +366,10 @@ pub struct Ctx<'a> { pub wb: Workbook, pub changed: Vec, pub structural: bool, + /// Recalculate every formula without the other effects of `structural` (table headers are not + /// resynchronized, array formulas are still checked): for edits that change only what a + /// function reads, like sheet protection. + pub recalc_all: bool, pub sel: &'a mut Selection, /// Rows whose automatic height should be recomputed (sheet, row). pub fit_rows: Vec<(usize, u32)>, @@ -400,13 +404,14 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) wb: (*d.wb).clone(), changed: Vec::new(), structural: false, + recalc_all: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false, edits: Vec::new(), }; let r = f(&mut ctx)?; - let Ctx { mut wb, mut changed, structural, mut fit_rows, protection_checked, edits, .. } = ctx; + let Ctx { mut wb, mut changed, structural, recalc_all, mut fit_rows, protection_checked, edits, .. } = ctx; // Equations whose source changed (or that arrived without a layout, as from a paste) are // laid out here, so painting only ever reads the cache. wb.refresh_equations(); @@ -435,7 +440,7 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) if let Some(t0) = t0 { eprintln!("commit: edit {:?}, {} changed", since(t0), changed.len()); } - if structural { + if structural || recalc_all { d.calc.recalc_all(&mut wb); } else if !changed.is_empty() { d.calc.cells_changed(&mut wb, &changed); diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index de9c98ecd..7eafa26db 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -163,6 +163,8 @@ fn protect_sheet(s: &mut Session, p: &Json) -> Result { let what = d.wb.sheet(sheet).map(|sh| sh.name.clone()).unwrap_or_default(); let prot = SheetProtection { password: str_param(p, "password").filter(|x| !x.is_empty()).map(|pw| new_password_hash(d, &what, pw)), ..prot }; edit(s, |cx| { + // FORMULATEXT reads a sheet's protection: its results are recalculated. + cx.recalc_all = true; cx.sheet_mut(sheet)?.protection = Some(prot.clone()); Ok(Json::Null) }) @@ -182,7 +184,10 @@ fn unprotect_sheet(s: &mut Session, p: &Json) -> Result { } } edit(s, |cx| { - cx.sheet_mut(sheet)?.protection = None; + // Only removing protection changes what FORMULATEXT reads; a no-op unprotect skips the full recalculation. + if cx.sheet_mut(sheet)?.protection.take().is_some() { + cx.recalc_all = true; + } Ok(Json::Null) }) } @@ -404,6 +409,21 @@ mod tests { assert_eq!(at(&s), (0, "A1".to_string())); } + #[test] + fn protecting_and_unprotecting_leave_table_headers_alone() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a", "b"], [1, 2]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B2", "header": true})).unwrap(); + // An imported header can hold a number: only a structural edit would turn it back into text. + let d = s.doc_mut().unwrap(); + std::sync::Arc::make_mut(&mut d.wb).sheet_mut(0).unwrap().set_value(gridcraft_core::CellRef::new(0, 0), gridcraft_core::Value::Number(7.0)); + s.execute("review.protectSheet", json!({})).unwrap(); + s.execute("review.unprotectSheet", json!({})).unwrap(); + let v = s.doc().unwrap().wb.sheet(0).unwrap().value(gridcraft_core::CellRef::new(0, 0)); + assert_eq!(v, gridcraft_core::Value::Number(7.0)); + } + #[test] fn protection_that_forbids_selecting_locked_cells_is_enforced_and_kept() { let mut s = book(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ed5e33945..55e795a01 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6480,6 +6480,23 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() { assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty()); } +#[test] +fn formulatext_does_not_reveal_a_hidden_formula() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "=1+1"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A1", "style": {"protection": {"locked": false, "hidden": true}}})).unwrap(); + s.execute("home.formatCells", json!({"range": "B1", "style": {"protection": {"locked": false}}})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": "=FORMULATEXT(A1)"})).unwrap(); + assert_eq!(v(&s, "B1"), Value::from("=1+1"), "not protected yet"); + s.execute("review.protectSheet", json!({})).unwrap(); + // Without entering the formula again: protecting recalculates what reads the protection. + assert_eq!(v(&s, "B1"), Value::Error(gridcraft_core::CellError::NA)); + s.execute("cell.set", json!({"cell": "B1", "input": "=FORMULATEXT(A1)"})).unwrap(); + assert_eq!(v(&s, "B1"), Value::Error(gridcraft_core::CellError::NA)); + s.execute("review.unprotectSheet", json!({})).unwrap(); + assert_eq!(v(&s, "B1"), Value::from("=1+1"), "unprotecting shows it again without re-entering"); +} + #[test] fn protected_sheet_refuses_layout_commands_its_permissions_do_not_allow() { let mut s = s(); diff --git a/crates/ui-egui/src/app/editing.rs b/crates/ui-egui/src/app/editing.rs index 8682a435c..a5ac8a176 100644 --- a/crates/ui-egui/src/app/editing.rs +++ b/crates/ui-egui/src/app/editing.rs @@ -50,6 +50,11 @@ impl SheetApp { self.session.mode = gridcraft_engine::Mode::Edit; return; } + // A hidden formula's source isn't loaded into an edit (a blank edit would also write over + // it on commit): no edit starts. Typing still replaces the cell. + if text.is_none() && gridcraft_engine::display::formula_hidden(&d.wb, sh, at) { + return; + } let locale = self.ui.language.formula_locale(); let current = if sh.cell_pictures.contains_key(&at) { String::new()