From 8938fdfdcf10759e20172515e960b1d2e101d281 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 21:30:54 -0400 Subject: [PATCH] refactor: split complex functions for Sonar S3776 Extract helpers from backstage options (general, calculation and iteration sections), code_editor python (string scanning), xtask i18n literals (escape and string readers) and calc recalc loops_among (SCC search). No behaviour change. --- crates/calc/src/recalc.rs | 113 ++++++++++++++++-------------- crates/ui-egui/src/backstage.rs | 65 ++++++++++------- crates/ui-egui/src/code_editor.rs | 49 +++++++------ xtask/src/i18n_report.rs | 88 +++++++++++++---------- 4 files changed, 177 insertions(+), 138 deletions(-) diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index 4cbbcc8b4..72d7b217f 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -927,6 +927,64 @@ impl Host for PassHost<'_> { } } +/// 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]) -> (Vec, Vec) { + let n = edges.len(); + let (mut low, mut num) = (vec![0usize; n], vec![usize::MAX; n]); + let (mut on_stack, mut stack, mut comp) = (vec![false; n], Vec::new(), vec![usize::MAX; n]); + let (mut next, mut comps) = (0usize, 0usize); + for root in 0..n { + if num[root] != usize::MAX { + continue; + } + let mut call: Vec<(usize, usize)> = vec![(root, 0)]; + num[root] = next; + low[root] = next; + next += 1; + stack.push(root); + on_stack[root] = true; + while let Some(top) = call.last_mut() { + let v = top.0; + if let Some(&w) = edges.get(v).and_then(|e| e.get(top.1)) { + top.1 += 1; + if num[w] == usize::MAX { + num[w] = next; + low[w] = next; + next += 1; + stack.push(w); + on_stack[w] = true; + call.push((w, 0)); + } else if on_stack[w] { + low[v] = low[v].min(num[w]); + } + continue; + } + call.pop(); + if let Some(&(u, _)) = call.last() { + low[u] = low[u].min(low[v]); + } + if low[v] == num[v] { + while let Some(w) = stack.pop() { + on_stack[w] = false; + comp[w] = comps; + if w == v { + break; + } + } + comps += 1; + } + } + } + let mut size = vec![0usize; comps]; + for &c in &comp { + if let Some(s) = size.get_mut(c) { + *s += 1; + } + } + (comp, size) +} + impl Calc { pub fn new() -> Self { Self::default() @@ -981,59 +1039,8 @@ impl Calc { e }) .collect(); - // Tarjan's strongly connected components, iterative. - let n = circ.len(); - let (mut low, mut num) = (vec![0usize; n], vec![usize::MAX; n]); - let (mut on_stack, mut stack, mut comp) = (vec![false; n], Vec::new(), vec![usize::MAX; n]); - let (mut next, mut comps) = (0usize, 0usize); - for root in 0..n { - if num[root] != usize::MAX { - continue; - } - let mut call: Vec<(usize, usize)> = vec![(root, 0)]; - num[root] = next; - low[root] = next; - next += 1; - stack.push(root); - on_stack[root] = true; - while let Some(top) = call.last_mut() { - let v = top.0; - if let Some(&w) = edges.get(v).and_then(|e| e.get(top.1)) { - top.1 += 1; - if num[w] == usize::MAX { - num[w] = next; - low[w] = next; - next += 1; - stack.push(w); - on_stack[w] = true; - call.push((w, 0)); - } else if on_stack[w] { - low[v] = low[v].min(num[w]); - } - continue; - } - call.pop(); - if let Some(&(u, _)) = call.last() { - low[u] = low[u].min(low[v]); - } - if low[v] == num[v] { - while let Some(w) = stack.pop() { - on_stack[w] = false; - comp[w] = comps; - if w == v { - break; - } - } - comps += 1; - } - } - } - let mut size = vec![0usize; comps]; - for &c in &comp { - if let Some(s) = size.get_mut(c) { - *s += 1; - } - } + // Loop members: nodes in a component of more than one, or with an edge to themselves. + let (comp, size) = components(&edges); let on_loop = |i: usize| comp.get(i).and_then(|c| size.get(*c)).is_some_and(|s| *s > 1) || edges.get(i).is_some_and(|e| e.contains(&i)); // A formula with dynamic references (INDIRECT, OFFSET) may close a loop the graph can't see. let mut out: Vec = diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index ec097c201..f17d476ef 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -957,6 +957,35 @@ pub fn export_as(app: &mut SheetApp, fmt: &str) -> bool { fn options(app: &mut SheetApp, ui: &mut Ui) { let lang = app.ui.language; section(ui, lang, "Options|General"); + general_options(app, ui, lang); + ui.add_space(6.0); + let mut formula_bar = app.session.prefs.formula_bar; + if ui.checkbox(&mut formula_bar, lang.tr("Show formula bar")).changed() { + app.run_or_alert("view.formulaBar", json!({"on": formula_bar})); + } + ui.add_space(18.0); + section(ui, lang, "Options|Editing"); + let mut autocomplete = app.ui.autocomplete; + if ui.checkbox(&mut autocomplete, lang.tr("Enable AutoComplete for cell values")).changed() { + app.run_or_alert("options.autoComplete", json!({"on": autocomplete})); + } + ui.add_space(18.0); + section(ui, lang, "Options|Save"); + let mut autosave = app.ui.autosave; + if ui.checkbox(&mut autosave, lang.tr("Save .xlsx workbooks automatically after every change")).changed() { + app.toggle_autosave(); + } + let Some(calc) = app.session.active().map(|d| d.wb.calc.clone()) else { return }; + ui.add_space(18.0); + section(ui, lang, "Formulas"); + ui.label(lang.tr("Workbook calculation")); + calc_options(app, ui, lang, &calc); + ui.add_space(6.0); + dim(ui, lang.tr("Calculation settings are saved with the workbook.")); +} + +/// The display theme and interface language controls of the General section. +fn general_options(app: &mut SheetApp, ui: &mut Ui, lang: Language) { egui::Grid::new("backstage_options_general").num_columns(2).spacing(vec2(18.0, 10.0)).show(ui, |ui| { let label = ui.label(lang.tr("Display theme")); let mode = app.ui.theme_mode(); @@ -993,27 +1022,10 @@ fn options(app: &mut SheetApp, ui: &mut Ui) { .labelled_by(label.id); ui.end_row(); }); - ui.add_space(6.0); - let mut formula_bar = app.session.prefs.formula_bar; - if ui.checkbox(&mut formula_bar, lang.tr("Show formula bar")).changed() { - app.run_or_alert("view.formulaBar", json!({"on": formula_bar})); - } - ui.add_space(18.0); - section(ui, lang, "Options|Editing"); - let mut autocomplete = app.ui.autocomplete; - if ui.checkbox(&mut autocomplete, lang.tr("Enable AutoComplete for cell values")).changed() { - app.run_or_alert("options.autoComplete", json!({"on": autocomplete})); - } - ui.add_space(18.0); - section(ui, lang, "Options|Save"); - let mut autosave = app.ui.autosave; - if ui.checkbox(&mut autosave, lang.tr("Save .xlsx workbooks automatically after every change")).changed() { - app.toggle_autosave(); - } - let Some(calc) = app.session.active().map(|d| d.wb.calc.clone()) else { return }; - ui.add_space(18.0); - section(ui, lang, "Formulas"); - ui.label(lang.tr("Workbook calculation")); +} + +/// The calculation mode, and iterative calculation, of the active workbook. +fn calc_options(app: &mut SheetApp, ui: &mut Ui, lang: Language, calc: &gridcraft_engine::model::CalcSettings) { use gridcraft_engine::model::CalcMode; for (mode, id, label) in [ (CalcMode::Automatic, "automatic", "Automatic"), @@ -1030,6 +1042,13 @@ fn options(app: &mut SheetApp, ui: &mut Ui) { app.run_or_alert("formulas.calculationOptions", json!({"iterative": iterative})); } // Maximum Iterations and Maximum Change apply only while iteration is on, as in Excel. + iteration_options(app, ui, lang, calc, iterative); + // TODO: add "Set precision as displayed" here once `formulas.calculationOptions` takes + // `precisionAsDisplayed` (PR #1366); the model already stores it. +} + +/// Maximum Iterations and Maximum Change, live only while iterative calculation is on. +fn iteration_options(app: &mut SheetApp, ui: &mut Ui, lang: Language, calc: &gridcraft_engine::model::CalcSettings, iterative: bool) { ui.add_enabled_ui(iterative, |ui| { ui.horizontal(|ui| { ui.add_space(24.0); @@ -1070,10 +1089,6 @@ fn options(app: &mut SheetApp, ui: &mut Ui) { }); }); }); - // TODO: add "Set precision as displayed" here once `formulas.calculationOptions` takes - // `precisionAsDisplayed` (PR #1366); the model already stores it. - ui.add_space(6.0); - dim(ui, lang.tr("Calculation settings are saved with the workbook.")); } /// Refuses a typed setting with a message, leaving the workbook as it was. The Enter that diff --git a/crates/ui-egui/src/code_editor.rs b/crates/ui-egui/src/code_editor.rs index 6f02b4044..e462e46ef 100644 --- a/crates/ui-egui/src/code_editor.rs +++ b/crates/ui-egui/src/code_editor.rs @@ -78,6 +78,32 @@ pub fn highlight(text: &str, lang: Lang) -> Vec<(Range, Token)> { } } +/// End (exclusive, in chars) of the Python string literal opening with quote `c` at char `i`. +fn python_string_end(chars: &[(usize, char)], i: usize, c: char) -> usize { + let triple = chars.get(i + 1).is_some_and(|(_, d)| *d == c) && chars.get(i + 2).is_some_and(|(_, d)| *d == c); + let mut j = if triple { i + 3 } else { i + 1 }; + loop { + match chars.get(j) { + None => break, + Some((_, '\\')) => j += 2, + Some((_, '\n')) if !triple => break, + Some((_, d)) if *d == c => { + if !triple { + j += 1; + break; + } + if chars.get(j + 1).is_some_and(|(_, d)| *d == c) && chars.get(j + 2).is_some_and(|(_, d)| *d == c) { + j += 3; + break; + } + j += 1; + } + Some(_) => j += 1, + } + } + j.min(chars.len()) +} + fn python(text: &str) -> Vec<(Range, Token)> { let chars: Vec<(usize, char)> = text.char_indices().collect(); let end_of = |i: usize| chars.get(i).map(|(b, _)| *b).unwrap_or(text.len()); @@ -92,28 +118,7 @@ fn python(text: &str) -> Vec<(Range, Token)> { out.push((start..end_of(j), Token::Comment)); i = j; } else if c == '"' || c == '\'' { - let triple = chars.get(i + 1).is_some_and(|(_, d)| *d == c) && chars.get(i + 2).is_some_and(|(_, d)| *d == c); - let mut j = if triple { i + 3 } else { i + 1 }; - loop { - match chars.get(j) { - None => break, - Some((_, '\\')) => j += 2, - Some((_, '\n')) if !triple => break, - Some((_, d)) if *d == c => { - if !triple { - j += 1; - break; - } - if chars.get(j + 1).is_some_and(|(_, d)| *d == c) && chars.get(j + 2).is_some_and(|(_, d)| *d == c) { - j += 3; - break; - } - j += 1; - } - Some(_) => j += 1, - } - } - let j = j.min(chars.len()); + let j = python_string_end(&chars, i, c); out.push((start..end_of(j), Token::String)); i = j; } else if c.is_ascii_digit() || (c == '.' && chars.get(i + 1).is_some_and(|(_, d)| d.is_ascii_digit())) { diff --git a/xtask/src/i18n_report.rs b/xtask/src/i18n_report.rs index 344ed7569..99cbaa9ee 100644 --- a/xtask/src/i18n_report.rs +++ b/xtask/src/i18n_report.rs @@ -116,6 +116,55 @@ pub fn catalog_keys(source: &str) -> BTreeSet { keys } +/// The character a `\u{...}` escape stands for, if it names a valid code point. +fn unicode_escape(chars: &mut std::iter::Peekable>) -> Option { + let mut hex = String::new(); + for (_, c) in chars.by_ref() { + match c { + '}' => break, + '{' => {} + c => hex.push(c), + } + } + u32::from_str_radix(&hex, 16).ok().and_then(char::from_u32) +} + +/// The value of a string literal whose opening quote was just read, and the byte offset past +/// its closing quote (`len`, the source's length, when it is never closed). +fn string_literal(chars: &mut std::iter::Peekable>, len: usize) -> (String, usize) { + let mut value = String::new(); + let mut end = len; + while let Some((j, c)) = chars.next() { + match c { + '"' => { + end = j + 1; + break; + } + '\\' => match chars.next().map(|(_, c)| c) { + Some('n') => value.push('\n'), + Some('t') => value.push('\t'), + Some('r') => value.push('\r'), + Some('0') => value.push('\0'), + Some(c @ ('\\' | '"' | '\'')) => value.push(c), + Some('u') => { + if let Some(c) = unicode_escape(chars) { + value.push(c); + } + } + // A line continuation skips the newline and the next line's indentation. + Some('\n') => { + while chars.peek().is_some_and(|(_, c)| c.is_whitespace()) { + chars.next(); + } + } + _ => {} + }, + c => value.push(c), + } + } + (value, end) +} + /// Every string literal outside `//` comments: its decoded value, and the byte range from its /// opening quote to just past its closing one. fn literals(source: &str) -> Vec<(String, usize, usize)> { @@ -133,44 +182,7 @@ fn literals(source: &str) -> Vec<(String, usize, usize)> { if c != '"' { continue; } - let mut value = String::new(); - let mut end = source.len(); - while let Some((j, c)) = chars.next() { - match c { - '"' => { - end = j + 1; - break; - } - '\\' => match chars.next().map(|(_, c)| c) { - Some('n') => value.push('\n'), - Some('t') => value.push('\t'), - Some('r') => value.push('\r'), - Some('0') => value.push('\0'), - Some(c @ ('\\' | '"' | '\'')) => value.push(c), - Some('u') => { - let mut hex = String::new(); - for (_, c) in chars.by_ref() { - match c { - '}' => break, - '{' => {} - c => hex.push(c), - } - } - if let Some(c) = u32::from_str_radix(&hex, 16).ok().and_then(char::from_u32) { - value.push(c); - } - } - // A line continuation skips the newline and the next line's indentation. - Some('\n') => { - while chars.peek().is_some_and(|(_, c)| c.is_whitespace()) { - chars.next(); - } - } - _ => {} - }, - c => value.push(c), - } - } + let (value, end) = string_literal(&mut chars, source.len()); out.push((value, i, end)); } out