FORMULATEXT and the cell editor keep a hidden formula's source hidden

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),
                     }
                 }
             }

crates/engine/src/cmd/print/tests.rs

@@ -695,79 +695,6 @@ fn bands_unit() {
     assert!(w.len() > 2);
 }
 
-/// The wrapper `wrap_lines` replaced, re-measuring strings, as the reference for its lines.
-fn wrap_lines_remeasuring(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
-    let mut out = Vec::new();
-    for para in text.split('\n') {
-        let mut line = String::new();
-        for word in para.split(' ') {
-            let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
-            if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
-                line = cand;
-            } else {
-                out.push(std::mem::take(&mut line));
-                line = word.to_string();
-            }
-            while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
-                let chars: Vec<char> = line.chars().collect();
-                let (mut lo, mut hi) = (1usize, chars.len());
-                while lo < hi {
-                    let mid = lo + (hi - lo).div_ceil(2);
-                    let cut: String = chars[..mid].iter().collect();
-                    if gridcraft_pdf::text_width(font, size, &cut) <= width {
-                        lo = mid;
-                    } else {
-                        hi = mid - 1;
-                    }
-                }
-                if lo == chars.len() {
-                    break;
-                }
-                out.push(chars[..lo].iter().collect());
-                line = chars[lo..].iter().collect();
-            }
-            if out.len() > 2000 {
-                return out;
-            }
-        }
-        out.push(line);
-    }
-    out
-}
-
-#[test]
-fn wrap_lines_matches_the_remeasuring_wrapper() {
-    let texts = [
-        "the quick brown fox jumps",
-        "Supercalifragilistic",
-        "a  b   c\n\nverylongwordthatneedsbreaking short\nend ",
-        "Wide WWWWWWWWWW words, iiiiiiii narrow ones; mixed MMMMiiiiMMMM",
-        "€ ü ß — unicode Ünïcödé text",
-        "",
-        " ",
-    ];
-    for text in texts {
-        for font in [Font::Helvetica, Font::HelveticaBold, Font::Courier] {
-            for (size, width) in [(10.0, 50.0), (10.0, 20.0), (11.0, 1.0), (8.0, 300.0), (12.0, 0.0)] {
-                assert_eq!(wrap_lines(text, font, size, width), wrap_lines_remeasuring(text, font, size, width), "{text:?} {font:?} {size} {width}");
-            }
-        }
-    }
-}
-
-#[test]
-fn wrapping_a_long_word_in_a_narrow_box_stops_at_the_line_limit() {
-    // One character per line: the old wrapper re-measured and copied the rest of the word for
-    // each of the 100,000 lines before checking the limit.
-    let word = "w".repeat(100_000);
-    let lines = wrap_lines(&word, Font::Helvetica, 10.0, 1.0);
-    assert!(lines.len() <= 2002, "{}", lines.len());
-    assert!(lines.iter().all(|l| l == "w"));
-    // A long paragraph in a very wide box is one line.
-    let para = vec!["word"; 50_000].join(" ");
-    assert_eq!(wrap_lines(&para, Font::Helvetica, 10.0, 1.0e9), vec![para.clone()]);
-}
-
 #[test]
 fn center_across_spans_following_empty_center_across_cells() {
     let mut styles = StyleTable::default();

crates/engine/src/cmd/print/text.rs

@@ -3,57 +3,42 @@ use super::{CellRef, Color, Font, HAlign, MAX_COLS, Page, Point, RangeRef, Sheet
 
 const PAD_PX: f32 = 2.0;
 
-/// Most lines [`wrap_lines`] returns (it may return one more, the rest of a paragraph).
-const MAX_WRAP_LINES: usize = 2000;
-
 /// Word-wraps `text` to `width` points.
-///
-/// Widths are kept as running sums of glyph widths (the sum `text_width` makes), so each
-/// character is measured once: re-measuring the line for every word, or the rest of an
-/// over-long word for every piece cut from it, made a long paragraph or word quadratic.
 pub(super) fn wrap_lines(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
-    let units_of = |s: &str| -> u64 { s.chars().map(|c| u64::from(font.char_width(c))).sum() };
-    let fits = |units: u64| units as f32 * size / 1000.0 <= width;
-    let space = units_of(" ");
     let mut out = Vec::new();
     for para in text.split('\n') {
         let mut line = String::new();
-        let mut line_units: u64 = 0;
         for word in para.split(' ') {
-            let word_units = units_of(word);
-            if line.is_empty() {
-                line.push_str(word);
-                line_units = word_units;
-            } else if fits(line_units + space + word_units) {
-                line.push(' ');
-                line.push_str(word);
-                line_units += space + word_units;
+            let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
+            if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
+                line = cand;
             } else {
                 out.push(std::mem::take(&mut line));
-                line.push_str(word);
-                line_units = word_units;
+                line = word.to_string();
             }
-            // Break a single over-long word by characters: each piece is the longest that fits
-            // (at least one character).
-            if !fits(line_units) && line.chars().nth(1).is_some() {
-                let mut piece = String::new();
-                let mut units: u64 = 0;
-                for c in line.chars() {
-                    let w = u64::from(font.char_width(c));
-                    if !piece.is_empty() && !fits(units + w) {
-                        out.push(std::mem::take(&mut piece));
-                        units = 0;
-                        if out.len() > MAX_WRAP_LINES {
-                            return out;
-                        }
+            // Break a single over-long word by characters. The cut that fits is found by binary
+            // search (each try measures one candidate prefix), so an over-long word costs
+            // O(n log n) measurements instead of re-measuring the whole cut per character for
+            // every emitted line (`a` × 100k pasted text would be cubic worst case).
+            while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
+                let chars: Vec<char> = line.chars().collect();
+                let (mut lo, mut hi) = (1usize, chars.len());
+                while lo < hi {
+                    let mid = lo + (hi - lo).div_ceil(2);
+                    let cut: String = chars[..mid].iter().collect();
+                    if gridcraft_pdf::text_width(font, size, &cut) <= width {
+                        lo = mid;
+                    } else {
+                        hi = mid - 1;
                     }
-                    piece.push(c);
-                    units += w;
                 }
-                line = piece;
-                line_units = units;
+                if lo == chars.len() {
+                    break;
+                }
+                out.push(chars[..lo].iter().collect());
+                line = chars[lo..].iter().collect();
             }
-            if out.len() > MAX_WRAP_LINES {
+            if out.len() > 2000 {
                 return out;
             }
         }

crates/engine/src/tests.rs

@@ -5572,3 +5572,16 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
     let sh = s.doc().unwrap().wb.active().unwrap().clone();
     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();
+    s.execute("cell.set", json!({"cell": "B1", "input": "=FORMULATEXT(A1)"})).unwrap();
+    assert_eq!(v(&s, "B1"), Value::Error(gridcraft_core::CellError::NA));
+}

crates/functions/src/stat/descriptive.rs

@@ -50,27 +50,12 @@ pub(super) fn median(v: Vec<f64>) -> R<f64> {
 
 /// Values occurring at least twice, ordered by first occurrence, with their (equal, maximal) count.
 fn modes(v: &[f64]) -> Vec<f64> {
-    // Index of each distinct number in `counts`, so counting is linear rather than a search of
-    // every distinct number so far. Keyed by bits with -0 folded into 0 (they compare equal); NaN
-    // never equals anything, so it is never merged.
-    let mut index: std::collections::HashMap<u64, usize> = std::collections::HashMap::new();
     let mut counts: Vec<(f64, usize)> = Vec::new();
     for &x in v {
-        if x.is_nan() {
+        if let Some(e) = counts.iter_mut().find(|e| e.0 == x) {
+            e.1 += 1;
+        } else {
             counts.push((x, 1));
-            continue;
-        }
-        let key = if x == 0.0 { 0 } else { x.to_bits() };
-        match index.entry(key) {
-            std::collections::hash_map::Entry::Occupied(e) => {
-                if let Some(c) = counts.get_mut(*e.get()) {
-                    c.1 += 1;
-                }
-            }
-            std::collections::hash_map::Entry::Vacant(e) => {
-                e.insert(counts.len());
-                counts.push((x, 1));
-            }
         }
     }
     let best = counts.iter().map(|e| e.1).max().unwrap_or(0);
@@ -466,15 +451,11 @@ pub(super) fn frequency(a: &[Arg], _: C) -> R<Value> {
     if bins.is_empty() {
         return col_value(vec![data.len() as f64]);
     }
-    // a NaN bin equals no value (and would break the ascending order searched below)
-    let mut order: Vec<usize> = (0..bins.len()).filter(|&i| bins.get(i).is_some_and(|b| !b.is_nan())).collect();
+    let mut order: Vec<usize> = (0..bins.len()).collect();
     order.sort_by(|&i, &j| bins.get(i).copied().unwrap_or(0.0).total_cmp(&bins.get(j).copied().unwrap_or(0.0)));
     let mut counts = vec![0.0; bins.len() + 1];
     for x in data {
-        // `order` sorts the bins ascending, so the bins `x` exceeds come first: binary search for
-        // the first bin at or above `x` (the first of equal bins, as a linear scan would find).
-        let pos = if x.is_nan() { order.len() } else { order.partition_point(|&i| bins.get(i).copied().unwrap_or(f64::INFINITY) < x) };
-        let slot = order.get(pos).copied().unwrap_or(bins.len());
+        let slot = order.iter().find(|&&i| x <= bins.get(i).copied().unwrap_or(f64::INFINITY)).copied().unwrap_or(bins.len());
         if let Some(c) = counts.get_mut(slot) {
             *c += 1.0;
         }

crates/functions/src/stat/tests.rs

@@ -617,23 +617,3 @@ fn aggregate_helper() {
     close(aggregate_values_k(9, &vals, &nv(2.0)), 9.0);
     is_err(aggregate_values_k(14, &vals, &nv(10.0)), CellError::Num);
 }
-
-/// Security: FREQUENCY finds each value's bin by binary search, and MODE counts distinct numbers
-/// through an index, so neither is quadratic in its inputs. Results are as before: unsorted and
-/// duplicate bins keep their positions (a value goes to the first of equal bins), and tied modes
-/// come in first-occurrence order.
-#[test]
-fn frequency_and_mode_scale_with_their_inputs() {
-    let f = ev("FREQUENCY", vec![c(&[1.0, 2.0, 2.0, 3.0, 5.0, 9.0, -1.0]), c(&[3.0, 2.0, 2.0, 0.0])]);
-    assert_eq!(rows_of(&f), vec![vec![nv(1.0)], vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)], vec![nv(2.0)]]);
-    // NaN data goes to the overflow bin, and a NaN bin takes nothing
-    let f = ev("FREQUENCY", vec![c(&[f64::NAN, 0.5, 2.0]), c(&[f64::NAN, 1.0])]);
-    assert_eq!(rows_of(&f), vec![vec![nv(0.0)], vec![nv(1.0)], vec![nv(2.0)]]);
-    let data: Vec<f64> = (0..200_000).map(|i| f64::from(i) + 1e6).collect();
-    let bins: Vec<f64> = (0..200_000).map(f64::from).collect();
-    let f = ev("FREQUENCY", vec![c(&data), c(&bins)]);
-    assert_eq!(rows_of(&f).last(), Some(&vec![nv(200_000.0)]));
-    let distinct = ev("MODE", vec![c(&data)]);
-    is_err(distinct, CellError::NA);
-    assert_eq!(rows_of(&ev("MODE.MULT", vec![c(&[3.0, 0.0, 1.0, -0.0, 3.0, 1.0])])), vec![vec![nv(3.0)], vec![nv(0.0)], vec![nv(1.0)]]);
-}

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()