Merge pull request #7 from enaut/usability-ergonomics-fixes

Fix and improve svg export

turtle-lib-macros/src/lib.rs

@@ -66,17 +66,11 @@ use syn::ItemFn;
 /// use turtle_lib::*;
 ///
 /// fn main() {
-///     // Handle optional SVG export headlessly without opening a window
-///     if let Some(filename) = turtle_lib::export::parse_svg_export_arg() {
 ///     let mut build_commands = |turtle: &mut turtle_lib::TurtlePlan| {
 ///         my_drawing(turtle);
 ///     };
-///         if let Err(e) = turtle_lib::export::run_headless_svg_export(&mut build_commands, &filename) {
-///             eprintln!("Error exporting SVG: {:?}", e);
-///             std::process::exit(1);
-///         }
-///         return;
-///     }
+///     // Handle optional SVG export headlessly without opening a window
+///     turtle_lib::export::handle_svg_export(&mut build_commands);
 ///
 ///     // Normal interactive GUI mode with window
 ///     turtle_lib::macroquad::Window::new("My Turtle Drawing", async {
@@ -252,18 +246,7 @@ fn turtle_main_impl(
             };
 
             // If --export-svg flag is present, export headlessly without opening a window
-            if let Some(filename) = turtle_lib::export::parse_svg_export_arg() {
-                match turtle_lib::export::run_headless_svg_export(&mut build_commands, &filename) {
-                    Ok(()) => {
-                        println!("SVG exported successfully to: {}", filename);
-                        return;
-                    }
-                    Err(e) => {
-                        eprintln!("Error exporting SVG: {:?}", e);
-                        std::process::exit(1);
-                    }
-                }
-            }
+            turtle_lib::export::handle_svg_export(&mut build_commands);
 
             // Normal rendering mode (interactive window)
             turtle_lib::macroquad::Window::new(#window_title, async {

turtle-lib/Cargo.toml

@@ -14,6 +14,7 @@ tracing = { version = "0.1", features = [
 ], default-features = false }
 turtle-lib-macros = { path = "../turtle-lib-macros" }
 crossbeam = "0.8"
+pico-args = { version = "0.5", features = ["eq-separator"] }
 
 [dev-dependencies]
 # For examples and testing

turtle-lib/examples/sierpinski_triangle.rs

@@ -17,6 +17,10 @@
 //! That is relatively easy to implement, as long as you follow these steps and let recursion do
 //! the rest. Another little bonus this example provides is the ability to customize the drawing
 //! size: the triangle will stay correctly sized and positioned automatically.
+//!
+//! Note: This example queries `screen_width()` and `screen_height()` to calculate positioning
+//! relative to the window. Consequently, it requires an active graphics window and does not
+//! support headless SVG export (`--export-svg`).
 
 use macroquad::window::{screen_height, screen_width};
 use turtle_lib::*;
@@ -86,6 +90,9 @@ fn sierpinski_triangle(turtle: &mut TurtlePlan, level: u8, size: f32) {
 /// `level` is still required, it can't be computed automatically. However, given the used
 /// canvas size, it will compute the appropriate size and start point so the triangle gets
 /// centered and occupies as much drawing space as possible while staying in bounds.
+///
+/// Note: Because this function queries window dimensions, it requires an active graphics
+/// window and cannot be run in headless SVG export mode.
 fn sierpinski_triangle_auto(turtle: &mut TurtlePlan, level: u8) {
     let size = TRIANGLE_SIZE;
 

turtle-lib/examples/star.rs

@@ -9,11 +9,11 @@ fn draw(turtle: &mut TurtlePlan) {
     turtle.set_pen_width(0.5);
 
     // Draw a 5-pointed star pattern repeatedly
-    for _i in 0..50000 {
+    for _i in 0..500 {
         turtle.forward(200.0);
-        turtle.circle_left(10.0, 72.0, 1000);
-        turtle.circle_right(5.0, 360.0, 1000);
-        turtle.circle_left(10.0, 72.0, 1000);
+        turtle.circle_left(10.0, 72.0, 100);
+        turtle.circle_right(5.0, 360.0, 100);
+        turtle.circle_left(10.0, 72.0, 100);
     }
 
     // Set animation speed

turtle-lib/src/export.rs

@@ -7,9 +7,31 @@ use crate::TurtlePlan;
 pub enum ExportError {
     Io(std::io::Error),
     Format(String),
+    Execution(String),
+    MissingFeature(String),
     // Additional formats can be added here.
 }
 
+impl std::fmt::Display for ExportError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Io(err) => write!(f, "I/O error: {err}"),
+            Self::Format(msg) => write!(f, "format error: {msg}"),
+            Self::Execution(msg) => write!(f, "execution error: {msg}"),
+            Self::MissingFeature(msg) => write!(f, "{msg}"),
+        }
+    }
+}
+
+impl std::error::Error for ExportError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::Io(err) => Some(err),
+            Self::Format(_) | Self::Execution(_) | Self::MissingFeature(_) => None,
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug)]
 pub enum DrawingFormat {
     #[cfg(feature = "svg")]
@@ -27,18 +49,27 @@ pub(crate) trait DrawingExporter {
     fn export(&self, world: &TurtleWorld, filename: &str) -> Result<(), ExportError>;
 }
 
+/// Parse the `--export-svg` parameter from a `pico_args::Arguments` instance.
+///
+/// # Errors
+///
+/// Returns `pico_args::Error` if `--export-svg` is provided without a valid filename.
+pub fn parse_svg_export_from_args(
+    args: &mut pico_args::Arguments,
+) -> Result<Option<String>, pico_args::Error> {
+    match args.opt_value_from_str::<_, String>("--export-svg") {
+        Ok(Some(s)) if s.trim().is_empty() || s.starts_with('-') => {
+            Err(pico_args::Error::OptionWithoutAValue("--export-svg"))
+        }
+        other => other,
+    }
+}
+
 /// Check command-line arguments for the `--export-svg <filename>` flag.
 #[must_use]
 pub fn parse_svg_export_arg() -> Option<String> {
-    let args: Vec<String> = std::env::args().collect();
-    let mut i = 1;
-    while i < args.len() {
-        if args[i] == "--export-svg" && i + 1 < args.len() {
-            return Some(args[i + 1].clone());
-        }
-        i += 1;
-    }
-    None
+    let mut args = pico_args::Arguments::from_env();
+    parse_svg_export_from_args(&mut args).ok().flatten()
 }
 
 /// Headless SVG export that executes drawing commands and writes an SVG file
@@ -46,27 +77,57 @@ pub fn parse_svg_export_arg() -> Option<String> {
 ///
 /// # Errors
 ///
-/// Returns `ExportError` if file I/O fails or if the `svg` feature is not enabled.
+/// Returns `ExportError` if file I/O fails, drawing execution panics, or if the `svg` feature is not enabled.
 pub fn run_headless_svg_export<F>(mut build_commands: F, filename: &str) -> Result<(), ExportError>
 where
     F: FnMut(&mut TurtlePlan),
 {
     #[cfg(feature = "svg")]
     {
-        let mut turtle = crate::create_turtle_plan();
-        build_commands(&mut turtle);
+        use std::panic::AssertUnwindSafe;
 
+        let mut turtle = crate::create_turtle_plan();
+
+        let result = std::panic::catch_unwind(AssertUnwindSafe(|| {
+            build_commands(&mut turtle);
+        }));
+
+        match result {
+            Ok(()) => {
                 let mut app = crate::TurtleApp::new();
                 app.execute_immediate(0, turtle);
 
                 app.export_drawing(filename, crate::export::DrawingFormat::Svg)
             }
+            Err(payload) => {
+                let raw_msg = if let Some(s) = payload.downcast_ref::<&str>() {
+                    (*s).to_string()
+                } else if let Some(s) = payload.downcast_ref::<String>() {
+                    s.clone()
+                } else {
+                    String::new()
+                };
+
+                let is_context_panic = raw_msg.contains("THREAD_ID.is_some()")
+                    || raw_msg.contains("macroquad");
+
+                let err_msg = if is_context_panic {
+                    "Drawing function called Macroquad window/GUI functions (e.g. `screen_width()`, `screen_height()`) which are unavailable in headless SVG export mode.".to_string()
+                } else if !raw_msg.is_empty() {
+                    raw_msg
+                } else {
+                    "Drawing function panicked during execution".to_string()
+                };
+                Err(ExportError::Execution(err_msg))
+            }
+        }
+    }
 
     #[cfg(not(feature = "svg"))]
     {
         let _ = &mut build_commands;
         let _ = filename;
-        Err(ExportError::Format(
+        Err(ExportError::MissingFeature(
             "SVG export feature is not enabled. Please rebuild with --features svg".to_string(),
         ))
     }
@@ -79,16 +140,179 @@ pub fn handle_svg_export<F>(build_commands: F)
 where
     F: FnMut(&mut TurtlePlan),
 {
-    if let Some(filename) = parse_svg_export_arg() {
+    let mut args = pico_args::Arguments::from_env();
+    let export_arg = parse_svg_export_from_args(&mut args);
+
+    #[cfg(not(feature = "svg"))]
+    {
+        let _ = build_commands;
+        if export_arg.is_err() || matches!(export_arg, Ok(Some(_))) {
+            eprintln!("Error: SVG export feature is not enabled. Please rebuild with --features svg");
+            std::process::exit(1);
+        }
+    }
+
+    #[cfg(feature = "svg")]
+    {
+        match export_arg {
+            Ok(None) => {}
+            Err(_) => {
+                eprintln!("Error: --export-svg: option requires an argument");
+                std::process::exit(1);
+            }
+            Ok(Some(filename)) => {
                 match run_headless_svg_export(build_commands, &filename) {
                     Ok(()) => {
                         println!("SVG exported successfully to: {filename}");
                         std::process::exit(0);
                     }
                     Err(e) => {
-                eprintln!("Error exporting SVG: {e:?}");
+                        if let ExportError::Execution(ref msg) = e {
+                            if msg.contains("unavailable in headless SVG export mode") {
+                                eprintln!("\n================================================================================");
+                                eprintln!("Headless SVG Export Note:");
+                                eprintln!("A Macroquad window/rendering function (e.g. `screen_width()`, `screen_height()`,");
+                                eprintln!("or input check) was called while running in headless export mode.");
+                                eprintln!("Headless export does not initialize a graphics window. To resolve this:");
+                                eprintln!("  - Use relative turtle commands or fixed coordinates instead of window queries, or");
+                                eprintln!("  - Run the program in windowed GUI mode without the `--export-svg` flag.");
+                                eprintln!("================================================================================\n");
+                            }
+                        }
+                        eprintln!("Error: {e}");
                         std::process::exit(1);
                     }
                 }
             }
         }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::ffi::OsString;
+
+    fn make_args(args: &[&str]) -> pico_args::Arguments {
+        let os_args: Vec<OsString> = args.iter().map(OsString::from).collect();
+        pico_args::Arguments::from_vec(os_args)
+    }
+
+    #[test]
+    fn test_parse_svg_export_with_filename() {
+        let mut args = make_args(&["--export-svg", "output.svg"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert_eq!(res.unwrap(), Some("output.svg".to_string()));
+    }
+
+    #[test]
+    fn test_parse_svg_export_with_equals() {
+        let mut args = make_args(&["--export-svg=output.svg"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert_eq!(res.unwrap(), Some("output.svg".to_string()));
+    }
+
+    #[test]
+    fn test_parse_svg_export_missing_argument_at_end() {
+        let mut args = make_args(&["--export-svg"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert!(matches!(res, Err(pico_args::Error::OptionWithoutAValue(_))));
+    }
+
+    #[test]
+    fn test_parse_svg_export_missing_argument_followed_by_flag() {
+        let mut args = make_args(&["--export-svg", "--verbose"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert!(matches!(res, Err(pico_args::Error::OptionWithoutAValue(_))));
+    }
+
+    #[test]
+    fn test_parse_svg_export_empty_string() {
+        let mut args = make_args(&["--export-svg", ""]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert!(matches!(res, Err(pico_args::Error::OptionWithoutAValue(_))));
+    }
+
+    #[test]
+    fn test_parse_svg_export_empty_equals() {
+        let mut args = make_args(&["--export-svg="]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert!(matches!(res, Err(pico_args::Error::OptionWithoutAValue(_))));
+    }
+
+    #[test]
+    fn test_parse_svg_export_not_present() {
+        let mut args = make_args(&["--verbose", "input.txt"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert_eq!(res.unwrap(), None);
+    }
+
+    #[test]
+    fn test_parse_svg_export_preserves_unknown_args() {
+        let mut args = make_args(&["--other-flag", "--export-svg", "out.svg", "positional"]);
+        let res = parse_svg_export_from_args(&mut args);
+        assert_eq!(res.unwrap(), Some("out.svg".to_string()));
+        assert!(args.contains("--other-flag"));
+    }
+
+    #[test]
+    fn test_export_error_display_missing_feature() {
+        let err = ExportError::MissingFeature(
+            "SVG export feature is not enabled. Please rebuild with --features svg".to_string(),
+        );
+        assert_eq!(
+            err.to_string(),
+            "SVG export feature is not enabled. Please rebuild with --features svg"
+        );
+    }
+
+    #[cfg(feature = "svg")]
+    #[test]
+    fn test_headless_svg_export_success() {
+        use crate::Movement;
+
+        let temp_dir = std::env::temp_dir();
+        let path = temp_dir.join(format!("turtle_test_export_{}.svg", std::process::id()));
+        let path_str = path.to_str().expect("valid utf-8 path");
+
+        let res = run_headless_svg_export(|turtle| {
+            turtle.forward(10.0);
+        }, path_str);
+
+        assert!(res.is_ok());
+        assert!(path.exists());
+        let content = std::fs::read_to_string(&path).expect("read temp svg");
+        assert!(content.contains("<svg"));
+        let _ = std::fs::remove_file(&path);
+    }
+
+    #[cfg(feature = "svg")]
+    #[test]
+    fn test_headless_svg_export_macroquad_panic() {
+        let res = run_headless_svg_export(|_turtle| {
+            panic!("assertion failed: THREAD_ID.is_some()");
+        }, "unused.svg");
+
+        match res {
+            Err(ExportError::Execution(msg)) => {
+                assert!(msg.contains("unavailable in headless SVG export mode"));
+            }
+            other => panic!("expected ExportError::Execution, got {other:?}"),
+        }
+    }
+
+    #[cfg(feature = "svg")]
+    #[test]
+    fn test_headless_svg_export_custom_panic() {
+        let res = run_headless_svg_export(|_turtle| {
+            panic!("custom user panic occurred");
+        }, "unused.svg");
+
+        match res {
+            Err(ExportError::Execution(msg)) => {
+                assert_eq!(msg, "custom user panic occurred");
+            }
+            other => panic!("expected ExportError::Execution, got {other:?}"),
+        }
+    }
+}

turtle-lib/src/export_svg.rs

@@ -27,9 +27,11 @@ pub mod svg_export {
 
     pub struct SvgExporter;
 
-    impl DrawingExporter for SvgExporter {
+    impl SvgExporter {
+        /// Generate an SVG [`Document`] from the given [`TurtleWorld`].
+        #[must_use]
         #[allow(clippy::too_many_lines)]
-        fn export(&self, world: &TurtleWorld, filename: &str) -> Result<(), ExportError> {
+        pub fn to_svg_document(world: &TurtleWorld) -> Document {
             let mut doc = Document::new();
 
             let mut min_x = f32::INFINITY;
@@ -58,7 +60,8 @@ pub mod svg_export {
                                 .set("x2", end.x)
                                 .set("y2", end.y)
                                 .set("stroke", color_to_svg(*color))
-                                .set("stroke-width", *pen_width);
+                                .set("stroke-width", *pen_width)
+                                .set("stroke-linecap", "round");
                             doc = doc.add(line);
                         }
 
@@ -80,6 +83,7 @@ pub mod svg_export {
                                 *direction,
                             );
                             let center = geom.center;
+                            let radius_val = radius.abs();
                             // Include the bounding box of the full circle so partial arcs
                             // are never clipped.
                             update_bounds(
@@ -87,24 +91,24 @@ pub mod svg_export {
                                 &mut max_x,
                                 &mut min_y,
                                 &mut max_y,
-                                center.x - radius,
-                                center.y - radius,
+                                center.x - radius_val,
+                                center.y - radius_val,
                             );
                             update_bounds(
                                 &mut min_x,
                                 &mut max_x,
                                 &mut min_y,
                                 &mut max_y,
-                                center.x + radius,
-                                center.y + radius,
+                                center.x + radius_val,
+                                center.y + radius_val,
                             );
 
-                            if (angle.value() - 360.0).abs() < 1e-3 {
+                            if (angle.value().abs() - 360.0).abs() < 1e-3 {
                                 // Full circle — emit as <circle>
                                 let circle = Circle::new()
                                     .set("cx", center.x)
                                     .set("cy", center.y)
-                                    .set("r", *radius)
+                                    .set("r", radius_val)
                                     .set("stroke", color_to_svg(*color))
                                     .set("stroke-width", *pen_width)
                                     .set("fill", "none");
@@ -112,17 +116,19 @@ pub mod svg_export {
                             } else {
                                 // Partial arc — emit as <path A …>
                                 let end = geom.position_at_angle(angle.as_radians().value());
-                                let large_arc = i32::from(angle.value() > 180.0);
-                                let sweep = match direction {
-                                    crate::circle_geometry::CircleDirection::Left => 0,
-                                    crate::circle_geometry::CircleDirection::Right => 1,
+                                let large_arc = i32::from(angle.value().abs() > 180.0);
+                                let sweep = match (direction, angle.value() >= 0.0) {
+                                    (crate::circle_geometry::CircleDirection::Right, true)
+                                    | (crate::circle_geometry::CircleDirection::Left, false) => 1,
+                                    (crate::circle_geometry::CircleDirection::Left, true)
+                                    | (crate::circle_geometry::CircleDirection::Right, false) => 0,
                                 };
                                 let d = format!(
                                     "M {} {} A {} {} 0 {} {} {} {}",
                                     start_position.x,
                                     start_position.y,
-                                    radius,
-                                    radius,
+                                    radius_val,
+                                    radius_val,
                                     large_arc,
                                     sweep,
                                     end.x,
@@ -132,6 +138,7 @@ pub mod svg_export {
                                     .set("d", d)
                                     .set("stroke", color_to_svg(*color))
                                     .set("stroke-width", *pen_width)
+                                    .set("stroke-linecap", "round")
                                     .set("fill", "none");
                                 doc = doc.add(path);
                             }
@@ -218,6 +225,13 @@ pub mod svg_export {
                 doc = doc.set("viewBox", "0 0 400 400");
             }
 
+            doc
+        }
+    }
+
+    impl DrawingExporter for SvgExporter {
+        fn export(&self, world: &TurtleWorld, filename: &str) -> Result<(), ExportError> {
+            let doc = Self::to_svg_document(world);
             let mut file = File::create(filename).map_err(ExportError::Io)?;
             svg::write(&mut file, &doc).map_err(ExportError::Io)?;
             Ok(())
@@ -234,4 +248,95 @@ pub mod svg_export {
             format!("rgb({r},{g},{b})")
         }
     }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+        use crate::circle_geometry::CircleDirection;
+        use crate::general::{Color, Coordinate, Degrees};
+        use crate::state::{SvgRecord, Turtle};
+
+        #[test]
+        fn test_svg_export_line_has_round_line_caps() {
+            let mut world = TurtleWorld::new();
+            let mut turtle = Turtle::default();
+            turtle.svg_log.push(SvgRecord::Line {
+                start: Coordinate::new(0.0, 0.0),
+                end: Coordinate::new(100.0, 0.0),
+                color: Color::new(0.0, 0.0, 0.0, 1.0),
+                pen_width: 2.0,
+            });
+            world.turtles.push(turtle);
+
+            let doc = SvgExporter::to_svg_document(&world);
+            let svg_string = doc.to_string();
+            assert!(
+                svg_string.contains(r#"stroke-linecap="round""#),
+                "SVG export of lines should have round line caps: {svg_string}"
+            );
+        }
+
+        #[test]
+        fn test_svg_export_arc_has_round_line_caps() {
+            let mut world = TurtleWorld::new();
+            let mut turtle = Turtle::default();
+            turtle.svg_log.push(SvgRecord::Arc {
+                start_position: Coordinate::new(0.0, 0.0),
+                start_heading: 0.0,
+                radius: 50.0,
+                angle: Degrees::new(90.0),
+                direction: CircleDirection::Right,
+                color: Color::new(0.0, 0.0, 0.0, 1.0),
+                pen_width: 2.0,
+            });
+            world.turtles.push(turtle);
+
+            let doc = SvgExporter::to_svg_document(&world);
+            let svg_string = doc.to_string();
+            assert!(
+                svg_string.contains(r#"stroke-linecap="round""#),
+                "SVG export of partial arcs should have round line caps: {svg_string}"
+            );
+        }
+
+        #[test]
+        fn test_svg_export_arc_negative_angle_sweep() {
+            let mut world = TurtleWorld::new();
+            let mut turtle = Turtle::default();
+            // Circle right with negative angle should sweep counter-clockwise (sweep = 0)
+            turtle.svg_log.push(SvgRecord::Arc {
+                start_position: Coordinate::new(0.0, 0.0),
+                start_heading: 0.0,
+                radius: 50.0,
+                angle: Degrees::new(-90.0),
+                direction: CircleDirection::Right,
+                color: Color::new(0.0, 0.0, 0.0, 1.0),
+                pen_width: 2.0,
+            });
+            // Circle left with negative angle should sweep clockwise (sweep = 1)
+            turtle.svg_log.push(SvgRecord::Arc {
+                start_position: Coordinate::new(100.0, 100.0),
+                start_heading: 0.0,
+                radius: 50.0,
+                angle: Degrees::new(-90.0),
+                direction: CircleDirection::Left,
+                color: Color::new(0.0, 0.0, 0.0, 1.0),
+                pen_width: 2.0,
+            });
+            world.turtles.push(turtle);
+
+            let doc = SvgExporter::to_svg_document(&world);
+            let svg_string = doc.to_string();
+            // Right with negative angle: large_arc=0, sweep=0 -> "0 0 0"
+            assert!(
+                svg_string.contains("A 50 50 0 0 0"),
+                "Circle right with negative angle should have sweep=0: {svg_string}"
+            );
+            // Left with negative angle: large_arc=0, sweep=1 -> "0 0 1"
+            assert!(
+                svg_string.contains("A 50 50 0 0 1"),
+                "Circle left with negative angle should have sweep=1: {svg_string}"
+            );
+        }
+    }
 }

turtle-lib/src/state.rs

@@ -126,6 +126,7 @@ pub(crate) struct SvgLog {
 }
 
 impl SvgLog {
+    #[allow(clippy::unused_self)]
     pub(crate) fn clear(&mut self) {
         #[cfg(feature = "svg")]
         self.records.clear();