Extract the SCC pop step from components() to reduce cognitive complexity (S3776)

Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>

crates/calc/src/recalc.rs

@@ -927,6 +927,18 @@ impl Host for PassHost<'_> {
     }
 }
 
+/// Pops one finished component off Tarjan's stack, labelling its nodes with `comp_index`. The
+/// component is everything up to and including `root`.
+fn pop_component(stack: &mut Vec<usize>, on_stack: &mut [bool], comp: &mut [usize], comp_index: usize, root: usize) {
+    while let Some(w) = stack.pop() {
+        on_stack[w] = false;
+        comp[w] = comp_index;
+        if w == root {
+            break;
+        }
+    }
+}
+
 /// Strongly connected components of a graph given as adjacency lists (Tarjan, iterative): each
 /// node's component index, and the number of nodes in each component.
 fn components(edges: &[Vec<usize>]) -> (Vec<usize>, Vec<usize>) {
@@ -965,13 +977,7 @@ fn components(edges: &[Vec<usize>]) -> (Vec<usize>, Vec<usize>) {
                 low[u] = low[u].min(low[v]);
             }
             if low[v] == num[v] {
-                while let Some(w) = stack.pop() {
+                pop_component(&mut stack, &mut on_stack, &mut comp, comps, v);
-                    on_stack[w] = false;
-                    comp[w] = comps;
-                    if w == v {
-                        break;
-                    }
-                }
                 comps += 1;
             }
         }
@@ -1025,6 +1031,7 @@ impl Calc {
         if circ.is_empty() {
             return Vec::new();
         }
+        let n = circ.len();
         let index: HashMap<Key, usize> = circ.iter().enumerate().map(|(i, k)| (*k, i)).collect();
         // Edges from a cell to the `#CIRC!` formulas that read it; loops are the same either way.
         let mut deps = Vec::new();

crates/engine/src/cmd/data.rs

@@ -1454,26 +1454,12 @@ fn show_levels(s: &mut Session, p: &Json) -> Result<Json> {
 
 /// Data ▸ Outline ▸ Settings: whether summary rows sit below their detail and summary columns
 /// right of it. A changed direction moves each collapsed group's mark (+) to its new summary line.
-fn outline_settings(s: &mut Session, p: &Json) -> Result<Json> {
+/// Moves the collapsed marks of a line axis to the summary line on its new side (`on` is the new
-    let si = target_sheet(s, p)?;
+/// setting, `was_before` the old one). A line that nested groups share keeps its mark on the outermost
-    edit(s, |cx| {
+/// group whose detail is all hidden, or the outermost when none is.
-        let sh = cx.sheet_mut(si)?;
+fn move_marks(map: &mut std::collections::BTreeMap<u32, LineInfo>, was_before: bool, on: bool) {
-        for (rows, key) in [(true, "summaryBelow"), (false, "summaryRight")] {
+    let mut marked: Vec<outline::OutlineGroup> =
-            let Some(on) = bool_param(p, key) else { continue };
+        outline::groups_with(map, was_before).into_iter().filter(|g| g.has_summary() && map.get(&g.summary()).is_some_and(|e| e.collapsed)).collect();
-            let was_before = outline::summary_before(sh, rows);
-            if rows {
-                sh.summary_below = on;
-            } else {
-                sh.summary_right = on;
-            }
-            if was_before != on {
-                continue;
-            }
-            let map = if rows { &mut sh.rows } else { &mut sh.cols };
-            let mut marked: Vec<outline::OutlineGroup> = outline::groups_with(map, was_before)
-                .into_iter()
-                .filter(|g| g.has_summary() && map.get(&g.summary()).is_some_and(|e| e.collapsed))
-                .collect();
     // A mark on a line that nested groups share moves with the outermost of them whose
     // detail is all hidden, or with the outermost when none is.
     let mut owner: std::collections::BTreeMap<u32, (bool, u8)> = std::collections::BTreeMap::new();
@@ -1495,6 +1481,24 @@ fn outline_settings(s: &mut Session, p: &Json) -> Result<Json> {
         }
     }
 }
+
+fn outline_settings(s: &mut Session, p: &Json) -> Result<Json> {
+    let si = target_sheet(s, p)?;
+    edit(s, |cx| {
+        let sh = cx.sheet_mut(si)?;
+        for (rows, key) in [(true, "summaryBelow"), (false, "summaryRight")] {
+            let Some(on) = bool_param(p, key) else { continue };
+            let was_before = outline::summary_before(sh, rows);
+            if rows {
+                sh.summary_below = on;
+            } else {
+                sh.summary_right = on;
+            }
+            if was_before != on {
+                continue;
+            }
+            move_marks(if rows { &mut sh.rows } else { &mut sh.cols }, was_before, on);
+        }
         Ok(json!({"summaryBelow": sh.summary_below, "summaryRight": sh.summary_right}))
     })
 }

crates/ui-egui/src/outline.rs

@@ -165,6 +165,35 @@ impl Dir<'_> {
 }
 
 /// The band's buttons and brackets for this frame.
+/// The bracket for one group's detail, and its tick, on the lane `across`; `button` is the group's
+/// summary button when it is on screen.
+fn group_bracket(out: &mut Layout, dir: &Dir, grp: &OutlineGroup, button: Option<f32>, across: f32, side: f32) {
+    let first = dir.first_visible(grp.start, grp.end);
+    let Some((first_line, (a0, _))) = first else { return };
+    if grp.summary_before {
+        // From the button down (or across) to the last detail line, with the tick there.
+        let a0 = button.map_or(a0, |mid| mid + side / 2.0);
+        let end = dir.start(grp.end) + dir.size(grp.end);
+        let a1 = end.min(dir.hi);
+        if a1 <= a0 {
+            return;
+        }
+        let tick = (end <= dir.hi).then(|| [dir.point(a1 - 0.5, across), dir.point(a1 - 0.5, across + side / 2.0)]);
+        out.brackets.push(Bracket { line: [dir.point(a0, across), dir.point(a1 - 0.5, across)], tick });
+        return;
+    }
+    let a1 = match button {
+        Some(mid) => mid - side / 2.0,
+        None => dir.start(grp.summary()).min(dir.hi),
+    };
+    if a1 <= a0 {
+        return;
+    }
+    let start_shown = first_line == grp.start && (a0 - dir.start(grp.start)).abs() < 0.5;
+    let tick = start_shown.then(|| [dir.point(a0 + 0.5, across), dir.point(a0 + 0.5, across + side / 2.0)]);
+    out.brackets.push(Bracket { line: [dir.point(a0 + 0.5, across), dir.point(a1, across)], tick });
+}
+
 pub fn layout(geo: &Geo, sh: &Sheet) -> Layout {
     let mut out = Layout::default();
     let zoom = (geo.z / DISPLAY_SCALE).max(0.1);
@@ -212,30 +241,7 @@ pub fn layout(geo: &Geo, sh: &Sheet) -> Layout {
                 continue;
             }
             // The bracket, over the part of the detail that is on screen.
-            let first = dir.first_visible(grp.start, grp.end);
+            group_bracket(&mut out, dir, grp, button, across, side);
-            let Some((first_line, (a0, _))) = first else { continue };
-            if grp.summary_before {
-                // From the button down (or across) to the last detail line, with the tick there.
-                let a0 = button.map_or(a0, |mid| mid + side / 2.0);
-                let end = dir.start(grp.end) + dir.size(grp.end);
-                let a1 = end.min(dir.hi);
-                if a1 <= a0 {
-                    continue;
-                }
-                let tick = (end <= dir.hi).then(|| [dir.point(a1 - 0.5, across), dir.point(a1 - 0.5, across + side / 2.0)]);
-                out.brackets.push(Bracket { line: [dir.point(a0, across), dir.point(a1 - 0.5, across)], tick });
-                continue;
-            }
-            let a1 = match button {
-                Some(mid) => mid - side / 2.0,
-                None => dir.start(grp.summary()).min(dir.hi),
-            };
-            if a1 <= a0 {
-                continue;
-            }
-            let start_shown = first_line == grp.start && (a0 - dir.start(grp.start)).abs() < 0.5;
-            let tick = start_shown.then(|| [dir.point(a0 + 0.5, across), dir.point(a0 + 0.5, across + side / 2.0)]);
-            out.brackets.push(Bracket { line: [dir.point(a0 + 0.5, across), dir.point(a1, across)], tick });
         }
     }
     out