From 63394e872ddfc72856c1ecf2d67790cd78eb6f92 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 04:34:49 -0400 Subject: [PATCH] chore: resolve SonarCloud maintainability issues (#2025) * chore: resolve SonarCloud maintainability issues * style: format with rustfmt * fix(engine): reference DocState instead of DrawingState * fix: restore cmd wildcard imports and remove unused Arc import * Unit tests for the extracted mleader alignment steps and spacemouse filter * test: fix assertions flagged in review and apply rustfmt Co-Authored-By: Claude Sonnet 5.5 * test: initialise drift filter in struct literal for clippy Co-Authored-By: Claude Sonnet 5.5 --------- Co-authored-by: Dylan Taylor Co-authored-by: Claude Sonnet 5.5 --- apps/cadcraft/src/spacemouse/motion.rs | 105 ++++++++++++++-- crates/constraints/src/expr.rs | 2 +- crates/doc/src/constraint.rs | 2 +- crates/doc/src/tables.rs | 2 +- crates/doc/src/tables/cellstyle.rs | 8 +- crates/engine/src/autosave.rs | 2 +- crates/engine/src/cmd/mleader_edit.rs | 116 ++++++++++-------- crates/engine/src/cmd/mleader_edit_tests.rs | 85 +++++++++++++ crates/engine/src/cmd/utility/calc.rs | 2 +- crates/engine/src/guard.rs | 2 +- crates/engine/src/lib.rs | 11 +- crates/engine/src/lisp/builtins.rs | 14 ++- crates/engine/src/lisp/mod.rs | 4 +- crates/engine/src/prompt.rs | 2 +- crates/io/src/dxf_ext.rs | 2 +- crates/io/src/svg_read.rs | 6 +- crates/mcp/tests/it/cmdline_key.rs | 2 +- crates/mcp/tests/it/cmdline_parity.rs | 2 +- crates/render/src/dim.rs | 2 +- crates/ui-egui/src/mtext_editor.rs | 2 +- crates/ui-egui/src/quick.rs | 2 +- crates/ui-egui/tests/it/chrome_geometry.rs | 48 ++++---- crates/ui-egui/tests/it/control_cmdline.rs | 2 +- .../ui-egui/tests/it/control_cmdline_key.rs | 2 +- xtask/src/assets.rs | 2 +- xtask/src/layers.rs | 2 +- 26 files changed, 310 insertions(+), 121 deletions(-) diff --git a/apps/cadcraft/src/spacemouse/motion.rs b/apps/cadcraft/src/spacemouse/motion.rs index 1ef6d195..24f4bf1a 100644 --- a/apps/cadcraft/src/spacemouse/motion.rs +++ b/apps/cadcraft/src/spacemouse/motion.rs @@ -107,6 +107,23 @@ impl Filter { *self = Self::default(); } + fn update_drift(&mut self, complete: bool, now: Instant, bias: &mut [f64; 6]) { + if !complete { + return; + } + if self.raw.iter().zip(*bias).all(|(raw, b)| (*raw - b).abs() <= FULL_SCALE * DEADZONE) { + let since = *self.idle_since.get_or_insert(now); + if now.saturating_duration_since(since) >= Duration::from_secs(2) { + for (b, raw) in bias.iter_mut().zip(self.raw) { + *b += (raw - *b) * 0.01; + } + self.bias = Some(*bias); + } + } else { + self.idle_since = None; + } + } + pub fn update(&mut self, values: [i16; 6], mask: u8, now: Instant) -> Motion { for (index, seen) in self.seen.iter_mut().enumerate() { if mask & (1 << index) != 0 { @@ -128,19 +145,7 @@ impl Filter { } let Some(mut bias) = self.bias else { return Motion::default() }; // Follow small resting drift only after two seconds entirely inside the deadzone. - if complete { - if self.raw.iter().zip(bias).all(|(raw, bias)| (*raw - bias).abs() <= FULL_SCALE * DEADZONE) { - let since = *self.idle_since.get_or_insert(now); - if now.saturating_duration_since(since) >= Duration::from_secs(2) { - for (bias, raw) in bias.iter_mut().zip(self.raw) { - *bias += (raw - *bias) * 0.01; - } - self.bias = Some(bias); - } - } else { - self.idle_since = None; - } - } + self.update_drift(complete, now, &mut bias); let mut axes = [0.0; 6]; for ((axis, raw), bias) in axes.iter_mut().zip(self.raw).zip(bias) { let value = ((raw - bias) / FULL_SCALE).clamp(-1.0, 1.0); @@ -156,3 +161,77 @@ impl Filter { Motion { pan: [tx, ty], zoom: (tz + rz).clamp(-1.0, 1.0) } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn decode_reports() { + assert_eq!(decode(&[]), None); + assert_eq!(decode(&[3]), None); + assert_eq!(decode(&[3, 5]), Some(Report::Buttons(5))); + let values = [1_i16, -2, 3, 4, -5, 6]; + let mut bytes = vec![1_u8]; + for v in values { + bytes.extend_from_slice(&v.to_le_bytes()); + } + assert_eq!(decode(&bytes), Some(Report::Axes { values, mask: 3 })); + bytes[0] = 2; + assert_eq!(decode(&bytes), Some(Report::Axes { values: [0, 0, 0, 1, -2, 3], mask: 2 })); + assert_eq!(decode(&[2, 1, 2, 3]), None, "too short for a rotation report"); + } + + /// Sixteen complete reports at rest, spread over more than CALIBRATION_TIME, all stamped + /// on a made-up point in time big enough that they sit in the past of later updates. + fn calibrated(t0: Instant) -> Filter { + let mut f = Filter::default(); + for i in 0..17 { + let at = t0 + Duration::from_millis(50 * i); + assert_eq!(f.update([0; 6], 1, at), Motion::default()); + assert_eq!(f.update([0; 6], 2, at), Motion::default()); + } + assert!(f.calibrated()); + f + } + + #[test] + fn axes_deflect_and_expire() { + let t0 = Instant::now(); + let mut f = calibrated(t0); + let at = t0 + Duration::from_secs(2); + let ms = Duration::from_millis; + let m = f.update([500, 500, 0, 0, 0, 0], 1, at); + assert!((m.pan[0] - 1.0).abs() < 1e-9); + assert!((m.pan[1] - 1.0).abs() < 1e-9); + assert_eq!(m.zoom, 0.0, "no rotation input"); + // A fresh rotation report drives the zoom. + let m = f.update([0, 0, 0, 0, 0, 500], 2, at + ms(100)); + assert!((m.zoom - 1.0).abs() < 1e-9); + // The rotation axes went quiet for over 250 ms while translations still arrive. + let m = f.update([500, 500, 0, 0, 0, 0], 1, at + ms(400)); + assert_eq!(m.zoom, 0.0); + // A fresh rotation report puts the zoom back. + let m = f.update([500, 500, 0, 0, 0, 500], 2, at + ms(500)); + assert!((m.zoom - 1.0).abs() < 1e-9); + } + + #[test] + fn drift_tracks_rest_after_two_idle_seconds() { + // Five axes units of drift sit inside the deadzone (FULL_SCALE * DEADZONE = 20). + let mut f = Filter { raw: [5.0; 6], bias: Some([0.0; 6]), ..Filter::default() }; + let t0 = Instant::now(); + let mut b = [0.0; 6]; + f.update_drift(true, t0, &mut b); // starts the idle window: no shift yet + assert_eq!(b, [0.0; 6]); + f.update_drift(true, t0 + Duration::from_secs(3), &mut b); + assert!((b[0] - 0.05).abs() < 1e-12, "bias follows resting drift"); + assert_eq!(f.bias, Some(b)); + // Motion outside the deadzone resets the idle window. + f.raw = [100.0; 6]; + f.update_drift(true, t0 + Duration::from_secs(4), &mut b); + assert_eq!(f.idle_since, None); + f.update_drift(true, t0 + Duration::from_secs(5), &mut b); + assert!((b[0] - 0.05).abs() < 1e-12, "no shift before two seconds at rest"); + } +} diff --git a/crates/constraints/src/expr.rs b/crates/constraints/src/expr.rs index 4bc25a55..c71ed348 100644 --- a/crates/constraints/src/expr.rs +++ b/crates/constraints/src/expr.rs @@ -49,7 +49,7 @@ struct Parser<'a> { impl Parser<'_> { fn ws(&mut self) { - while self.s.get(self.i).is_some_and(|c| c.is_ascii_whitespace()) { + while self.s.get(self.i).is_some_and(u8::is_ascii_whitespace) { self.i += 1; } } diff --git a/crates/doc/src/constraint.rs b/crates/doc/src/constraint.rs index 05e56552..10f42301 100644 --- a/crates/doc/src/constraint.rs +++ b/crates/doc/src/constraint.rs @@ -208,7 +208,7 @@ impl Default for ParametricSettings { angle_tolerance: 1.0, auto_types: ["Coincident", "Collinear", "Parallel", "Perpendicular", "Horizontal", "Vertical", "Tangent", "Concentric", "Equal"] .iter() - .map(|s| s.to_string()) + .map(ToString::to_string) .collect(), bars_visible: true, bar_exceptions: Vec::new(), diff --git a/crates/doc/src/tables.rs b/crates/doc/src/tables.rs index 45e7a469..69badebb 100644 --- a/crates/doc/src/tables.rs +++ b/crates/doc/src/tables.rs @@ -857,7 +857,7 @@ pub struct VportConfig { impl VportConfig { /// The name of the configuration shown. - pub const ACTIVE: &'static str = "*Active"; + pub const ACTIVE: &str = "*Active"; pub fn is_active(&self) -> bool { self.name.eq_ignore_ascii_case(Self::ACTIVE) } diff --git a/crates/doc/src/tables/cellstyle.rs b/crates/doc/src/tables/cellstyle.rs index b2907f69..737fe7eb 100644 --- a/crates/doc/src/tables/cellstyle.rs +++ b/crates/doc/src/tables/cellstyle.rs @@ -109,9 +109,9 @@ impl Default for CellStyle { } impl CellStyle { - pub const TITLE: &'static str = "Title"; - pub const HEADER: &'static str = "Header"; - pub const DATA: &'static str = "Data"; + pub const TITLE: &str = "Title"; + pub const HEADER: &str = "Header"; + pub const DATA: &str = "Data"; /// Whether `name` is one of the three styles every table style has. pub fn is_builtin(name: &str) -> bool { @@ -190,7 +190,7 @@ impl TableStyle { /// The names of its cell styles: Title, Header, Data, then the custom ones. pub fn cell_style_names(&self) -> Vec { - let mut out: Vec = [CellStyle::TITLE, CellStyle::HEADER, CellStyle::DATA].iter().map(|s| s.to_string()).collect(); + let mut out: Vec = [CellStyle::TITLE, CellStyle::HEADER, CellStyle::DATA].iter().map(ToString::to_string).collect(); out.extend(self.cell_styles.iter().filter(|c| !CellStyle::is_builtin(&c.name)).map(|c| c.name.clone())); out } diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index e1a30f08..0904711e 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -302,7 +302,7 @@ mod native { /// files of stopped apps that have no files left. Only an autosave file is deleted: a path /// whose name doesn't parse as one ([`parse_file_name`]) is left alone. pub fn discard(path: &str) { - let ours = Path::new(path).file_name().and_then(|n| n.to_str()).is_some_and(|n| parse_file_name(n).is_some()); + let ours = Path::new(path).file_name().and_then(std::ffi::OsStr::to_str).is_some_and(|n| parse_file_name(n).is_some()); if !ours { log::warn!("autosave: not deleting {path}: not an autosave file"); return; diff --git a/crates/engine/src/cmd/mleader_edit.rs b/crates/engine/src/cmd/mleader_edit.rs index ee613460..1416d71a 100644 --- a/crates/engine/src/cmd/mleader_edit.rs +++ b/crates/engine/src/cmd/mleader_edit.rs @@ -310,6 +310,67 @@ fn segment_dir(m: &MLeader) -> Option { (v.len() > 1e-12).then(|| v.normalized()) } +fn align_parallel(items: &[(Handle, MLeader)], reference: Option<&MLeader>) -> Result> { + let rdir = reference.and_then(segment_dir).ok_or_else(|| bad("mleaderalign", "the reference multileader has no leader"))?; + let mut moves = Vec::new(); + for (h, m) in items { + let Some(arrow) = m.leaders.first().and_then(|l| l.last()) else { continue }; + let len = (m.landing.xy() - arrow.xy()).len(); + moves.push((*h, arrow.xy() + rdir * len - m.landing.xy())); + } + Ok(moves) +} + +fn align_distribute(mut items: Vec<(Handle, MLeader)>, line: Option<(Vec2, Vec2)>) -> Result> { + let (a, b) = line.ok_or_else(|| bad("mleaderalign", "`points` (first and last position) are required"))?; + let d = b - a; + if d.len() < 1e-12 { + return Err(bad("mleaderalign", "the two points coincide")); + } + let u = d.normalized(); + items.sort_by(|x, y| (x.1.landing.xy() - a).dot(u).total_cmp(&(y.1.landing.xy() - a).dot(u))); + let n = items.len(); + let mut moves = Vec::new(); + for (i, (h, m)) in items.iter().enumerate() { + let f = if n > 1 { i as f64 / (n - 1) as f64 } else { 0.0 }; + moves.push((*h, a + d * f - m.landing.xy())); + } + Ok(moves) +} + +fn align_directional( + items: &[(Handle, MLeader)], + reference: Option<&MLeader>, + line: Option<(Vec2, Vec2)>, + spacing: Spacing, +) -> Result> { + let (a, b) = line.ok_or_else(|| bad("mleaderalign", "`direction` (two points or an angle) is required"))?; + let d = b - a; + if d.len() < 1e-12 { + return Err(bad("mleaderalign", "the direction is zero")); + } + let u = d.normalized(); + let origin = reference.map(|r| r.landing.xy()).unwrap_or(a); + let t_of = |m: &MLeader| (m.landing.xy() - origin).dot(u); + let mut moves = Vec::new(); + if let Spacing::Value(sp) = spacing { + let (mut pos, mut neg): (Vec<_>, Vec<_>) = items.iter().partition(|(_, m)| t_of(m) >= 0.0); + pos.sort_by(|x, y| t_of(&x.1).total_cmp(&t_of(&y.1))); + neg.sort_by(|x, y| t_of(&y.1).total_cmp(&t_of(&x.1))); + for (k, (h, m)) in pos.iter().enumerate() { + moves.push((*h, origin + u * (sp * (k + 1) as f64) - m.landing.xy())); + } + for (k, (h, m)) in neg.iter().enumerate() { + moves.push((*h, origin - u * (sp * (k + 1) as f64) - m.landing.xy())); + } + } else { + for (h, m) in items { + moves.push((*h, origin + u * t_of(m) - m.landing.xy())); + } + } + Ok(moves) +} + fn align(s: &mut Session, hs: &[Handle], to: Option, spacing: Spacing, line: Option<(Vec2, Vec2)>) -> Result { let mut items: Vec<(Handle, MLeader)> = Vec::new(); for h in hs { @@ -331,56 +392,11 @@ fn align(s: &mut Session, hs: &[Handle], to: Option, spacing: Spacing, l (_, Some(t)) => Some(get_mleader(s, t).ok_or_else(|| bad("mleaderalign", "`to` is not a multileader"))?), (_, None) => return Err(bad("mleaderalign", "`to` (the multileader to align to) is required")), }; - let mut moves: Vec<(Handle, Vec2)> = Vec::new(); - match spacing { - Spacing::Parallel => { - let rdir = reference.as_ref().and_then(segment_dir).ok_or_else(|| bad("mleaderalign", "the reference multileader has no leader"))?; - for (h, m) in &items { - let Some(arrow) = m.leaders.first().and_then(|l| l.last()) else { continue }; - let len = (m.landing.xy() - arrow.xy()).len(); - moves.push((*h, arrow.xy() + rdir * len - m.landing.xy())); - } - } - Spacing::Distribute => { - let (a, b) = line.ok_or_else(|| bad("mleaderalign", "`points` (first and last position) are required"))?; - let d = b - a; - if d.len() < 1e-12 { - return Err(bad("mleaderalign", "the two points coincide")); - } - let u = d.normalized(); - items.sort_by(|x, y| (x.1.landing.xy() - a).dot(u).total_cmp(&(y.1.landing.xy() - a).dot(u))); - let n = items.len(); - for (i, (h, m)) in items.iter().enumerate() { - let f = if n > 1 { i as f64 / (n - 1) as f64 } else { 0.0 }; - moves.push((*h, a + d * f - m.landing.xy())); - } - } - Spacing::Current | Spacing::Value(_) => { - let (a, b) = line.ok_or_else(|| bad("mleaderalign", "`direction` (two points or an angle) is required"))?; - let d = b - a; - if d.len() < 1e-12 { - return Err(bad("mleaderalign", "the direction is zero")); - } - let u = d.normalized(); - let origin = reference.as_ref().map(|r| r.landing.xy()).unwrap_or(a); - let t_of = |m: &MLeader| (m.landing.xy() - origin).dot(u); - if let Spacing::Value(sp) = spacing { - let (mut pos, mut neg): (Vec<_>, Vec<_>) = items.iter().partition(|(_, m)| t_of(m) >= 0.0); - pos.sort_by(|x, y| t_of(&x.1).total_cmp(&t_of(&y.1))); - neg.sort_by(|x, y| t_of(&y.1).total_cmp(&t_of(&x.1))); - for (k, (h, m)) in pos.iter().enumerate() { - moves.push((*h, origin + u * (sp * (k + 1) as f64) - m.landing.xy())); - } - for (k, (h, m)) in neg.iter().enumerate() { - moves.push((*h, origin - u * (sp * (k + 1) as f64) - m.landing.xy())); - } - } else { - for (h, m) in &items { - moves.push((*h, origin + u * t_of(m) - m.landing.xy())); - } - } - } - } + let moves = match spacing { + Spacing::Parallel => align_parallel(&items, reference.as_ref())?, + Spacing::Distribute => align_distribute(items, line)?, + Spacing::Current | Spacing::Value(_) => align_directional(&items, reference.as_ref(), line, spacing)?, + }; move_content(s, &moves)?; Ok(moves.len()) } diff --git a/crates/engine/src/cmd/mleader_edit_tests.rs b/crates/engine/src/cmd/mleader_edit_tests.rs index 304a5ece..78c786d2 100644 --- a/crates/engine/src/cmd/mleader_edit_tests.rs +++ b/crates/engine/src/cmd/mleader_edit_tests.rs @@ -120,3 +120,88 @@ fn hostile_mleader_edit_params_never_panic() { } let _ = cadcraft_render::build(s.doc().unwrap(), &cadcraft_doc::Space::Model, &cadcraft_render::Options::default()); } + +mod units { + use super::super::{Spacing, align_directional, align_distribute, align_parallel}; + use cadcraft_doc::{Handle, MLeader}; + use cadcraft_geom::{Vec2, Vec3}; + + fn unit_ml(arrow: [f64; 2], land: [f64; 2]) -> MLeader { + MLeader { + leaders: vec![vec![Vec3::new(arrow[0], arrow[1], 0.0)]], + landing: Vec3::new(land[0], land[1], 0.0), + dogleg: 0.0, + text: None, + style: String::new(), + arrow_size: 1.0, + spline: false, + block: None, + } + } + + fn no_leader(land: [f64; 2]) -> MLeader { + MLeader { + leaders: vec![], + landing: Vec3::new(land[0], land[1], 0.0), + dogleg: 0.0, + text: None, + style: String::new(), + arrow_size: 1.0, + spline: false, + block: None, + } + } + + /// A five-unit-long leader whose direction differs from the reference's `+y`: moving the + /// arrow onto `y` plus `5` (a five-unit parallel push) and back along the landing gives + /// `(-3, 1)`. + #[test] + fn parallel_requires_a_reference_and_skips_leaders() { + assert!(align_parallel(&[(Handle(1), unit_ml([0.0, 0.0], [3.0, 4.0]))], None).is_err()); + let d = align_parallel( + &[(Handle(2), no_leader([1.0, 1.0])), (Handle(3), unit_ml([0.0, 0.0], [3.0, 4.0]))], + Some(&unit_ml([0.0, 0.0], [0.0, 4.0])), + ) + .unwrap(); + assert_eq!(d.len(), 1, "a multileader without a leader line is skipped"); + assert_eq!(d[0].0, Handle(3)); + assert!((d[0].1.x + 3.0).abs() < 1e-9); + assert!((d[0].1.y - 1.0).abs() < 1e-9); + } + + #[test] + fn distribute_orders_landings_along_the_line() { + let mk = |h: u64, land: [f64; 2]| (Handle(h), unit_ml([0.0, 0.0], land)); + assert!(align_distribute(vec![mk(1, [2.0, 0.0])], None).is_err(), "needs two points"); + assert!(align_distribute(Vec::new(), Some((Vec2::new(0.0, 0.0), Vec2::new(0.0, 0.0)))).is_err(), "coinciding points give no direction"); + let items = vec![mk(1, [2.0, 0.0]), mk(2, [0.0, 0.0])]; + let d = align_distribute(items, Some((Vec2::new(0.0, 0.0), Vec2::new(4.0, 0.0)))).unwrap(); + assert_eq!(d[0].0, Handle(2), "sorted by projection onto the line"); + assert!((d[0].1.x - 0.0).abs() < 1e-9 && (d[0].1.y - 0.0).abs() < 1e-9, "first fraction is 0"); + assert_eq!(d[1].0, Handle(1)); + assert!((d[1].1.x - 2.0).abs() < 1e-9 && (d[1].1.y - 0.0).abs() < 1e-9, "last fraction is 1"); + } + + #[test] + fn directional_projects_landings_and_supports_spacing() { + let line = Some((Vec2::new(0.0, 0.0), Vec2::new(1.0, 0.0))); + assert!(align_directional(&[], None, None, Spacing::Current).is_err(), "needs a direction"); + assert!( + align_directional(&[], None, Some((Vec2::new(0.0, 0.0), Vec2::new(0.0, 0.0))), Spacing::Current).is_err(), + "a zero direction is an error" + ); + // Without a reference the origin is the first point: the landing is projected onto it. + let d = align_directional(&[(Handle(7), unit_ml([0.0, 0.0], [1.0, 5.0]))], None, line, Spacing::Current).unwrap(); + assert_eq!(d.len(), 1); + assert!((d[0].1.x - 0.0).abs() < 1e-9); + assert!((d[0].1.y + 5.0).abs() < 1e-9); + // With a reference as origin, given spacing counts outward on both sides of it. + let items = vec![(Handle(1), unit_ml([0.0, 0.0], [4.0, 0.0])), (Handle(2), unit_ml([0.0, 0.0], [-4.0, 0.0]))]; + let d = align_directional(&items, Some(&unit_ml([0.0, 0.0], [0.0, 4.0])), line, Spacing::Value(2.0)).unwrap(); + assert_eq!(d.len(), 2); + assert_eq!(d[0].0, Handle(1)); + assert!((d[0].1.x + 2.0).abs() < 1e-9 && (d[0].1.y - 4.0).abs() < 1e-9); + assert_eq!(d[1].0, Handle(2)); + assert!((d[1].1.x - 2.0).abs() < 1e-9 && (d[1].1.y - 4.0).abs() < 1e-9); + } +} diff --git a/crates/engine/src/cmd/utility/calc.rs b/crates/engine/src/cmd/utility/calc.rs index 437501aa..05c6fc52 100644 --- a/crates/engine/src/cmd/utility/calc.rs +++ b/crates/engine/src/cmd/utility/calc.rs @@ -73,7 +73,7 @@ struct Parser<'a> { impl Parser<'_> { fn peek(&mut self) -> Option { - while self.s.get(self.i).is_some_and(|c| c.is_ascii_whitespace()) { + while self.s.get(self.i).is_some_and(u8::is_ascii_whitespace) { self.i += 1; } self.s.get(self.i).copied() diff --git a/crates/engine/src/guard.rs b/crates/engine/src/guard.rs index 7d05f1c1..8a64e2b7 100644 --- a/crates/engine/src/guard.rs +++ b/crates/engine/src/guard.rs @@ -23,7 +23,7 @@ pub(crate) struct Guarded { } pub(crate) fn message(p: &(dyn Any + Send)) -> String { - p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) + p.downcast_ref::<&str>().map(ToString::to_string).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) } impl Guarded { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ab9c0285..117db581 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -829,7 +829,7 @@ impl Session { } /// The space commands act on (model space while working inside a layout viewport). pub fn space(&self) -> Space { - self.state().map(|s| s.edit_space()).unwrap_or_default() + self.state().map(DocState::edit_space).unwrap_or_default() } /// The tab being displayed: Model or a layout, regardless of MSPACE. pub fn layout_space(&self) -> Space { @@ -891,7 +891,7 @@ impl Session { /// World units per pixel at the current view. pub fn pixel_size(&self) -> f64 { - let v = self.state().map(|s| s.view()).unwrap_or_default(); + let v = self.state().map(DocState::view).unwrap_or_default(); v.height / self.viewport_px.1.max(1.0) } @@ -911,8 +911,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 drawing as it was. if let Some((doc, sel, uid)) = &before && let Some(st) = self.docs.iter_mut().find(|d| d.uid == *uid) diff --git a/crates/engine/src/lisp/builtins.rs b/crates/engine/src/lisp/builtins.rs index e421686c..0fcb22f0 100644 --- a/crates/engine/src/lisp/builtins.rs +++ b/crates/engine/src/lisp/builtins.rs @@ -5,6 +5,8 @@ //! Errors use AutoLISP's wording (`bad argument type: numberp: nil`, `too few arguments`, //! `divide by zero`), since routines and their `*error*` handlers match on it. +use std::cmp::Ordering; +use std::ffi::OsStr; use std::sync::Arc; use super::eval::{Ctx, Flow, Special}; @@ -335,16 +337,16 @@ fn num_ne(a: &[Value]) -> R { eq_chain(a, false) } fn lt(a: &[Value]) -> R { - chain(a, |o| o.is_lt()) + chain(a, Ordering::is_lt) } fn gt(a: &[Value]) -> R { - chain(a, |o| o.is_gt()) + chain(a, Ordering::is_gt) } fn le(a: &[Value]) -> R { - chain(a, |o| o.is_le()) + chain(a, Ordering::is_le) } fn ge(a: &[Value]) -> R { - chain(a, |o| o.is_ge()) + chain(a, Ordering::is_ge) } fn zerop(a: &[Value]) -> R { @@ -658,12 +660,12 @@ fn vl_list_to_string(a: &[Value]) -> R { fn vl_filename_base(a: &[Value]) -> R { arity(a, 1, 1)?; let s = string(&a[0])?; - Ok(Value::str(std::path::Path::new(&*s.replace('\\', "/")).file_stem().and_then(|x| x.to_str()).unwrap_or(""))) + Ok(Value::str(std::path::Path::new(&*s.replace('\\', "/")).file_stem().and_then(OsStr::to_str).unwrap_or(""))) } fn vl_filename_extension(a: &[Value]) -> R { arity(a, 1, 1)?; let s = string(&a[0])?; - Ok(std::path::Path::new(&*s.replace('\\', "/")).extension().and_then(|x| x.to_str()).map(|e| Value::str(&format!(".{e}"))).unwrap_or_default()) + Ok(std::path::Path::new(&*s.replace('\\', "/")).extension().and_then(OsStr::to_str).map(|e| Value::str(&format!(".{e}"))).unwrap_or_default()) } fn vl_filename_directory(a: &[Value]) -> R { arity(a, 1, 1)?; diff --git a/crates/engine/src/lisp/mod.rs b/crates/engine/src/lisp/mod.rs index d596fa93..b3175c51 100644 --- a/crates/engine/src/lisp/mod.rs +++ b/crates/engine/src/lisp/mod.rs @@ -624,7 +624,7 @@ impl Session { self.lisp.files.insert(file_key(name), text.to_string()); let r = self.lisp_eval(text, false, blocking, Vec::new())?; if !matches!(r, Some(Err(_))) { - let shown = std::path::Path::new(name).file_name().and_then(|n| n.to_str()).unwrap_or(name).to_string(); + let shown = std::path::Path::new(name).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(name).to_string(); self.echo(format!("{shown} successfully loaded.")); } Ok(r) @@ -632,7 +632,7 @@ impl Session { } fn file_key(name: &str) -> String { - let base = std::path::Path::new(&name.replace('\\', "/")).file_name().and_then(|n| n.to_str()).unwrap_or(name).to_ascii_lowercase(); + let base = std::path::Path::new(&name.replace('\\', "/")).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(name).to_ascii_lowercase(); base.strip_suffix(".lsp").map(str::to_string).unwrap_or(base) } diff --git a/crates/engine/src/prompt.rs b/crates/engine/src/prompt.rs index b9c45eae..63883240 100644 --- a/crates/engine/src/prompt.rs +++ b/crates/engine/src/prompt.rs @@ -101,7 +101,7 @@ impl Prompt { self } pub fn kw(mut self, k: &[&str]) -> Self { - self.keywords = k.iter().map(|s| s.to_string()).collect(); + self.keywords = k.iter().map(ToString::to_string).collect(); self } pub fn base(mut self, p: Vec2) -> Self { diff --git a/crates/io/src/dxf_ext.rs b/crates/io/src/dxf_ext.rs index d699ecbd..486a9542 100644 --- a/crates/io/src/dxf_ext.rs +++ b/crates/io/src/dxf_ext.rs @@ -285,7 +285,7 @@ pub(crate) fn arrow_block_name(name: &str) -> Option<(String, Arrowhead)> { return None; } let kind = Arrowhead::parse(n); - let norm: String = n.trim_start_matches('_').chars().filter(|c| c.is_ascii_alphanumeric()).collect::().to_ascii_lowercase(); + let norm: String = n.trim_start_matches('_').chars().filter(char::is_ascii_alphanumeric).collect::().to_ascii_lowercase(); if kind == Arrowhead::ClosedFilled && !matches!(norm.as_str(), "closedfilled" | "filled") { return None; } diff --git a/crates/io/src/svg_read.rs b/crates/io/src/svg_read.rs index 36f8c458..21508fe7 100644 --- a/crates/io/src/svg_read.rs +++ b/crates/io/src/svg_read.rs @@ -16,7 +16,7 @@ use std::collections::{BTreeMap, HashSet}; use cadcraft_color::Color; -use cadcraft_doc::*; +use cadcraft_doc::{Circle, Common, Drawing, Ellipse, EntityKind, Layer, Line, LwPolyline, Space}; use cadcraft_geom::{Mat3, PolyVertex, Spline, Vec2, Vec3}; use quick_xml::Reader; use quick_xml::XmlVersion; @@ -527,7 +527,7 @@ impl Parser { } else if *rx > 0.0 && *ry > 0.0 { self.flush_run(&mut run, false)?; self.flush_ctrl(&mut ctrl)?; - for p in flatten_arc(&m, cur_user, *end, *rx, *ry, *rot, *large, *sweep)? { + for p in flatten_arc(&m, cur_user, *end, (*rx, *ry), *rot, *large, *sweep)? { if run.is_empty() { run.push(PolyVertex::new(cur)); } @@ -1004,7 +1004,7 @@ fn pick_sweep(ccw: f64, large: bool, sweep: bool) -> f64 { /// Flatten a general elliptical arc (any `rx`/`ry`/rotation) from `p0` to `p1` in user /// space, then map every sample through `m`. -fn flatten_arc(m: &Mat3, p0: Vec2, p1: Vec2, rx: f64, ry: f64, rot_deg: f64, large: bool, sweep: bool) -> Result> { +fn flatten_arc(m: &Mat3, p0: Vec2, p1: Vec2, (rx, ry): (f64, f64), rot_deg: f64, large: bool, sweep: bool) -> Result> { let (c, theta, delta) = elliptical_params(p0, p1, rx, ry, rot_deg.to_radians(), large, sweep) .ok_or_else(|| IoError::Format("SVG: elliptical arc cannot be converted".into()))?; let axes = Mat3::rotate(rot_deg.to_radians()); diff --git a/crates/mcp/tests/it/cmdline_key.rs b/crates/mcp/tests/it/cmdline_key.rs index 0f074beb..273a9456 100644 --- a/crates/mcp/tests/it/cmdline_key.rs +++ b/crates/mcp/tests/it/cmdline_key.rs @@ -14,7 +14,7 @@ fn keys(v: &Value) -> Vec { /// The `cmdline.state` fields plus `extra`. fn want(b: &mut Headless, extra: &[&str]) -> Vec { let mut k = keys(&b.call("cmdline.state", json!({})).unwrap()); - k.extend(extra.iter().map(|s| s.to_string())); + k.extend(extra.iter().map(ToString::to_string)); k.sort(); k } diff --git a/crates/mcp/tests/it/cmdline_parity.rs b/crates/mcp/tests/it/cmdline_parity.rs index 27209b57..5dae73e7 100644 --- a/crates/mcp/tests/it/cmdline_parity.rs +++ b/crates/mcp/tests/it/cmdline_parity.rs @@ -13,7 +13,7 @@ fn keys(v: &Value) -> Vec { } fn want(extra: &[&str]) -> Vec { - let mut k: Vec = STATE_KEYS.iter().chain(extra).map(|s| s.to_string()).collect(); + let mut k: Vec = STATE_KEYS.iter().chain(extra).map(ToString::to_string).collect(); k.sort(); k } diff --git a/crates/render/src/dim.rs b/crates/render/src/dim.rs index d9504c56..c05a2001 100644 --- a/crates/render/src/dim.rs +++ b/crates/render/src/dim.rs @@ -157,7 +157,7 @@ pub enum Arrowhead { impl Arrowhead { /// Parse a DIMBLK value ("" / "_DOT" / "Architectural tick" / "closed blank"…). pub fn parse(name: &str) -> Arrowhead { - let n: String = name.trim().trim_start_matches('_').chars().filter(|c| c.is_ascii_alphanumeric()).collect::().to_ascii_lowercase(); + let n: String = name.trim().trim_start_matches('_').chars().filter(char::is_ascii_alphanumeric).collect::().to_ascii_lowercase(); match n.as_str() { "" | "closedfilled" | "filled" => Arrowhead::ClosedFilled, "closedblank" | "blank" => Arrowhead::ClosedBlank, diff --git a/crates/ui-egui/src/mtext_editor.rs b/crates/ui-egui/src/mtext_editor.rs index 9e7db580..3bd54a3c 100644 --- a/crates/ui-egui/src/mtext_editor.rs +++ b/crates/ui-egui/src/mtext_editor.rs @@ -195,7 +195,7 @@ fn toolbar(ui: &mut egui::Ui, st: &mut State, base_h: f64) { // Font let shown = cur.font.clone().unwrap_or_else(|| st.fmt.base_font.clone()); egui::ComboBox::from_id_salt("mtext_font").width(120.0).selected_text(shown.clone()).show_ui(ui, |ui| { - for fam in FAMILIES.iter().map(|s| s.to_string()).chain(cur.font.clone().filter(|f| !FAMILIES.contains(&f.as_str()))) { + for fam in FAMILIES.iter().map(ToString::to_string).chain(cur.font.clone().filter(|f| !FAMILIES.contains(&f.as_str()))) { if ui.selectable_label(shown == fam, &fam).clicked() { change(st, |c| c.font = Some(fam.clone())); refocus = true; diff --git a/crates/ui-egui/src/quick.rs b/crates/ui-egui/src/quick.rs index 5effd59c..91f6740e 100644 --- a/crates/ui-egui/src/quick.rs +++ b/crates/ui-egui/src/quick.rs @@ -144,7 +144,7 @@ pub fn qselect_dialog(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) { ui.label(crate::tl!("Value:")); let choices: Vec = match st.property.as_str() { "color" => { - ["ByLayer", "ByBlock", "Red", "Yellow", "Green", "Cyan", "Blue", "Magenta", "White"].iter().map(|s| s.to_string()).collect() + ["ByLayer", "ByBlock", "Red", "Yellow", "Green", "Cyan", "Blue", "Magenta", "White"].iter().map(ToString::to_string).collect() } "layer" => app.session.doc().map(|d| d.layers.iter().map(|l| l.name.clone()).collect()).unwrap_or_default(), "linetype" => app.session.doc().map(|d| d.linetypes.iter().map(|l| l.name.clone()).collect()).unwrap_or_default(), diff --git a/crates/ui-egui/tests/it/chrome_geometry.rs b/crates/ui-egui/tests/it/chrome_geometry.rs index 280fc5a4..ab9c5b8f 100644 --- a/crates/ui-egui/tests/it/chrome_geometry.rs +++ b/crates/ui-egui/tests/it/chrome_geometry.rs @@ -45,34 +45,38 @@ fn visible_text(shape: &Shape, clip: Rect, result: &mut Vec<(String, Rect)>) { } } +fn check_status_labels_case(width: f32, height: f32, scale: f32, message: bool, overlaps: &mut Vec) { + let mut h = harness(width, height, scale, message); + if let Ok(directory) = std::env::var("CRAFT_UI_VISUAL_DIR") { + std::fs::create_dir_all(&directory).unwrap(); + h.render().unwrap().save(format!("{directory}/cad-{width}-{scale}-{message}.png")).unwrap(); + } + let mut texts = Vec::new(); + for shape in &h.output().shapes { + visible_text(&shape.shape, shape.clip_rect, &mut texts); + } + texts.retain(|(_, rect)| rect.center().y > height - 26.0); + // The current tab is always drawn; the others move to the layout list when the bar is narrow. + assert!(texts.iter().any(|(text, _)| text == "Model")); + if width >= 1200.0 { + assert!(texts.iter().any(|(text, _)| text == "Layout2")); + } + for (i, (first, first_rect)) in texts.iter().enumerate() { + for (second, second_rect) in texts.iter().skip(i + 1) { + if first_rect.intersects(*second_rect) { + overlaps.push(format!("{width}px @{scale}: {first:?} {first_rect:?} overlaps {second:?} {second_rect:?}")); + } + } + } +} + #[test] fn status_labels_never_overlap_at_standard_and_compact_sizes() { let mut overlaps = Vec::new(); for (width, height) in [(1200.0, 760.0), (900.0, 560.0)] { for scale in [1.0, 2.0] { for message in [false, true] { - let mut h = harness(width, height, scale, message); - if let Ok(directory) = std::env::var("CRAFT_UI_VISUAL_DIR") { - std::fs::create_dir_all(&directory).unwrap(); - h.render().unwrap().save(format!("{directory}/cad-{width}-{scale}-{message}.png")).unwrap(); - } - let mut texts = Vec::new(); - for shape in &h.output().shapes { - visible_text(&shape.shape, shape.clip_rect, &mut texts); - } - texts.retain(|(_, rect)| rect.center().y > height - 26.0); - // The current tab is always drawn; the others move to the layout list when the bar is narrow. - assert!(texts.iter().any(|(text, _)| text == "Model")); - if width >= 1200.0 { - assert!(texts.iter().any(|(text, _)| text == "Layout2")); - } - for (i, (first, first_rect)) in texts.iter().enumerate() { - for (second, second_rect) in texts.iter().skip(i + 1) { - if first_rect.intersects(*second_rect) { - overlaps.push(format!("{width}px @{scale}: {first:?} {first_rect:?} overlaps {second:?} {second_rect:?}")); - } - } - } + check_status_labels_case(width, height, scale, message, &mut overlaps); } } } diff --git a/crates/ui-egui/tests/it/control_cmdline.rs b/crates/ui-egui/tests/it/control_cmdline.rs index 34a3f8cb..928be2f8 100644 --- a/crates/ui-egui/tests/it/control_cmdline.rs +++ b/crates/ui-egui/tests/it/control_cmdline.rs @@ -23,7 +23,7 @@ fn keys(v: &Value) -> Vec { } fn want(extra: &[&str]) -> Vec { - let mut k: Vec = STATE_KEYS.iter().chain(extra).map(|s| s.to_string()).collect(); + let mut k: Vec = STATE_KEYS.iter().chain(extra).map(ToString::to_string).collect(); k.sort(); k } diff --git a/crates/ui-egui/tests/it/control_cmdline_key.rs b/crates/ui-egui/tests/it/control_cmdline_key.rs index b3858ffa..ac0f39d3 100644 --- a/crates/ui-egui/tests/it/control_cmdline_key.rs +++ b/crates/ui-egui/tests/it/control_cmdline_key.rs @@ -23,7 +23,7 @@ fn keys(v: &Value) -> Vec { /// The `cmdline.state` fields plus `extra`. fn want(app: &mut CadApp, extra: &[&str]) -> Vec { let mut k = keys(&call(app, "cmdline.state", json!({}))["result"]); - k.extend(extra.iter().map(|s| s.to_string())); + k.extend(extra.iter().map(ToString::to_string)); k.sort(); k } diff --git a/xtask/src/assets.rs b/xtask/src/assets.rs index 56ffc79b..32a50cc9 100644 --- a/xtask/src/assets.rs +++ b/xtask/src/assets.rs @@ -14,7 +14,7 @@ const ASSET_EXT: &[&str] = &[ const ASSET_DIRS: &[&str] = &["assets/", "docs/images/", "examples/"]; pub fn is_asset(path: &str) -> bool { - let ext = Path::new(path).extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase).unwrap_or_default(); + let ext = Path::new(path).extension().and_then(std::ffi::OsStr::to_str).map(str::to_ascii_lowercase).unwrap_or_default(); ASSET_DIRS.iter().any(|d| path.starts_with(d)) || ASSET_EXT.contains(&ext.as_str()) } diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 0017c10b..aab24578 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -190,7 +190,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 }