From 4e86fe7ad30be1490cd883f88b058a49e36bc6d4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 08:14:50 +0000 Subject: [PATCH] Sonar: explicit imports, fn paths for closures, extract helpers for cognitive complexity Co-Authored-By: Claude Sonnet 5.5 --- apps/gridcraft-cli/src/check.rs | 4 +- apps/gridcraft-web/src/web.rs | 2 +- apps/gridcraft/src/main.rs | 4 +- apps/gridcraft/src/print.rs | 4 +- crates/chart/src/render.rs | 2 +- crates/chart/src/tests.rs | 2 +- crates/core/src/addr.rs | 2 +- crates/core/src/value.rs | 4 +- crates/engine/src/cf.rs | 5 +- crates/engine/src/cmd/extra.rs | 2 +- crates/engine/src/cmd/file.rs | 6 +- crates/engine/src/cmd/mod.rs | 2 +- crates/engine/src/cmd/pivot.rs | 2 +- crates/engine/src/cmd/print.rs | 2 +- crates/engine/src/cmd/sheet.rs | 46 +++++--- crates/engine/src/cmd/solver.rs | 49 +++++---- crates/engine/src/fill.rs | 4 +- crates/engine/src/io.rs | 6 +- crates/engine/src/lib.rs | 7 +- crates/engine/src/pivot.rs | 5 +- crates/engine/src/tables.rs | 2 +- .../engine/tests/it/command_sequence_fuzz.rs | 2 +- crates/formula/src/adjust.rs | 2 +- crates/formula/src/parser.rs | 2 +- crates/formula/src/printer.rs | 2 +- crates/functions/src/criteria.rs | 104 ++++++++++-------- crates/functions/src/engineering.rs | 2 +- crates/functions/src/ets.rs | 4 +- crates/functions/src/lib.rs | 2 +- crates/functions/src/stat.rs | 2 +- crates/functions/src/text.rs | 10 +- crates/model/src/sheet.rs | 10 +- crates/numfmt/src/render.rs | 2 +- crates/pdf/src/metrics.rs | 4 +- crates/ui-egui/src/backstage.rs | 6 +- crates/ui-egui/src/dialogs.rs | 4 +- crates/ui-egui/src/fonts.rs | 14 ++- crates/ui-egui/src/grid.rs | 3 +- crates/ui-egui/src/lib.rs | 2 +- crates/ui-egui/src/theme.rs | 60 +++++----- crates/xlsx/src/drawing/ink.rs | 39 ++++++- crates/xlsx/src/lib.rs | 2 +- crates/xlsx/src/pivot.rs | 2 +- crates/xlsx/src/xml.rs | 2 +- xtask/src/assets.rs | 2 +- xtask/src/i18n_report.rs | 4 +- xtask/src/layers.rs | 2 +- 47 files changed, 264 insertions(+), 188 deletions(-) diff --git a/apps/gridcraft-cli/src/check.rs b/apps/gridcraft-cli/src/check.rs index 74d2fca71..253fcd54a 100644 --- a/apps/gridcraft-cli/src/check.rs +++ b/apps/gridcraft-cli/src/check.rs @@ -304,7 +304,7 @@ fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool { if !walk(&p, depth + 1, out) { return false; } - } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) + } else if p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) && !out.add_file(p) { return false; @@ -319,7 +319,7 @@ fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool { pub fn check_bytes(path: &str, bytes: &[u8], opts: &Options) -> FileReport { let guarded = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| check_inner(path, bytes, opts))); guarded.unwrap_or_else(|p| { - let msg = p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); + let msg = p.downcast_ref::<&str>().map(ToString::to_string).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); FileReport { path: path.to_string(), error: Some(format!("internal error while checking: {msg}")), ..Default::default() } }) } diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index 8e2e60837..31c2b6108 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -350,7 +350,7 @@ fn download(name: &str, bytes: &[u8]) -> Result<(), String> { js_sys::Reflect::get(&window, &JsValue::from_str("showSaveFilePicker")).ok().and_then(|v| v.dyn_into().ok()); if let Some(picker_fn) = show_picker_fn { - let ext = std::path::Path::new(name).extension().and_then(|e| e.to_str()).unwrap_or("xlsx").to_ascii_lowercase(); + let ext = std::path::Path::new(name).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("xlsx").to_ascii_lowercase(); let dot_ext = format!(".{ext}"); let mime = mime_for(name); diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 6c0d2f676..1f90bb865 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -180,11 +180,11 @@ fn services() -> Services { })), pick_save: Some(Box::new(|suggested: &str| { let path = std::path::Path::new(suggested); - let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or("Book1").to_string(); + let stem = path.file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Book1").to_string(); // The suggested name's type (an export asks for .pdf, .csv or .html) comes first. const TYPES: [(&str, &str); 4] = [("Excel Workbook (.xlsx)", "xlsx"), ("CSV UTF-8 (.csv)", "csv"), ("Web Page (.html)", "html"), ("PDF (.pdf)", "pdf")]; - let ext = path.extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase).filter(|e| TYPES.iter().any(|(_, x)| x == e)); + let ext = path.extension().and_then(std::ffi::OsStr::to_str).map(str::to_ascii_lowercase).filter(|e| TYPES.iter().any(|(_, x)| x == e)); let ext = ext.unwrap_or_else(|| "xlsx".to_string()); let mut dialog = rfd::FileDialog::new().set_file_name(format!("{stem}.{ext}")); for (label, x) in TYPES.iter().filter(|(_, x)| *x == ext).chain(TYPES.iter().filter(|(_, x)| *x != ext)) { diff --git a/apps/gridcraft/src/print.rs b/apps/gridcraft/src/print.rs index 09bb801bd..4ab1f32db 100644 --- a/apps/gridcraft/src/print.rs +++ b/apps/gridcraft/src/print.rs @@ -17,7 +17,7 @@ pub fn discover() -> Discover { static JOBS: std::sync::Mutex>> = std::sync::Mutex::new(Vec::new()); pub fn finish_pending_jobs() { - let workers = std::mem::take(&mut *JOBS.lock().unwrap_or_else(|poison| poison.into_inner())); + let workers = std::mem::take(&mut *JOBS.lock().unwrap_or_else(std::sync::PoisonError::into_inner)); for worker in workers { if worker.join().is_err() { log::error!("the print worker stopped unexpectedly during shutdown"); @@ -45,7 +45,7 @@ pub fn submit() -> Submit { let _ = sender.send(craft_print::submit(&bytes, &job).map_err(|error| error.to_string())); }); if let Ok(worker) = worker { - let mut jobs = JOBS.lock().unwrap_or_else(|poison| poison.into_inner()); + let mut jobs = JOBS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); jobs.retain(|job| !job.is_finished()); jobs.push(worker); } diff --git a/crates/chart/src/render.rs b/crates/chart/src/render.rs index 0a6fdbe32..2e495ee5b 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -425,7 +425,7 @@ fn frame(ctx: &mut Ctx, area: Bx, cat: CatAxis, y: &ValAxis, y2: Option<&ValAxis let maxw = |ctx: &Ctx, l: &[(f64, String)]| l.iter().map(|(_, t)| ctx.tw(t, ls)).fold(0.0f32, f32::max); if !horizontal { let left = (maxw(ctx, &ylabels) + 6.0).min(area.w * 0.4); - let y2labels = y2.map(|a| a.labels()); + let y2labels = y2.map(ValAxis::labels); let right = match (&y2labels, &cat) { (Some(l), _) => (maxw(ctx, l) + 6.0).min(area.w * 0.3), (None, CatAxis::Numeric(s, f)) => (ctx.tw(&f.format(s.max, Some(s.step)), ls) / 2.0).min(area.w * 0.2), diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index cfe5d5a42..846b19f53 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -76,7 +76,7 @@ fn sample(kind: ChartKind) -> ChartData { ChartData { category_levels: vec![], palette: vec![], - categories: cats.iter().map(|s| s.to_string()).collect(), + categories: cats.iter().map(ToString::to_string).collect(), series: vec![ series("North", &[12000.0, -4000.0, 23000.0, 31000.0, 18000.0, 27000.0], [20, 96, 130, 255], k1), b, diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 45812b0b6..234e4d345 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -246,7 +246,7 @@ fn is_r1c1_token(name: &str) -> bool { if chars.peek().is_some_and(|c| c.to_ascii_lowercase() == letter) { chars.next(); letters += 1; - while chars.peek().is_some_and(|c| c.is_ascii_digit()) { + while chars.peek().is_some_and(char::is_ascii_digit) { chars.next(); } } diff --git a/crates/core/src/value.rs b/crates/core/src/value.rs index 9a5c09bdf..c531de4dc 100644 --- a/crates/core/src/value.rs +++ b/crates/core/src/value.rs @@ -245,7 +245,7 @@ impl Value { Value::Bool(b) => Ok(if *b { "TRUE".into() } else { "FALSE".into() }), Value::Text(t) => Ok(t.to_string()), Value::Error(e) => Err(*e), - Value::Array(a) => a.data.first().map(|v| v.to_text()).unwrap_or(Ok(String::new())), + Value::Array(a) => a.data.first().map(Value::to_text).unwrap_or(Ok(String::new())), } } /// Coercion to a boolean as `IF` does it. @@ -264,7 +264,7 @@ impl Value { } } Value::Error(e) => Err(*e), - Value::Array(a) => a.data.first().map(|v| v.to_bool()).unwrap_or(Ok(false)), + Value::Array(a) => a.data.first().map(Value::to_bool).unwrap_or(Ok(false)), } } /// Display text with General formatting (for logs, CSV and tests). diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs index 425e5264e..ffb4130ca 100644 --- a/crates/engine/src/cf.rs +++ b/crates/engine/src/cf.rs @@ -4,7 +4,10 @@ use std::collections::HashMap; use gridcraft_core::{CellRef, RangeRef, Value, compare}; -use gridcraft_model::*; +use gridcraft_model::{ + Cell, CfOperator, CfRule, CfValueKind, Color, CondFormat, Fill, Formula, PatternType, Sheet, Style, Underline, Workbook, default_icon_thresholds, + icon_count, +}; /// The conditional look of one cell. #[derive(Clone, Debug, Default, PartialEq)] diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 009d77f2a..71e82df52 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -941,7 +941,7 @@ fn from_csv(s: &mut Session, p: &Json) -> Result { return Err(EngineError::Other("Nothing to import.".into())); } let name = str_param(p, "sheet").map(str::to_string).or_else(|| { - str_param(p, "path").and_then(|x| std::path::Path::new(x).file_stem().and_then(|f| f.to_str()).map(|f| f.chars().take(31).collect())) + str_param(p, "path").and_then(|x| std::path::Path::new(x).file_stem().and_then(std::ffi::OsStr::to_str).map(|f| f.chars().take(31).collect())) }); let r = s.execute("home.insertSheet", json!({"name": name})).or_else(|_| s.execute("home.insertSheet", json!({})))?; let w = rows.iter().map(Vec::len).max().unwrap_or(1).max(1); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index b3e7309e2..f261ea238 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -69,9 +69,9 @@ fn open(s: &mut Session, p: &Json) -> Result { matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb | crate::io::FileKind::Xls)) || matches!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Ods | gridcraft_xlsx::Format::Xlsb | gridcraft_xlsx::Format::Xls); let title = if imported { - format!("{} (imported).xlsx", std::path::Path::new(&name).file_stem().and_then(|n| n.to_str()).unwrap_or("Book")) + format!("{} (imported).xlsx", std::path::Path::new(&name).file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Book")) } else { - std::path::Path::new(&name).file_name().and_then(|n| n.to_str()).unwrap_or("Book").to_string() + std::path::Path::new(&name).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("Book").to_string() }; // An import has no save target: ordinary Save must ask for a new destination. Neither does a // workbook decrypted with its password, so Save never replaces the encrypted original with @@ -114,7 +114,7 @@ fn save(s: &mut Session, p: &Json) -> Result { }; // A PDF is a print of the sheet (Page Setup applies), not a copy of the workbook: the // document keeps its file and its unsaved changes. - if std::path::Path::new(&path).extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("pdf")) { + if std::path::Path::new(&path).extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("pdf")) { return super::print::export_pdf(s, &json!({"path": path})); } let d = s.doc_mut()?; diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 8a0bb1052..02b06c32a 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -165,7 +165,7 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> { test_commands().iter().find(|c| c.id == id).copied() } -static TEST_COMMANDS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); +static TEST_COMMANDS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); /// Test-only: register an extra command so UI integration tests can prove a pending /// keytip becomes live the moment its command lands (see crates/ui-egui/tests/keytips.rs). diff --git a/crates/engine/src/cmd/pivot.rs b/crates/engine/src/cmd/pivot.rs index 229ff22b6..9035555b6 100644 --- a/crates/engine/src/cmd/pivot.rs +++ b/crates/engine/src/cmd/pivot.rs @@ -244,7 +244,7 @@ fn find_pivot(s: &Session, p: &Json) -> Result<(usize, usize)> { } fn area_param(cmd: &str, v: Option<&str>) -> Result<&'static str> { - match v.map(|x| x.to_ascii_lowercase()) { + match v.map(str::to_ascii_lowercase) { Some(a) if a == "rows" || a == "row" => Ok("rows"), Some(a) if a == "columns" || a == "column" || a == "cols" => Ok("columns"), Some(a) if a == "values" || a == "value" || a == "data" => Ok("values"), diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index 4c80f2fa9..4b09f25fb 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -599,7 +599,7 @@ pub fn expand_header_footer(spec: &str, cx: &HfContext) -> [HfSection; 3] { i += rest.chars().count() + 1; } d if d.is_ascii_digit() => { - let digits: String = std::iter::once(d).chain(chars[i..].iter().copied().take_while(|c| c.is_ascii_digit())).take(3).collect(); + let digits: String = std::iter::once(d).chain(chars[i..].iter().copied().take_while(char::is_ascii_digit)).take(3).collect(); i += digits.len() - 1; if let Ok(v) = digits.parse::() { sec.size = Some(v.clamp(1.0, 409.0)); diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index a453f250b..40f0ce434 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -7,7 +7,10 @@ use gridcraft_formula::adjust::{Axis, Edit}; use gridcraft_model::{Anchor, AnchorMode, Sheet, Visibility}; use serde_json::{Value as Json, json}; -use super::*; +use super::{ + CommandSpec, Ctx, EngineError, PROTECTED, Result, Session, bad, bool_param, cell_param_on, cmd, commit, edit, has_clipboard, has_doc, info, ok, + parse_lines, sheet_index, sheet_mut, str_param, target_range, target_sheet, touch, u32_param, +}; pub fn specs() -> Vec { vec![ @@ -270,6 +273,29 @@ fn cancel_copy_mode(s: &mut Session) { } } +fn inherit_line_format(sh: &mut Sheet, axis: Axis, at: u32, count: u32) { + if at == 0 { + return; + } + let prev = if axis == Axis::Rows { sh.rows.get(&(at - 1)).copied() } else { sh.cols.get(&(at - 1)).copied() }; + if let Some(info) = prev { + for i in at..at + count { + let map = if axis == Axis::Rows { &mut sh.rows } else { &mut sh.cols }; + map.insert(i, gridcraft_model::LineInfo { hidden: false, outline: 0, collapsed: false, ..info }); + } + } +} + +fn check_push_off_sheet(s: &Session, sheet: usize, axis: Axis, at: u32, count: u32, limit: u32) -> Result<()> { + if let Some(u) = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.used_range()) { + let last = if axis == Axis::Rows { u.end.row } else { u.end.col }; + if last >= at && last as u64 + count as u64 >= limit as u64 { + return Err(EngineError::Other("To prevent possible loss of data, Excel cannot shift nonblank cells off of the worksheet.".into())); + } + } + Ok(()) +} + fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { let (at, count) = lines(s, p, axis)?; let limit = if axis == Axis::Rows { MAX_ROWS } else { MAX_COLS }; @@ -279,13 +305,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } check_lines_protection(s, p, axis, at, count, true)?; let sheet = target_sheet(s, p)?; - // Refuse to push data off the sheet. - if let Some(u) = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.used_range()) { - let last = if axis == Axis::Rows { u.end.row } else { u.end.col }; - if last >= at && last as u64 + count as u64 >= limit as u64 { - return Err(EngineError::Other("To prevent possible loss of data, Excel cannot shift nonblank cells off of the worksheet.".into())); - } - } + check_push_off_sheet(s, sheet, axis, at, count, limit)?; let out = edit(s, |cx| { let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); cx.protection_checked = true; @@ -297,15 +317,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { } shift_sheet_features(sh, axis, at, count, true); // New lines take the format of the line before (Excel's "Format Same As Above"). - if at > 0 { - let prev = if axis == Axis::Rows { sh.rows.get(&(at - 1)).copied() } else { sh.cols.get(&(at - 1)).copied() }; - if let Some(info) = prev { - for i in at..at + count { - let map = if axis == Axis::Rows { &mut sh.rows } else { &mut sh.cols }; - map.insert(i, gridcraft_model::LineInfo { hidden: false, outline: 0, collapsed: false, ..info }); - } - } - } + inherit_line_format(sh, axis, at, count); } rewrite(&mut cx.wb, &name, &Edit::Insert { axis, at, count }); cx.structural = true; diff --git a/crates/engine/src/cmd/solver.rs b/crates/engine/src/cmd/solver.rs index ffd426f5f..f5a4d6732 100644 --- a/crates/engine/src/cmd/solver.rs +++ b/crates/engine/src/cmd/solver.rs @@ -29,7 +29,10 @@ use gridcraft_model::{Cell, DefinedName, Sheet, Workbook}; use serde_json::{Value as Json, json}; use super::whatif::Trial; -use super::*; +use super::{ + CommandSpec, Ctx, DocState, EngineError, Result, Session, bad, bool_param, cmd, edit, f64_param, has_doc, sheet_index, sheet_mut, str_param, + touch, +}; const CMD: &str = "data.solver"; const REPORT_CMD: &str = "data.solverReport"; @@ -1705,6 +1708,26 @@ fn title(out: &mut Lines, kind: ReportKind, sheet: &str) -> u32 { 3 } +fn format_constraint_row(sh: &Sheet, c: &Constraint, cell: CellRef, v: f64, rhs: &[f64], k: usize) -> Vec { + let (formula, status, slack) = match (c.rel, &c.rhs) { + (Rel::Int, _) => (format!("{}=Integer", abs_ref(cell)), "Binding", 0.0), + (Rel::Bin, _) => (format!("{}=Binary", abs_ref(cell)), "Binding", 0.0), + (rel, rhs_cells) => { + let b = rhs.get(k).or_else(|| rhs.first()).copied().unwrap_or(f64::NAN); + let rhs_text = match rhs_cells { + Rhs::Cells(cells) => cells.get(k).or_else(|| cells.first()).map(|c| abs_ref(*c)).unwrap_or_default(), + _ => gridcraft_core::number_to_text(b), + }; + let slack = (v - b).abs(); + let binding = rel == Rel::Eq || slack <= PRECISION * (1.0 + b.abs()); + let slack = if binding { 0.0 } else { slack }; + (format!("{}{}{rhs_text}", abs_ref(cell), rel.text()), if binding { "Binding" } else { "Not Binding" }, slack) + } + }; + let num = |v: f64| if v.is_finite() { Value::number(v) } else { Value::Error(gridcraft_core::CellError::Value) }; + vec![Value::text(abs_ref(cell)), Value::text(cell_name(sh, cell)), num(v), Value::text(formula), Value::text(status), num(slack)] +} + fn answer_report(d: &DocState, sh: &Sheet, inp: &ReportInput) -> Lines { let m = inp.model; let mut trial = Trial::new(d); @@ -1768,29 +1791,7 @@ fn answer_report(d: &DocState, sh: &Sheet, inp: &ReportInput) -> Lines { Rhs::None => vec![], }; for (k, (cell, v)) in c.lhs.iter().zip(&lhs).enumerate() { - let (formula, status, slack) = match (c.rel, &c.rhs) { - (Rel::Int, _) => (format!("{}=Integer", abs_ref(*cell)), "Binding", 0.0), - (Rel::Bin, _) => (format!("{}=Binary", abs_ref(*cell)), "Binding", 0.0), - (rel, rhs_cells) => { - let b = rhs.get(k).or_else(|| rhs.first()).copied().unwrap_or(f64::NAN); - let rhs_text = match rhs_cells { - Rhs::Cells(cells) => cells.get(k).or_else(|| cells.first()).map(|c| abs_ref(*c)).unwrap_or_default(), - _ => gridcraft_core::number_to_text(b), - }; - let slack = (v - b).abs(); - let binding = rel == Rel::Eq || slack <= PRECISION * (1.0 + b.abs()); - let slack = if binding { 0.0 } else { slack }; - (format!("{}{}{rhs_text}", abs_ref(*cell), rel.text()), if binding { "Binding" } else { "Not Binding" }, slack) - } - }; - rows.push(vec![ - Value::text(abs_ref(*cell)), - Value::text(cell_name(sh, *cell)), - num(*v), - Value::text(formula), - Value::text(status), - num(slack), - ]); + rows.push(format_constraint_row(sh, c, *cell, *v, &rhs, k)); } } table(&mut out, r + 1, &["Cell", "Name", "Cell Value", "Formula", "Status", "Slack"], rows); diff --git a/crates/engine/src/fill.rs b/crates/engine/src/fill.rs index 9ff8eac31..3f1f6dd53 100644 --- a/crates/engine/src/fill.rs +++ b/crates/engine/src/fill.rs @@ -388,9 +388,9 @@ fn ordinal_suffix(n: i64, suffix: &str) -> String { /// `Item 007` → ("Item ", 7, "", 3); `3rd` style leading numbers → ("", 3, "rd", 1). fn split_num(t: &str) -> Option<(String, i64, String, usize)> { let chars: Vec = t.chars().collect(); - let end = chars.iter().rposition(|c| c.is_ascii_digit())?; + let end = chars.iter().rposition(char::is_ascii_digit)?; let mut start = end; - while start > 0 && chars.get(start - 1).is_some_and(|c| c.is_ascii_digit()) { + while start > 0 && chars.get(start - 1).is_some_and(char::is_ascii_digit) { start -= 1; } let digits: String = chars.get(start..=end)?.iter().collect(); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index b18e8577b..c83797c1b 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -209,7 +209,7 @@ fn part_too_large_message(name: &str, part: &str, size: u64) -> String { /// A CSV opens as a sheet named after the file (like Excel). fn rename_first_sheet(wb: &mut Workbook, path: &str) { - let stem = std::path::Path::new(path).file_stem().and_then(|s| s.to_str()).unwrap_or("Sheet1"); + let stem = std::path::Path::new(path).file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Sheet1"); let clean: String = stem.chars().filter(|c| !matches!(c, ':' | '\\' | '/' | '?' | '*' | '[' | ']')).take(31).collect(); let clean = clean.trim_matches('\'').to_string(); // "History" is reserved (see `Workbook::check_sheet_name`): keep the default name instead. @@ -226,7 +226,7 @@ pub fn save_bytes(wb: &Workbook, path: &str) -> Result> { // An unknown or missing extension is refused rather than written as an XLSX under that name: // a typo like `.cvs` would otherwise produce a file no other app recognises. let Some(kind) = FileKind::from_path(path) else { - let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(path); + let name = std::path::Path::new(path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(path); return Err(EngineError::Other(format!("{name}: unknown file format. Save as .xlsx, .csv, .tsv, .json or .html."))); }; save_bytes_as(wb, path, kind) @@ -239,7 +239,7 @@ pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result { - let book = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or_default(); + let book = std::path::Path::new(path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or_default(); gridcraft_xlsx::write_xlsx_named(wb, gridcraft_xlsx::XlsxKind::from_path(path), book).map_err(|e| EngineError::Other(e.to_string())) } FileKind::Xls => { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index b4ab60f3a..ce2a50c6b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -572,8 +572,11 @@ impl Session { let result = match result { Ok(r) => r, Err(p) => { - let msg = - p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()); + let msg = p + .downcast_ref::<&str>() + .map(ToString::to_string) + .or_else(|| p.downcast_ref::().cloned()) + .unwrap_or_else(|| "panic".into()); // Restore the workbook and its history as they were, and drop the undo groups the // command opened. self.groups.retain(|g| g.id < first_group); diff --git a/crates/engine/src/pivot.rs b/crates/engine/src/pivot.rs index 397dd5ad2..1329d31c9 100644 --- a/crates/engine/src/pivot.rs +++ b/crates/engine/src/pivot.rs @@ -11,7 +11,10 @@ use std::collections::{HashMap, HashSet}; use gridcraft_core::date::datetime_from_serial; pub use gridcraft_core::quote_sheet; use gridcraft_core::{CellError, CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value, split_sheet}; -use gridcraft_model::*; +use gridcraft_model::{ + BorderLine, BorderStyle, Cell, Chart, Color, Fill, HAlign, NumFmt, PivotDateGroup, PivotField, PivotFilter, PivotFunc, PivotLayout, PivotShowAs, + PivotSort, PivotTable, PivotValue, Series, Style, StyleId, Theme, Workbook, +}; use crate::pivot_calc::CalcSet; diff --git a/crates/engine/src/tables.rs b/crates/engine/src/tables.rs index 9182cd488..48e77d01c 100644 --- a/crates/engine/src/tables.rs +++ b/crates/engine/src/tables.rs @@ -2,7 +2,7 @@ //! `TableStyleLight1…21 / Medium1…28 / Dark1…11` names so files round-trip). use gridcraft_core::{CellRef, RangeRef}; -use gridcraft_model::*; +use gridcraft_model::{BorderLine, BorderStyle, Cell, Color, Fill, Formula, PatternType, Style, Table, Theme, Workbook}; /// (family, number) from a style name, e.g. `TableStyleMedium2` → ("Medium", 2). pub fn parse_style_name(name: &str) -> (&'static str, u32) { diff --git a/crates/engine/tests/it/command_sequence_fuzz.rs b/crates/engine/tests/it/command_sequence_fuzz.rs index 012757f69..73ae1edbe 100644 --- a/crates/engine/tests/it/command_sequence_fuzz.rs +++ b/crates/engine/tests/it/command_sequence_fuzz.rs @@ -232,7 +232,7 @@ fn guarded(step: usize, kind: &'static str, f: impl FnOnce() -> Result Ok(v), Ok(Err(e)) => Err(fail(step, kind, e)), Err(p) => { - let msg = p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); + let msg = p.downcast_ref::<&str>().map(ToString::to_string).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); Err(fail(step, kind, format!("panicked: {msg}"))) } } diff --git a/crates/formula/src/adjust.rs b/crates/formula/src/adjust.rs index d4881aa9e..1eaf4413b 100644 --- a/crates/formula/src/adjust.rs +++ b/crates/formula/src/adjust.rs @@ -3,7 +3,7 @@ use gridcraft_core::{CellError, MAX_COLS, MAX_ROWS, RangeRef}; -use crate::ast::*; +use crate::ast::{Anchor, Expr, RefKind, Reference, SheetSel}; /// Copy/fill: relative parts move by `(dr, dc)`. References pushed off the sheet become `#REF!`. pub fn shift_relative(e: Expr, dr: i64, dc: i64) -> Expr { diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 25759f055..4ff5a5b78 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use gridcraft_core::addr::{letters_to_col, parse_a1_prefix}; use gridcraft_core::{CellError, MAX_ROWS}; -use crate::ast::*; +use crate::ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, TrimMode, UnOp, precedence}; use crate::lexer::{Tok, Token, tokenize_locale}; #[derive(Clone, Debug, PartialEq, thiserror::Error)] diff --git a/crates/formula/src/printer.rs b/crates/formula/src/printer.rs index 62d59bff3..8a1c0c45b 100644 --- a/crates/formula/src/printer.rs +++ b/crates/formula/src/printer.rs @@ -4,7 +4,7 @@ use std::fmt::Write; use gridcraft_core::{CellRef, col_to_letters, number_to_text}; -use crate::ast::*; +use crate::ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, UnOp}; pub use gridcraft_core::{quote_sheet, sheet_needs_quotes as needs_quotes}; diff --git a/crates/functions/src/criteria.rs b/crates/functions/src/criteria.rs index 58f0e6594..f78f0aba3 100644 --- a/crates/functions/src/criteria.rs +++ b/crates/functions/src/criteria.rs @@ -123,6 +123,60 @@ impl Criterion { Criterion { op, operand, bare_empty: false } } + fn matches_blank(&self, v: &Value) -> bool { + let blank = matches!(v, Value::Empty); + match self.op { + Op::Eq => blank, + Op::Ne => !blank, + // "<" / ">" with nothing: Excel compares against "" for text cells, but an + // empty string never matches ("<=" and ">=" count it in neither). + _ => match v { + Value::Text(t) if !t.is_empty() => cmp_op(self.op, compare_text(t, "")), + _ => false, + }, + } + } + + fn matches_number(&self, v: &Value, n: f64) -> bool { + let cell = match v { + Value::Number(x) => Some(*x), + // Numeric text matches only for equality (COUNTIF("1") counts text "1"). + Value::Text(t) if matches!(self.op, Op::Eq | Op::Ne) => parse_number_text(t), + _ => None, + }; + match cell { + Some(x) => cmp_op(self.op, compare_numbers(x, n)), + None => self.op == Op::Ne, + } + } + + fn matches_text(&self, v: &Value, pat: &str) -> bool { + match self.op { + Op::Eq | Op::Ne => { + // Blank cells hold no text: `"*"` skips them and `"<>*"` counts them. + let cell_text = match v { + Value::Text(t) => Some(t.to_string()), + _ => None, + }; + let m = cell_text.is_some_and(|t| wildcard_match(pat, &t)); + if self.op == Op::Eq { m } else { !m } + } + _ => match v { + Value::Text(t) => cmp_op(self.op, compare_text(t, pat)), + _ => false, + }, + } + } + + fn matches_cut_text(&self, v: &Value, kept: &str) -> bool { + match self.op { + Op::Eq => false, + Op::Ne => true, + Op::Lt | Op::Le => matches!(v, Value::Text(t) if compare_text(t, kept) != Ordering::Greater), + Op::Gt | Op::Ge => matches!(v, Value::Text(t) if compare_text(t, kept) == Ordering::Greater), + } + } + /// Whether a cell value satisfies the criterion. pub(crate) fn matches(&self, v: &Value) -> bool { let v = match v { @@ -133,31 +187,8 @@ impl Criterion { return matches!(&v, Value::Empty) || matches!(&v, Value::Text(t) if t.is_empty()); } match &self.operand { - Operand::Blank => { - let blank = matches!(v, Value::Empty); - match self.op { - Op::Eq => blank, - Op::Ne => !blank, - // "<" / ">" with nothing: Excel compares against "" for text cells, but an - // empty string never matches ("<=" and ">=" count it in neither). - _ => match &v { - Value::Text(t) if !t.is_empty() => cmp_op(self.op, compare_text(t, "")), - _ => false, - }, - } - } - Operand::Number(n) => { - let cell = match &v { - Value::Number(x) => Some(*x), - // Numeric text matches only for equality (COUNTIF("1") counts text "1"). - Value::Text(t) if matches!(self.op, Op::Eq | Op::Ne) => parse_number_text(t), - _ => None, - }; - match cell { - Some(x) => cmp_op(self.op, compare_numbers(x, *n)), - None => self.op == Op::Ne, - } - } + Operand::Blank => self.matches_blank(&v), + Operand::Number(n) => self.matches_number(&v, *n), Operand::Bool(b) => match &v { Value::Bool(x) => cmp_op(self.op, x.cmp(b)), _ => self.op == Op::Ne, @@ -166,27 +197,8 @@ impl Criterion { Value::Error(x) => cmp_op(self.op, x.code().cmp(&e.code())), _ => self.op == Op::Ne, }, - Operand::Text(pat) => match self.op { - Op::Eq | Op::Ne => { - // Blank cells hold no text: `"*"` skips them and `"<>*"` counts them. - let cell_text = match &v { - Value::Text(t) => Some(t.to_string()), - _ => None, - }; - let m = cell_text.is_some_and(|t| wildcard_match(pat, &t)); - if self.op == Op::Eq { m } else { !m } - } - _ => match &v { - Value::Text(t) => cmp_op(self.op, compare_text(t, pat)), - _ => false, - }, - }, - Operand::CutText(kept) => match self.op { - Op::Eq => false, - Op::Ne => true, - Op::Lt | Op::Le => matches!(&v, Value::Text(t) if compare_text(t, kept) != Ordering::Greater), - Op::Gt | Op::Ge => matches!(&v, Value::Text(t) if compare_text(t, kept) == Ordering::Greater), - }, + Operand::Text(pat) => self.matches_text(&v, pat), + Operand::CutText(kept) => self.matches_cut_text(&v, kept), } } } diff --git a/crates/functions/src/engineering.rs b/crates/functions/src/engineering.rs index f34ce99f0..980c46d1f 100644 --- a/crates/functions/src/engineering.rs +++ b/crates/functions/src/engineering.rs @@ -742,7 +742,7 @@ fn im_binary(a: &[Arg], f: fn(Cx, Cx) -> R) -> R { fmt_complex(f(x, y)?, s.unwrap_or('i')) } fn imdiv(a: &[Arg], _c: &mut dyn Ctx) -> R { - im_binary(a, |x, y| x.div(y)) + im_binary(a, Cx::div) } fn imsub(a: &[Arg], _c: &mut dyn Ctx) -> R { im_binary(a, |x, y| Ok(x.sub(y))) diff --git a/crates/functions/src/ets.rs b/crates/functions/src/ets.rs index 90afb4382..2a502da49 100644 --- a/crates/functions/src/ets.rs +++ b/crates/functions/src/ets.rs @@ -278,7 +278,7 @@ fn aggregate(kind: Aggregate, xs: &mut [f64]) -> Option { Aggregate::Max => xs.iter().copied().reduce(f64::max), Aggregate::Min => xs.iter().copied().reduce(f64::min), Aggregate::Median => { - xs.sort_by(|a, b| a.total_cmp(b)); + xs.sort_by(f64::total_cmp); let k = xs.len() / 2; let hi = xs.get(k).copied()?; if xs.len() % 2 == 1 { Some(hi) } else { Some((xs.get(k.checked_sub(1)?).copied()? + hi) / 2.0) } @@ -415,7 +415,7 @@ impl Timeline { return None; } let mut sorted = times.to_vec(); - sorted.sort_by(|a, b| a.total_cmp(b)); + sorted.sort_by(f64::total_cmp); sorted.dedup(); let axis = detect_axis(&sorted, sys).ok()?; let last = axis.position(*sorted.last()?)?.round(); diff --git a/crates/functions/src/lib.rs b/crates/functions/src/lib.rs index c769ca3ed..ccc61b026 100644 --- a/crates/functions/src/lib.rs +++ b/crates/functions/src/lib.rs @@ -201,7 +201,7 @@ impl FnSpec { pub(crate) const VAR: usize = usize::MAX; fn registry() -> &'static (Vec, HashMap<&'static str, usize>) { - static REG: OnceLock<(Vec, HashMap<&'static str, usize>)> = OnceLock::new(); + static REG: OnceLock<(Vec, HashMap<&str, usize>)> = OnceLock::new(); REG.get_or_init(|| { let mut v: Vec = Vec::new(); for list in [ diff --git a/crates/functions/src/stat.rs b/crates/functions/src/stat.rs index 861c4768e..24518bea2 100644 --- a/crates/functions/src/stat.rs +++ b/crates/functions/src/stat.rs @@ -43,7 +43,7 @@ fn var_p(x: &[f64]) -> R { } fn sorted(mut v: Vec) -> Vec { - v.sort_by(|a, b| a.total_cmp(b)); + v.sort_by(f64::total_cmp); v } diff --git a/crates/functions/src/text.rs b/crates/functions/src/text.rs index 098010f34..df125f022 100644 --- a/crates/functions/src/text.rs +++ b/crates/functions/src/text.rs @@ -523,7 +523,7 @@ fn find_matches(text: &[char], delims: &[Vec], insensitive: bool, from_end let mut j = t.len(); while j > 0 { let best = - ds.iter().filter(|d| j.checked_sub(d.len()).and_then(|st| t.get(st..j)).is_some_and(|w| w == d.as_slice())).map(|d| d.len()).max(); + ds.iter().filter(|d| j.checked_sub(d.len()).and_then(|st| t.get(st..j)).is_some_and(|w| w == d.as_slice())).map(Vec::len).max(); match best { Some(l) => { out.push((j - l, j)); @@ -537,7 +537,7 @@ fn find_matches(text: &[char], delims: &[Vec], insensitive: bool, from_end } let mut i = 0; while i < t.len() { - let best = ds.iter().filter(|d| t.get(i..i + d.len()).is_some_and(|w| w == d.as_slice())).map(|d| d.len()).max(); + let best = ds.iter().filter(|d| t.get(i..i + d.len()).is_some_and(|w| w == d.as_slice())).map(Vec::len).max(); match best { Some(l) => { out.push((i, i + l)); @@ -588,7 +588,7 @@ fn textafter(a: &[Arg], _c: &mut dyn Ctx) -> R { } fn split_by(s: &[char], delims: &[Vec], insensitive: bool) -> Vec> { - if delims.iter().all(|d| d.is_empty()) { + if delims.iter().all(Vec::is_empty) { return vec![s.to_vec()]; } let mut out = Vec::new(); @@ -748,9 +748,9 @@ fn convert_replacement(r: &str) -> String { let mut i = 0; while i < c.len() { let ch = c[i]; - if ch == '$' && c.get(i + 1).is_some_and(|d| d.is_ascii_digit()) { + if ch == '$' && c.get(i + 1).is_some_and(char::is_ascii_digit) { let mut j = i + 1; - while c.get(j).is_some_and(|d| d.is_ascii_digit()) { + while c.get(j).is_some_and(char::is_ascii_digit) { j += 1; } out.push_str("${"); diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index f7ef8f3be..7db7cfb15 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -7,7 +7,10 @@ use gridcraft_core::{CellRef, RangeRef, Value}; use serde::{Deserialize, Serialize}; use crate::cell::{Cell, CellPicture}; -use crate::features::*; +use crate::features::{ + AutoFilter, Chart, Comment, CondFormat, Hyperlink, Image, LineInfo, PivotTable, PrintSettings, Shape, SheetProtection, Slicer, Sparkline, Table, + Validation, +}; use crate::store::CellStore; use crate::style::{Color, StyleId}; @@ -613,8 +616,11 @@ mod tests { // CellRef is a struct, so encode the sparse map as entries for JSON as well as other formats. mod picture_map { - use super::*; + use crate::cell::CellPicture; + use gridcraft_core::CellRef; use serde::{Deserializer, Serializer}; + use std::collections::BTreeMap; + use std::sync::Arc; pub fn serialize(map: &BTreeMap>, serializer: S) -> Result { map.iter().collect::>().serialize(serializer) } diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index d9f78314e..a610954c2 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -525,7 +525,7 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt let s = if k + 1 == count { dc.get(k..).map(|r| r.iter().collect::()).unwrap_or_default() } else { - dc.get(k).map(|c| c.to_string()).unwrap_or_default() + dc.get(k).map(ToString::to_string).unwrap_or_default() }; let s = if s.is_empty() { match toks.get(ti) { diff --git a/crates/pdf/src/metrics.rs b/crates/pdf/src/metrics.rs index 1cd000d81..09fbbfbc2 100644 --- a/crates/pdf/src/metrics.rs +++ b/crates/pdf/src/metrics.rs @@ -99,8 +99,8 @@ pub(crate) fn fold(c: char) -> Option { 'ž' => 'z', 'Œ' => 'W', 'œ' => 'm', - '‘' | '’' | '‚' => '\'', - '“' | '”' | '„' => '"', + '‘'..='‚' => '\'', + '“'..='„' => '"', '–' | '€' | '£' | '¥' | '¢' | '¤' => '0', '\u{a0}' => ' ', '·' | '•' => '.', diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index f5bdc20ab..7c032d72e 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -352,8 +352,8 @@ fn new_tiles(app: &mut SheetApp, ui: &mut Ui, out: &mut Outcome) { /// File name and folder of a recent path, split as text. fn split_path(path: &str) -> (String, String) { let p = std::path::Path::new(path); - let name = p.file_name().and_then(|n| n.to_str()).unwrap_or(path).to_string(); - let folder = p.parent().and_then(|f| f.to_str()).unwrap_or("").to_string(); + let name = p.file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(path).to_string(); + let folder = p.parent().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_string(); (name, folder) } @@ -888,7 +888,7 @@ pub fn export_as(app: &mut SheetApp, fmt: &str) -> bool { _ => return false, }; let title = app.session.active().map(|d| d.display_title()).unwrap_or_else(|| "Book1".into()); - let stem = std::path::Path::new(&title).file_stem().and_then(|s| s.to_str()).unwrap_or("Book1").to_string(); + let stem = std::path::Path::new(&title).file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Book1").to_string(); if let Some(pick) = &app.services.pick_save { let Some(path) = pick(&format!("{stem}.{fmt}")) else { return false }; // The picker may suggest a workbook extension; the export writes the chosen format. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index e83db2fa1..428f34966 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -343,7 +343,7 @@ impl Dialog { // The file rides along in the values (`path` on the desktop, `name` and `base64` // on the web) and goes back to `file.open` with the password. let source = p("path").as_str().or(p("name").as_str()).map(str::to_string).unwrap_or_default(); - let file = std::path::Path::new(&source).file_name().and_then(|n| n.to_str()).unwrap_or("This workbook").to_string(); + let file = std::path::Path::new(&source).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("This workbook").to_string(); // An empty password from the start, so OK on the untouched field says "incorrect" // rather than asking for the password again. let mut values = params.clone(); @@ -1336,7 +1336,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { && let Some(path) = pick() { d.values.remove("base64"); - let alt = std::path::Path::new(&path).file_name().and_then(|name| name.to_str()).unwrap_or(""); + let alt = std::path::Path::new(&path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(""); d.values.insert("alt".into(), json!(alt)); d.values.insert("path".into(), json!(path)); } diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs index 1a381b9d4..699806eba 100644 --- a/crates/ui-egui/src/fonts.rs +++ b/crates/ui-egui/src/fonts.rs @@ -78,7 +78,7 @@ pub fn system_font_dirs() -> Vec { /// Weight implied by a style name (`"Bold"` → 700, `"Light"` → 300). #[cfg(not(target_arch = "wasm32"))] fn style_weight(style: &str) -> f32 { - let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(|c| c.to_lowercase()).collect(); + let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(char::to_lowercase).collect(); const TABLE: &[(&str, f32)] = &[ ("hairline", 100.0), ("thin", 100.0), @@ -107,7 +107,7 @@ impl Face { /// Whether a style name reads as italic or oblique. #[cfg(not(target_arch = "wasm32"))] fn style_italic(style: &str) -> bool { - let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(|c| c.to_lowercase()).collect(); + let s: String = style.chars().filter(|c| c.is_alphanumeric()).flat_map(char::to_lowercase).collect(); s.contains("italic") || s.contains("oblique") } @@ -165,8 +165,10 @@ pub fn scan_dirs(dirs: &[PathBuf]) -> Vec { stack.push(path); continue; } - let is_font = - path.extension().and_then(|e| e.to_str()).is_some_and(|e| matches!(e.to_ascii_lowercase().as_str(), "ttf" | "otf" | "ttc" | "otc")); + let is_font = path + .extension() + .and_then(std::ffi::OsStr::to_str) + .is_some_and(|e| matches!(e.to_ascii_lowercase().as_str(), "ttf" | "otf" | "ttc" | "otc")); if !is_font || std::fs::metadata(&path).map(|m| m.len() > MAX_FONT_FILE).unwrap_or(true) { continue; } @@ -366,12 +368,12 @@ fn bound() -> &'static std::sync::RwLock> { /// Record the role families a freshly built [`egui::FontDefinitions`] bound. pub fn set_bound(keys: impl IntoIterator) { - *bound().write().unwrap_or_else(|e| e.into_inner()) = keys.into_iter().collect(); + *bound().write().unwrap_or_else(std::sync::PoisonError::into_inner) = keys.into_iter().collect(); } /// Is this exact role family bound to font data? pub fn is_bound(role: &str) -> bool { - bound().read().unwrap_or_else(|e| e.into_inner()).contains(role) + bound().read().unwrap_or_else(std::sync::PoisonError::into_inner).contains(role) } /// Serializes tests that mutate the process-wide bound-family registry. diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index b485104b1..1afa00e11 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2184,9 +2184,8 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & } } return; - } else { - app.selected_chart = None; } + app.selected_chart = None; } // A right-click on a chart, picture, shape or slicer opens the object's menu, not the cell's // under it; a right press that wobbles into a drag counts too (#2067). diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 14c442331..bf2425ee8 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -566,7 +566,7 @@ impl SheetApp { return; } let title = self.session.active().map(|d| d.display_title()).unwrap_or_else(|| "Book1".into()); - let stem = std::path::Path::new(&title).file_stem().and_then(|s| s.to_str()).unwrap_or("Book1").to_string(); + let stem = std::path::Path::new(&title).file_stem().and_then(std::ffi::OsStr::to_str).unwrap_or("Book1").to_string(); let Some(pick) = &self.services.pick_save else { return }; let Some(path) = pick(&format!("{stem}.pdf")) else { return }; // The picker may suggest a workbook extension; the export writes a PDF. diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index d454f0380..ed612954f 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -261,7 +261,7 @@ fn try_load(paths: &[(&str, u32)]) -> Option { /// looked for again next time. #[cfg(not(target_arch = "wasm32"))] fn font_file(path: &str, index: u32) -> Option<&'static [u8]> { - static FILES: std::sync::Mutex> = std::sync::Mutex::new(std::collections::BTreeMap::new()); + static FILES: std::sync::Mutex> = std::sync::Mutex::new(std::collections::BTreeMap::new()); let mut files = FILES.lock().unwrap_or_else(std::sync::PoisonError::into_inner); if let Some(&bytes) = files.get(path) { return font_parses(bytes, index).then_some(bytes); @@ -630,12 +630,13 @@ fn cjk_han_japanese() -> Vec<(String, u32)> { fn cjk_han_simplified() -> Vec<(String, u32)> { let mac = "/System/Library/Fonts"; let win = std::env::var("WINDIR").map(|w| format!("{w}\\Fonts")).unwrap_or_else(|_| "C:\\Windows\\Fonts".into()); - let mut v: Vec<(String, u32)> = Vec::new(); - v.push((format!("{win}\\msyh.ttc"), 0)); // Microsoft YaHei - v.push((format!("{win}\\simsun.ttc"), 0)); // SimSun - v.push((format!("{win}\\simhei.ttf"), 0)); // SimHei - v.push((format!("{mac}/Hiragino Sans GB.ttc"), 0)); // ships on every recent macOS - v.push((format!("{mac}/PingFang.ttc"), 0)); // older macOS only; now a downloadable asset + let mut v: Vec<(String, u32)> = vec![ + (format!("{win}\\msyh.ttc"), 0), // Microsoft YaHei + (format!("{win}\\simsun.ttc"), 0), // SimSun + (format!("{win}\\simhei.ttf"), 0), // SimHei + (format!("{mac}/Hiragino Sans GB.ttc"), 0), // ships on every recent macOS + (format!("{mac}/PingFang.ttc"), 0), // older macOS only; now a downloadable asset + ]; for d in LINUX_CJK_DIRS { v.push((format!("{d}/NotoSansCJKsc-Regular.otf"), 0)); v.push((format!("{d}/NotoSansCJK-Regular.ttc"), 2)); // SC face @@ -678,12 +679,13 @@ fn thai() -> Vec<(String, u32)> { "/usr/share/fonts", "/usr/share/fonts/chromeos/noto", // ChromeOS's Linux container (#1762) ]; - let mut v: Vec<(String, u32)> = Vec::new(); - v.push((format!("{win}\\LeelawUI.ttf"), 0)); // Leelawadee UI - v.push((format!("{win}\\tahoma.ttf"), 0)); // Tahoma - v.push((format!("{win}\\micross.ttf"), 0)); // Microsoft Sans Serif - v.push((format!("{mac_sup}/Thonburi.ttc"), 0)); - v.push((format!("{mac_sys}/SukhumvitSet.ttc"), 0)); + let mut v: Vec<(String, u32)> = vec![ + (format!("{win}\\LeelawUI.ttf"), 0), // Leelawadee UI + (format!("{win}\\tahoma.ttf"), 0), // Tahoma + (format!("{win}\\micross.ttf"), 0), // Microsoft Sans Serif + (format!("{mac_sup}/Thonburi.ttc"), 0), + (format!("{mac_sys}/SukhumvitSet.ttc"), 0), + ]; for d in lin { v.push((format!("{d}/NotoSansThai-Regular.ttf"), 0)); v.push((format!("{d}/tlwg/Garuda.ttf"), 0)); @@ -701,11 +703,12 @@ fn lao() -> Vec<(String, u32)> { let mac_sup = "/System/Library/Fonts/Supplemental"; let win = std::env::var("WINDIR").map(|w| format!("{w}\\Fonts")).unwrap_or_else(|_| "C:\\Windows\\Fonts".into()); let lin = ["/usr/share/fonts/truetype/noto", "/usr/share/fonts/opentype/noto", "/usr/share/fonts/truetype", "/usr/share/fonts"]; - let mut v: Vec<(String, u32)> = Vec::new(); - v.push((format!("{win}\\LeelawUI.ttf"), 0)); // Leelawadee UI - v.push((format!("{win}\\dokchamp.ttf"), 0)); // DokChampa - v.push((format!("{mac_sup}/Lao Sangam MN.ttf"), 0)); - v.push((format!("{mac_sup}/Lao MN.ttc"), 0)); + let mut v: Vec<(String, u32)> = vec![ + (format!("{win}\\LeelawUI.ttf"), 0), // Leelawadee UI + (format!("{win}\\dokchamp.ttf"), 0), // DokChampa + (format!("{mac_sup}/Lao Sangam MN.ttf"), 0), + (format!("{mac_sup}/Lao MN.ttc"), 0), + ]; for d in lin { v.push((format!("{d}/NotoSansLao-Regular.ttf"), 0)); v.push((format!("{d}/lao/Phetsarath_OT.ttf"), 0)); @@ -728,16 +731,17 @@ fn hebrew() -> Vec<(String, u32)> { "/usr/share/fonts", "/usr/share/fonts/chromeos/noto", // ChromeOS's Linux container (#1762) ]; - let mut v: Vec<(String, u32)> = Vec::new(); - v.push((format!("{win}\\david.ttf"), 0)); // David - v.push((format!("{win}\\frank.ttf"), 0)); // FrankRuehl - v.push((format!("{win}\\tahoma.ttf"), 0)); // Tahoma - v.push((format!("{mac_sys}/ArialHB.ttc"), 0)); // Arial Hebrew - v.push((format!("{mac_sys}/SFHebrew.ttf"), 0)); - v.push((format!("{mac_sup}/Arial Hebrew.ttf"), 0)); - v.push((format!("{mac_sup}/David.ttf"), 0)); - v.push((format!("{mac_sys}/LucidaGrande.ttc"), 0)); - v.push((format!("{mac_sys}/Lucida Grande.ttc"), 0)); + let mut v: Vec<(String, u32)> = vec![ + (format!("{win}\\david.ttf"), 0), // David + (format!("{win}\\frank.ttf"), 0), // FrankRuehl + (format!("{win}\\tahoma.ttf"), 0), // Tahoma + (format!("{mac_sys}/ArialHB.ttc"), 0), // Arial Hebrew + (format!("{mac_sys}/SFHebrew.ttf"), 0), + (format!("{mac_sup}/Arial Hebrew.ttf"), 0), + (format!("{mac_sup}/David.ttf"), 0), + (format!("{mac_sys}/LucidaGrande.ttc"), 0), + (format!("{mac_sys}/Lucida Grande.ttc"), 0), + ]; for d in lin { v.push((format!("{d}/NotoSansHebrew-Regular.ttf"), 0)); v.push((format!("{d}/dejavu/DejaVuSans.ttf"), 0)); diff --git a/crates/xlsx/src/drawing/ink.rs b/crates/xlsx/src/drawing/ink.rs index b48d99ee8..b5d21f044 100644 --- a/crates/xlsx/src/drawing/ink.rs +++ b/crates/xlsx/src/drawing/ink.rs @@ -1,7 +1,13 @@ //! The open, solid polylines the model can represent as editable ink. This is DrawingML //! geometry, not Office InkML. Unsupported geometry keeps the existing shape-import fallback. -use super::*; +use std::fmt::Write as _; + +use gridcraft_model::{Anchor, Color, Shape, ShapeKind, Theme}; + +use super::xfrm; +use crate::tables::EMU_PER_PX; +use crate::xml::El; const MAX_POINTS: usize = 20_000; const MAX_COORD: f64 = 1e7; @@ -24,9 +30,7 @@ fn one<'a>(e: &'a El, name: &str) -> Option<&'a El> { found.next().is_none().then_some(first) } -/// Do not partially import a path: losing a curve, subpath, transform or line effect changes -/// the drawing. Accept only the explicit subset emitted below, including its line styling. -pub(super) fn read(obj: &El, anchor: Anchor) -> Option { +fn validate_shape_obj(obj: &El) -> Option<&El> { if obj.name != "sp" || !only_children(obj, &["nvSpPr", "spPr"]) || !only_attrs(obj, &["macro", "textlink"]) @@ -43,6 +47,10 @@ pub(super) fn read(obj: &El, anchor: Anchor) -> Option { if !no_fill.attrs.is_empty() || !no_fill.children.is_empty() { return None; } + Some(sp) +} + +fn validate_xfrm(sp: &El, anchor: &Anchor) -> Option<()> { let xf = one(sp, "xfrm")?; if !xf.attrs.is_empty() || !only_children(xf, &["off", "ext"]) { return None; @@ -62,6 +70,10 @@ pub(super) fn read(obj: &El, anchor: Anchor) -> Option { if !same_extent(ew, anchor.width) || !same_extent(eh, anchor.height) { return None; } + Some(()) +} + +fn validate_cust_geom(sp: &El) -> Option<(&El, i64, i64)> { let geom = one(sp, "custGeom")?; if !geom.attrs.is_empty() || !only_children(geom, &["avLst", "gdLst", "ahLst", "cxnLst", "rect", "pathLst"]) { return None; @@ -90,6 +102,10 @@ pub(super) fn read(obj: &El, anchor: Anchor) -> Option { { return None; } + Some((path, w, h)) +} + +fn validate_line(sp: &El) -> Option<(i64, Color)> { let ln = one(sp, "ln")?; if !only_attrs(ln, &["w", "cap", "cmpd", "algn"]) || ln.attr("cap") != Some("flat") @@ -129,6 +145,10 @@ pub(super) fn read(obj: &El, anchor: Anchor) -> Option { } } let line = Color::from_hex(color.attr("val")?)?; + Some((width, line)) +} + +fn parse_points(path: &El, anchor: &Anchor, w: i64, h: i64) -> Option { let mut text = String::new(); for (i, command) in path.children.iter().enumerate() { if command.name != if i == 0 { "moveTo" } else { "lnTo" } || !command.attrs.is_empty() || command.children.len() != 1 { @@ -148,6 +168,17 @@ pub(super) fn read(obj: &El, anchor: Anchor) -> Option { } let _ = write!(text, "{},{}", x as f32, y as f32); } + Some(text) +} + +/// Do not partially import a path: losing a curve, subpath, transform or line effect changes +/// the drawing. Accept only the explicit subset emitted below, including its line styling. +pub(super) fn read(obj: &El, anchor: Anchor) -> Option { + let sp = validate_shape_obj(obj)?; + validate_xfrm(sp, &anchor)?; + let (path, w, h) = validate_cust_geom(sp)?; + let (width, line) = validate_line(sp)?; + let text = parse_points(path, &anchor, w, h)?; Some(Shape { id: 0, kind: ShapeKind::Ink, anchor, fill: Color::Rgb((width as u32) << 16), line, text, format: Default::default() }) } diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 4274f1196..650b22820 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -110,7 +110,7 @@ pub enum XlsxKind { impl XlsxKind { /// The kind a file name's extension asks for; anything else is a plain workbook. pub fn from_path(path: &str) -> XlsxKind { - let ext = std::path::Path::new(path).extension().and_then(|e| e.to_str()).unwrap_or_default().to_ascii_lowercase(); + let ext = std::path::Path::new(path).extension().and_then(std::ffi::OsStr::to_str).unwrap_or_default().to_ascii_lowercase(); match ext.as_str() { "xlsm" => XlsxKind::MacroWorkbook, "xltx" => XlsxKind::Template, diff --git a/crates/xlsx/src/pivot.rs b/crates/xlsx/src/pivot.rs index f75523432..ea3bc3729 100644 --- a/crates/xlsx/src/pivot.rs +++ b/crates/xlsx/src/pivot.rs @@ -647,7 +647,7 @@ fn group_items(sys: DateSystem, by: PivotDateGroup, start: f64, end: f64) -> Vec } } PivotDateGroup::Quarters => v.extend((1..=4).map(|q| format!("Qtr{q}"))), - PivotDateGroup::Months => v.extend(MONTHS.iter().map(|m| m.to_string())), + PivotDateGroup::Months => v.extend(MONTHS.iter().map(ToString::to_string)), PivotDateGroup::Days | PivotDateGroup::None => { const DAYS: [u32; 12] = [31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; for (m, n) in MONTHS.iter().zip(DAYS) { diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index ec8cd9e75..b179fcc08 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -360,7 +360,7 @@ pub fn encode_xstring(s: &str) -> String { } fn looks_like_escape(chars: &[char], i: usize) -> bool { - chars.get(i + 1) == Some(&'x') && chars.get(i + 6) == Some(&'_') && (i + 2..i + 6).all(|k| chars.get(k).is_some_and(|c| c.is_ascii_hexdigit())) + chars.get(i + 1) == Some(&'x') && chars.get(i + 6) == Some(&'_') && (i + 2..i + 6).all(|k| chars.get(k).is_some_and(char::is_ascii_hexdigit)) } fn escape_unit(chars: &[char], i: usize) -> Option { diff --git a/xtask/src/assets.rs b/xtask/src/assets.rs index c9bbcd1ad..16fc2b8b8 100644 --- a/xtask/src/assets.rs +++ b/xtask/src/assets.rs @@ -20,7 +20,7 @@ const EXAMPLE_EXT: &[&str] = &["csv", "tsv", "json"]; const ASSET_DIRS: &[&str] = &["assets/", "docs/brand/", "docs/images/"]; fn ext(path: &str) -> String { - Path::new(path).extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase).unwrap_or_default() + Path::new(path).extension().and_then(std::ffi::OsStr::to_str).map(str::to_ascii_lowercase).unwrap_or_default() } pub fn is_asset(path: &str) -> bool { diff --git a/xtask/src/i18n_report.rs b/xtask/src/i18n_report.rs index d72a887fa..344ed7569 100644 --- a/xtask/src/i18n_report.rs +++ b/xtask/src/i18n_report.rs @@ -27,8 +27,8 @@ pub fn run(root: &Path) -> Result<(), String> { }; for entry in entries.flatten() { let path = entry.path(); - let Some(stem) = path.file_stem().and_then(|s| s.to_str()).map(str::to_owned) else { continue }; - if path.extension().and_then(|e| e.to_str()) != Some("rs") { + let Some(stem) = path.file_stem().and_then(std::ffi::OsStr::to_str).map(str::to_owned) else { continue }; + if path.extension().and_then(std::ffi::OsStr::to_str) != Some("rs") { continue; } let Ok(source) = std::fs::read_to_string(&path) else { continue }; diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 0d1990730..cd3229e8b 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -174,7 +174,7 @@ pub fn check(crates: &[Crate]) -> Vec { } } } - out.sort_by_key(|v| v.to_string()); + out.sort_by_key(ToString::to_string); out.dedup(); out }