From 52cf825395b8a64c1d93affac2ce7f9a8202b504 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 21:37:09 -0400 Subject: [PATCH] refactor: split complex functions for Sonar S3776 (#2609) Extract helpers from auto_sum (selection and single-cell guess), update_cursor (drag, hover, freeze, and header-edge cursors) and read_styles (dxf reader). No behaviour change. --- crates/engine/src/cmd/formulas.rs | 162 +++++++++++++++++------------- crates/ui-egui/src/grid.rs | 84 ++++++++++------ crates/xlsx/src/styles.rs | 96 +++++++++--------- 3 files changed, 191 insertions(+), 151 deletions(-) diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index beb80f497..dc523d333 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -96,75 +96,73 @@ pub fn specs() -> Vec { ] } -fn auto_sum(s: &mut Session, p: &Json) -> Result { - let func = str_param(p, "function").unwrap_or("SUM").to_ascii_uppercase(); - let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; - let sel = match str_param(p, "range") { - Some(_) => target_range(s, p)?, - None => d.selection.current(), - }; - if !sel.is_single() { - // A selection: totals go in its empty last row or column, as Excel's do, and otherwise - // past it: to the right of a single row, below each column of anything taller. Columns - // past the sheet's data get no total (a whole-row selection doesn't write 16,384 - // formulas), nor do rows past it, and all totals are one edit (one undo step). - let Some(used) = sh.cells.used_range() else { return ok() }; - let empty = |row: u32, col: u32| sh.value(CellRef::new(row, col)).is_empty(); - let sum = |from: CellRef, to: CellRef| format!("={func}({})", RangeRef::new(from, to).a1()); - let last_col = used.end.col.min(sel.end.col); - let mut writes: Vec<(CellRef, String)> = Vec::new(); - if sel.start.row == sel.end.row { - // One row: its total goes in its last cell when that is empty, else just after it. - let row = sel.start.row; - let end_col = if sel.is_full_rows() { last_col } else { sel.end.col }; - let (data_end, out) = if end_col > sel.start.col && empty(row, end_col) { (end_col - 1, end_col) } else { (end_col, end_col + 1) }; - if out >= gridcraft_core::MAX_COLS { - return Err(EngineError::Other("There's no room to the right of the selection for the total.".into())); - } - writes.push((CellRef::new(row, out), sum(CellRef::new(row, sel.start.col), CellRef::new(row, data_end)))); - } else { - // An empty last column takes each row's total; rows past the data get none. - let rows_end = sel.end.row.min(used.end.row); - let right_blank = !sel.is_full_rows() && sel.end.col > sel.start.col && (sel.start.row..=rows_end).all(|r| empty(r, sel.end.col)); - let data_end_col = if right_blank { sel.end.col - 1 } else { last_col }; - let last_row_blank = (sel.start.col..=last_col).all(|c| empty(sel.end.row, c)); - let (data_end_row, out_row) = if last_row_blank { (sel.end.row.saturating_sub(1), sel.end.row) } else { (sel.end.row, sel.end.row + 1) }; - // Each column's total, below it: unless only the last column is there to take totals. - if last_row_blank || !right_blank { - if out_row >= gridcraft_core::MAX_ROWS { - return Err(EngineError::Other("There's no room below the selection for the totals.".into())); - } - for c in sel.start.col..=data_end_col.min(last_col) { - writes.push((CellRef::new(out_row, c), sum(CellRef::new(sel.start.row, c), CellRef::new(data_end_row, c)))); - } - } - if right_blank { - for r in sel.start.row..=data_end_row.min(rows_end) { - writes.push((CellRef::new(r, sel.end.col), sum(CellRef::new(r, sel.start.col), CellRef::new(r, data_end_col)))); - } - // Both empty: the corner takes the total of the whole block. - if last_row_blank { - writes.push(( - CellRef::new(sel.end.row, sel.end.col), - sum(CellRef::new(sel.start.row, sel.start.col), CellRef::new(data_end_row, data_end_col)), - )); - } - } - } - let sheet = d.wb.active_sheet; - edit(s, |cx| { - for (c, f) in &writes { - let cell = super::edit::input_to_cell(f, sheet, *c, &mut cx.wb)?.unwrap_or_default(); - cx.sheet_mut(sheet)?.set_cell(*c, cell); - cx.touch(sheet, *c); - } - Ok(()) - })?; - return ok(); +/// Whether a cell on the active sheet is empty. +fn blank(sh: &gridcraft_model::sheet::Sheet, row: u32, col: u32) -> bool { + sh.value(CellRef::new(row, col)).is_empty() +} + +fn sum_formula(func: &str, from: CellRef, to: CellRef) -> String { + format!("={func}({})", RangeRef::new(from, to).a1()) +} + +/// Totals for a multi-cell selection as `(cell, formula)` writes. `None` when the sheet has no data. +fn selection_totals(sh: &gridcraft_model::sheet::Sheet, sel: RangeRef, func: &str) -> Result>> { + // A selection: totals go in its empty last row or column, as Excel's do, and otherwise + // past it: to the right of a single row, below each column of anything taller. Columns + // past the sheet's data get no total (a whole-row selection doesn't write 16,384 + // formulas), nor do rows past it, and all totals are one edit (one undo step). + let Some(used) = sh.cells.used_range() else { return Ok(None) }; + if sel.start.row == sel.end.row { single_row_totals(sh, sel, used, func).map(Some) } else { block_totals(sh, sel, used, func).map(Some) } +} + +/// One row: its total goes in its last cell when that is empty, else just after it. +fn single_row_totals(sh: &gridcraft_model::sheet::Sheet, sel: RangeRef, used: RangeRef, func: &str) -> Result> { + let row = sel.start.row; + let last_col = used.end.col.min(sel.end.col); + let end_col = if sel.is_full_rows() { last_col } else { sel.end.col }; + let (data_end, out) = if end_col > sel.start.col && blank(sh, row, end_col) { (end_col - 1, end_col) } else { (end_col, end_col + 1) }; + if out >= gridcraft_core::MAX_COLS { + return Err(EngineError::Other("There's no room to the right of the selection for the total.".into())); } - // Single cell: guess the range above, else to the left. - let at = sel.start; + Ok(vec![(CellRef::new(row, out), sum_formula(func, CellRef::new(row, sel.start.col), CellRef::new(row, data_end)))]) +} + +/// A block of rows: each column's total below it, and each row's total in an empty last column. +fn block_totals(sh: &gridcraft_model::sheet::Sheet, sel: RangeRef, used: RangeRef, func: &str) -> Result> { + let mut writes: Vec<(CellRef, String)> = Vec::new(); + let last_col = used.end.col.min(sel.end.col); + // An empty last column takes each row's total; rows past the data get none. + let rows_end = sel.end.row.min(used.end.row); + let right_blank = !sel.is_full_rows() && sel.end.col > sel.start.col && (sel.start.row..=rows_end).all(|r| blank(sh, r, sel.end.col)); + let data_end_col = if right_blank { sel.end.col - 1 } else { last_col }; + let last_row_blank = (sel.start.col..=last_col).all(|c| blank(sh, sel.end.row, c)); + let (data_end_row, out_row) = if last_row_blank { (sel.end.row.saturating_sub(1), sel.end.row) } else { (sel.end.row, sel.end.row + 1) }; + // Each column's total, below it: unless only the last column is there to take totals. + if last_row_blank || !right_blank { + if out_row >= gridcraft_core::MAX_ROWS { + return Err(EngineError::Other("There's no room below the selection for the totals.".into())); + } + for c in sel.start.col..=data_end_col.min(last_col) { + writes.push((CellRef::new(out_row, c), sum_formula(func, CellRef::new(sel.start.row, c), CellRef::new(data_end_row, c)))); + } + } + if right_blank { + for r in sel.start.row..=data_end_row.min(rows_end) { + writes.push((CellRef::new(r, sel.end.col), sum_formula(func, CellRef::new(r, sel.start.col), CellRef::new(r, data_end_col)))); + } + // Both empty: the corner takes the total of the whole block. + if last_row_blank { + writes.push(( + CellRef::new(sel.end.row, sel.end.col), + sum_formula(func, CellRef::new(sel.start.row, sel.start.col), CellRef::new(data_end_row, data_end_col)), + )); + } + } + Ok(writes) +} + +/// The range a single cell's AutoSum guesses: the numbers above it, else the numbers to its left. +fn guessed_range(sh: &gridcraft_model::sheet::Sheet, at: CellRef) -> Option { let numeric = |c: CellRef| sh.value(c).is_number(); let mut range = None; if at.row > 0 && (numeric(CellRef::new(at.row - 1, at.col)) || at.col == 0 || !numeric(CellRef::new(at.row, at.col - 1))) { @@ -186,7 +184,33 @@ fn auto_sum(s: &mut Session, p: &Json) -> Result { } range = Some(RangeRef::new(CellRef::new(at.row, left), CellRef::new(at.row, at.col - 1))); } - let text = match range { + range +} + +fn auto_sum(s: &mut Session, p: &Json) -> Result { + let func = str_param(p, "function").unwrap_or("SUM").to_ascii_uppercase(); + let d = s.doc()?; + let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sel = match str_param(p, "range") { + Some(_) => target_range(s, p)?, + None => d.selection.current(), + }; + if !sel.is_single() { + let Some(writes) = selection_totals(sh, sel, &func)? else { return ok() }; + let sheet = d.wb.active_sheet; + edit(s, |cx| { + for (c, f) in &writes { + let cell = super::edit::input_to_cell(f, sheet, *c, &mut cx.wb)?.unwrap_or_default(); + cx.sheet_mut(sheet)?.set_cell(*c, cell); + cx.touch(sheet, *c); + } + Ok(()) + })?; + return ok(); + } + // Single cell: guess the range above, else to the left. + let at = sel.start; + let text = match guessed_range(sh, at) { Some(r) => format!("={func}({})", r.a1()), None => { s.ui_requests.push(crate::UiRequest::EditCell(Some(format!("={func}()")))); diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index d13b6e4df..1c5704ee3 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2046,38 +2046,46 @@ fn update_cursor(app: &mut SheetApp, ui: &egui::Ui, resp: &egui::Response, geo: } // A held drag owns the cursor even outside the grid; a press below egui's threshold still // has Drag::None and uses hover. A click can briefly leave a drag state after release. - if resp.dragged() && pointer.primary_down() { - let cursor = match app.grid.drag { - Drag::None => None, - Drag::Select => Some(CursorIcon::Cell), - Drag::Rows(_) | Drag::Cols(_) => Some(CursorIcon::Default), - Drag::Fill { .. } => Some(CursorIcon::Crosshair), - Drag::Move { .. } | Drag::MoveCols { .. } | Drag::Object { resize: false, .. } => Some(CursorIcon::Move), - Drag::ResizeCol { .. } => Some(CursorIcon::ResizeColumn), - Drag::ResizeRow { .. } => Some(CursorIcon::ResizeRow), - Drag::Freeze { cols, .. } => Some(if cols { CursorIcon::ResizeHorizontal } else { CursorIcon::ResizeVertical }), - Drag::Object { resize: true, .. } => Some(CursorIcon::ResizeNwSe), - }; - if let Some(cursor) = cursor { - ctx.set_cursor_icon(cursor); - return; - } + if resp.dragged() + && pointer.primary_down() + && let Some(cursor) = drag_cursor(&app.grid.drag) + { + ctx.set_cursor_icon(cursor); + return; } if !resp.hovered() { return; } let Some(p) = pos else { return }; + hover_cursor(app, ctx, resp, &pointer, geo, sh, p); +} + +/// The cursor a held drag shows, or `None` when the drag leaves it to hover. +fn drag_cursor(drag: &Drag) -> Option { + match drag { + Drag::None => None, + Drag::Select => Some(CursorIcon::Cell), + Drag::Rows(_) | Drag::Cols(_) => Some(CursorIcon::Default), + Drag::Fill { .. } => Some(CursorIcon::Crosshair), + Drag::Move { .. } | Drag::MoveCols { .. } | Drag::Object { resize: false, .. } => Some(CursorIcon::Move), + Drag::ResizeCol { .. } => Some(CursorIcon::ResizeColumn), + Drag::ResizeRow { .. } => Some(CursorIcon::ResizeRow), + Drag::Freeze { cols, .. } => Some(if *cols { CursorIcon::ResizeHorizontal } else { CursorIcon::ResizeVertical }), + Drag::Object { resize: true, .. } => Some(CursorIcon::ResizeNwSe), + } +} + +/// The cursor over the grid when no drag owns it: freeze bars, header edges, the fill handle, cells. +fn hover_cursor(app: &SheetApp, ctx: &egui::Context, resp: &egui::Response, pointer: &egui::PointerState, geo: &Geo, sh: &Sheet, p: Pos2) { + let (freeze_cols, freeze_rows) = freeze_handles(geo); if freeze_cols.contains(p) || freeze_rows.contains(p) { - let cols = freeze_cols.contains(p); - ctx.set_cursor_icon(if cols { CursorIcon::ResizeHorizontal } else { CursorIcon::ResizeVertical }); - if app.grid.drag == Drag::None && !pointer.any_down() { - let tip = if cols { "Drag to freeze columns" } else { "Drag to freeze rows" }; - resp.clone().on_hover_text_at_pointer(crate::i18n::current(ctx).tr(tip)); - } + freeze_cursor(app, ctx, resp, pointer, freeze_cols.contains(p)); return; } + let in_cells = geo.in_cells(p); let in_col_header = p.y < geo.cells.top() && p.y >= geo.head_top() && p.x >= geo.cells.left(); let in_row_header = p.x < geo.cells.left() && p.x >= geo.head_left() && p.y >= geo.cells.top(); + let near_col_edge = in_col_header && col_edge(geo, sh, p.x).is_some(); let sel = app.session.active().map(|d| d.selection.clone()).unwrap_or_default(); let cur_rect = geo.range_rect(sh, sel.current()); let handle = Rect::from_center_size(cur_rect.right_bottom(), vec2(9.0, 9.0)); @@ -2089,16 +2097,8 @@ fn update_cursor(app: &mut SheetApp, ui: &egui::Ui, resp: &egui::Response, geo: let near_corner = selected_object.is_some_and(|(_, _, rect)| (p - rect.right_bottom()).length() < OBJECT_RESIZE_RADIUS); if app.grid.drag == Drag::None && near_corner { ctx.set_cursor_icon(CursorIcon::ResizeNwSe); - } else if (in_col_header && col_edge(geo, sh, p.x).is_some()) || (in_row_header && row_edge(geo, sh, p.y).is_some()) { - let near_col_edge = in_col_header && col_edge(geo, sh, p.x).is_some(); - // Just past a hidden column or row, Excel shows a split cursor: dragging or - // double-clicking there unhides it. - let hidden = app.grid.drag == Drag::None - && if near_col_edge { hidden_col_edge(geo, sh, p.x).is_some() } else { hidden_row_edge(geo, sh, p.y).is_some() }; - match pointer.hover_pos() { - Some(at) if hidden => paint_split_cursor(ctx, at, near_col_edge), - _ => ctx.set_cursor_icon(if near_col_edge { CursorIcon::ResizeColumn } else { CursorIcon::ResizeRow }), - } + } else if near_col_edge || (in_row_header && row_edge(geo, sh, p.y).is_some()) { + header_edge_cursor(app, ctx, pointer, geo, sh, p, near_col_edge); } else if on_handle && app.editor.is_none() { ctx.set_cursor_icon(CursorIcon::Crosshair); if app.grid.drag == Drag::None && !pointer.any_down() { @@ -2115,6 +2115,26 @@ fn update_cursor(app: &mut SheetApp, ui: &egui::Ui, resp: &egui::Response, geo: } } +/// The resize cursor for dragging a freeze bar. +fn freeze_cursor(app: &SheetApp, ctx: &egui::Context, resp: &egui::Response, pointer: &egui::PointerState, cols: bool) { + ctx.set_cursor_icon(if cols { CursorIcon::ResizeHorizontal } else { CursorIcon::ResizeVertical }); + if app.grid.drag == Drag::None && !pointer.any_down() { + let tip = if cols { "Drag to freeze columns" } else { "Drag to freeze rows" }; + resp.clone().on_hover_text_at_pointer(crate::i18n::current(ctx).tr(tip)); + } +} + +/// The resize cursor over a column or row header edge. Just past a hidden column or row, Excel +/// shows a split cursor: dragging or double-clicking there unhides it. +fn header_edge_cursor(app: &SheetApp, ctx: &egui::Context, pointer: &egui::PointerState, geo: &Geo, sh: &Sheet, p: Pos2, near_col_edge: bool) { + let hidden = + app.grid.drag == Drag::None && if near_col_edge { hidden_col_edge(geo, sh, p.x).is_some() } else { hidden_row_edge(geo, sh, p.y).is_some() }; + match pointer.hover_pos() { + Some(at) if hidden => paint_split_cursor(ctx, at, near_col_edge), + _ => ctx.set_cursor_icon(if near_col_edge { CursorIcon::ResizeColumn } else { CursorIcon::ResizeRow }), + } +} + fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &Geo, sh: &Sheet, wb: &Workbook) { // Text selection and editing must not also move the object or edit worksheet cells. if app.text_box_editor.is_some() || app.grid.hf_edit.is_some() { diff --git a/crates/xlsx/src/styles.rs b/crates/xlsx/src/styles.rs index 0baff5c0f..c1f93aca9 100644 --- a/crates/xlsx/src/styles.rs +++ b/crates/xlsx/src/styles.rs @@ -359,59 +359,55 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn } } } - let dxfs = root - .child("dxfs") - .map(|d| { - d.kids("dxf") - .map(|x| { - let mut s = Style::default(); - if let Some(e) = x.child("font") { - let f = read_font(e, &palette); - // Differential fonts rarely name a face; unless stated, keep the default face and size. - let has_size = e.child("sz").and_then(|n| n.attr_f64("val")).is_some(); - let has_name = e.child("name").and_then(|n| n.attr("val")).is_some(); - let name = if has_name { f.name.clone() } else { s.font.name.clone() }; - let size = if has_size { f.size } else { s.font.size }; - let def = Font::default(); - let st = &mut s.dxf_stated; - st.bold = e.child("b").is_some() && !f.bold; - st.italic = e.child("i").is_some() && !f.italic; - st.strike = e.child("strike").is_some() && !f.strike; - st.underline = e.child("u").is_some() && f.underline == Underline::None; - st.size = has_size && size == def.size; - st.name = has_name && name == def.name; - st.scheme = e.child_val("scheme").map(str::to_string); - // A differential font's scheme is in `dxf_stated`. - s.font = Font { name, size, minor: s.font.minor, ..f }; - } - if let Some(n) = x.child("numFmt") { - if let Some(code) = n.attr("formatCode") { - s.num_fmt = NumFmt::new(code); - } else if let Some(id) = n.attr_u32("numFmtId") { - s.num_fmt = num_fmt_code(id, &fmts); - } - } - if let Some(f) = x.child("fill") { - s.fill = read_fill(f, &palette, true); - } - if let Some(b) = x.child("border") { - s.borders = read_border(b, &palette); - } - if let Some(a) = x.child("alignment") { - s.align = read_alignment(a); - } - if let Some(p) = x.child("protection") { - s.protection = read_protection(p); - s.dxf_stated.protection = s.protection == Protection::default(); - } - s - }) - .collect() - }) - .unwrap_or_default(); + let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &fmts)).collect()).unwrap_or_default(); StylesIn { xfs, dxfs, palette, named, num_fmts: fmts } } +/// One differential format (`dxf`): the style it sets over a cell's own. +fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap) -> Style { + let mut s = Style::default(); + if let Some(e) = x.child("font") { + let f = read_font(e, palette); + // Differential fonts rarely name a face; unless stated, keep the default face and size. + let has_size = e.child("sz").and_then(|n| n.attr_f64("val")).is_some(); + let has_name = e.child("name").and_then(|n| n.attr("val")).is_some(); + let name = if has_name { f.name.clone() } else { s.font.name.clone() }; + let size = if has_size { f.size } else { s.font.size }; + let def = Font::default(); + let st = &mut s.dxf_stated; + st.bold = e.child("b").is_some() && !f.bold; + st.italic = e.child("i").is_some() && !f.italic; + st.strike = e.child("strike").is_some() && !f.strike; + st.underline = e.child("u").is_some() && f.underline == Underline::None; + st.size = has_size && size == def.size; + st.name = has_name && name == def.name; + st.scheme = e.child_val("scheme").map(str::to_string); + // A differential font's scheme is in `dxf_stated`. + s.font = Font { name, size, minor: s.font.minor, ..f }; + } + if let Some(n) = x.child("numFmt") { + if let Some(code) = n.attr("formatCode") { + s.num_fmt = NumFmt::new(code); + } else if let Some(id) = n.attr_u32("numFmtId") { + s.num_fmt = num_fmt_code(id, fmts); + } + } + if let Some(f) = x.child("fill") { + s.fill = read_fill(f, palette, true); + } + if let Some(b) = x.child("border") { + s.borders = read_border(b, palette); + } + if let Some(a) = x.child("alignment") { + s.align = read_alignment(a); + } + if let Some(p) = x.child("protection") { + s.protection = read_protection(p); + s.dxf_stated.protection = s.protection == Protection::default(); + } + s +} + // ---------------------------------------------------------------- writing pub fn color_attrs(c: &Color) -> String {