From 3c577968f61960c23d22d0530cbd612756dd4a15 Mon Sep 17 00:00:00 2001 From: Books Date: Sun, 11 Oct 2026 03:26:20 +1100 Subject: [PATCH] ARC: refuse zero-radius arcs instead of adding them (fixes #85) (#123) The `arc` command checked its three-point form for collinear points but added the arcs built by its start/center/end and center/radius forms unchecked, so radius 0 (or a start point on the center) added a degenerate Arc, dirtied the drawing and recorded an undo step. Every form now goes through the existing finite_arc check and fails with the same "these values do not define an arc" error the arc.* variants and the interactive ARC already use, leaving the drawing unchanged. Fixes #85. --- crates/engine/src/cmd/curves.rs | 2 +- crates/engine/src/cmd/draw.rs | 3 +++ crates/engine/src/tests.rs | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/curves.rs b/crates/engine/src/cmd/curves.rs index a98c8d2..fccb71a 100644 --- a/crates/engine/src/cmd/curves.rs +++ b/crates/engine/src/cmd/curves.rs @@ -43,7 +43,7 @@ pub fn is_locked(s: &Session, h: Handle) -> bool { // ---------------- arc constructions ---------------- -fn finite_arc(a: Arc) -> Option { +pub(crate) fn finite_arc(a: Arc) -> Option { (a.center.is_finite() && a.radius.is_finite() && a.radius > EPS && a.start.is_finite() && a.end.is_finite()).then_some(a) } diff --git a/crates/engine/src/cmd/draw.rs b/crates/engine/src/cmd/draw.rs index ea03fed..0bea1cf 100644 --- a/crates/engine/src/cmd/draw.rs +++ b/crates/engine/src/cmd/draw.rs @@ -172,6 +172,9 @@ fn run_arc(s: &mut Session, p: &Value) -> Result { } else { return Err(bad("arc", "give {p1,p2,p3}, {start,center,end} or {center,radius,start,end}")); }; + // Zero or non-finite radius (e.g. start == center): refuse, like the interactive ARC and the + // arc.* variants, before anything is added (#85). + let a = super::curves::finite_arc(a).ok_or_else(|| bad("arc", "these values do not define an arc"))?; added(s.add_entity(arc(&a))?) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index a83a38c..98b21bb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -846,3 +846,24 @@ fn deferred_pick_elsewhere_is_a_plain_point() { }); assert_eq!(c, Some(Vec2::new(0.0, 5.0))); } + +/// #85: an `arc` request that builds a zero-radius arc is refused and the drawing is unchanged. +#[test] +fn arc_refuses_zero_radius() { + for params in [json!({"center": [0, 0], "radius": 0, "start": 0, "end": 90}), json!({"start": [0, 0], "center": [0, 0], "end": [1, 0]})] { + let mut s = Session::new(); + let err = s.execute("arc", ¶ms).unwrap_err().to_string(); + assert!(err.contains("do not define an arc"), "{params}: {err}"); + let info = s.execute("drawing.inspect", &json!({})).unwrap(); + assert_eq!(info["entityCount"], 0, "{params}: {info}"); + assert_eq!(info["dirty"], false, "{params}: {info}"); + assert_eq!(info["undo"], json!([]), "{params}: {info}"); + } + // The positive-radius control still adds the quarter circle. + let mut s = Session::new(); + s.execute("arc", &json!({"center": [0, 0], "radius": 5, "start": 0, "end": 90})).unwrap(); + match &s.doc().unwrap().model.iter().next().unwrap().kind { + EntityKind::Arc(a) => assert_eq!((a.radius, a.start, a.end), (5.0, 0.0, std::f64::consts::FRAC_PI_2)), + other => panic!("{other:?}"), + } +}