From c35b902f3a5aa76a3bd98e1ea24fe90cf8aad8ec Mon Sep 17 00:00:00 2001
From: Karim Jerbi <k4rim.jerbi@gmail.com>
Date: Mon, 5 Oct 2026 13:23:25 +0100
Subject: [PATCH] fix(model): respect typed height on cylinder and primitive
 creation

---
 src/modules/model/cylinder_cmd.rs  | 90 +++++++++++++++++++++++++++++-
 src/modules/model/primitive_cmd.rs | 25 +++++++--
 2 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/src/modules/model/cylinder_cmd.rs b/src/modules/model/cylinder_cmd.rs
index 6b1b11df4..a0ca277f4 100644
--- a/src/modules/model/cylinder_cmd.rs
+++ b/src/modules/model/cylinder_cmd.rs
@@ -734,10 +734,14 @@ impl CadCommand for CylinderCommand {
     fn point_step_accepts_keywords(&self) -> bool {
         matches!(
             self.step,
-            Step::BaseCenter | Step::BaseRadius | Step::EllipseFirst | Step::Height
+            Step::BaseCenter | Step::BaseRadius | Step::EllipseFirst
         )
     }
 
+    fn dyn_commit_as_text(&self) -> bool {
+        matches!(self.step, Step::Height | Step::BaseDiameter)
+    }
+
     fn on_text_input(&mut self, raw: &str) -> Option<CmdResult> {
         self.on_text(raw)
     }
@@ -827,3 +831,87 @@ fn push_ellipse(
 fn wire(name: &str, points: Vec<[f32; 3]>) -> WireModel {
     WireModel::solid(name.to_string(), points, WireModel::CYAN, false)
 }
+
+#[cfg(test)]
+mod cylinder_tests {
+    use super::*;
+
+    #[test]
+    fn cylinder_creation_height_workflow() {
+        // Reset defaults
+        if let Ok(mut defs) = defaults().lock() {
+            *defs = Defaults::default();
+        }
+
+        // Test 1: Typing height upon creation respects the typed height
+        let mut command = CylinderCommand::new();
+        assert!(!command.dyn_commit_as_text());
+
+        // Step 1: Center
+        assert!(matches!(command.on_point(DVec3::ZERO), CmdResult::NeedPoint));
+        assert!(!command.dyn_commit_as_text());
+
+        // Step 2: Radius
+        assert!(matches!(
+            command.on_point(DVec3::new(5.0, 0.0, 0.0)),
+            CmdResult::NeedPoint
+        ));
+
+        // Step 3: Height
+        assert!(command.dyn_commit_as_text());
+        assert!(!command.point_step_accepts_keywords());
+
+        let res = command.on_text_input("25.0");
+        let Some(CmdResult::CommitSolid { history, .. }) = res else {
+            panic!("expected CommitSolid");
+        };
+        let SolidHistoryOperation::Cylinder(cyl) = history else {
+            panic!("expected Cylinder history");
+        };
+        assert_eq!(cyl.height, 25.0);
+        assert_eq!(defaults().lock().unwrap().height, 25.0);
+
+        // Test 2: Create cylinder with mouse drag
+        let mut cmd1 = CylinderCommand::new();
+        cmd1.on_point(DVec3::ZERO);
+        cmd1.on_point(DVec3::new(4.0, 0.0, 0.0));
+        let res1 = cmd1.on_point(DVec3::new(0.0, 0.0, 10.0));
+        let CmdResult::CommitSolid { history, .. } = res1 else {
+            panic!("expected CommitSolid");
+        };
+        let SolidHistoryOperation::Cylinder(cyl1) = history else {
+            panic!("expected Cylinder history");
+        };
+        assert_eq!(cyl1.height, 10.0);
+        assert_eq!(defaults().lock().unwrap().height, 10.0);
+
+        // Test 3: Next cylinder with Enter respects remembered mouse-dragged height
+        let mut cmd2 = CylinderCommand::new();
+        cmd2.on_point(DVec3::ZERO);
+        cmd2.on_point(DVec3::new(4.0, 0.0, 0.0));
+        let res2 = cmd2.on_enter();
+        let CmdResult::CommitSolid { history, .. } = res2 else {
+            panic!("expected CommitSolid");
+        };
+        let SolidHistoryOperation::Cylinder(cyl2) = history else {
+            panic!("expected Cylinder history");
+        };
+        assert_eq!(cyl2.height, 10.0);
+
+        // Test 4: Next cylinder: typing different height respects the new typed height (not retaining 10.0)!
+        let mut cmd3 = CylinderCommand::new();
+        cmd3.on_point(DVec3::ZERO);
+        cmd3.on_point(DVec3::new(4.0, 0.0, 0.0));
+        assert!(cmd3.dyn_commit_as_text());
+        let res3 = cmd3.on_text_input("42.0");
+        let Some(CmdResult::CommitSolid { history, .. }) = res3 else {
+            panic!("expected CommitSolid");
+        };
+        let SolidHistoryOperation::Cylinder(cyl3) = history else {
+            panic!("expected Cylinder history");
+        };
+        assert_eq!(cyl3.height, 42.0);
+        assert_eq!(defaults().lock().unwrap().height, 42.0);
+    }
+}
+
diff --git a/src/modules/model/primitive_cmd.rs b/src/modules/model/primitive_cmd.rs
index cca272072..e725bd2ad 100644
--- a/src/modules/model/primitive_cmd.rs
+++ b/src/modules/model/primitive_cmd.rs
@@ -2866,8 +2866,6 @@ impl CadCommand for PrimitiveCommand {
                 ConeStep::BaseCenter
                     | ConeStep::BaseRadius
                     | ConeStep::EllipseFirst
-                    | ConeStep::Height
-                    | ConeStep::HeightAfterTopRadius
             );
         }
         if self.shape == Shape::Pyramid {
@@ -2875,17 +2873,34 @@ impl CadCommand for PrimitiveCommand {
                 self.pyramid_step,
                 PyramidStep::BaseCenter
                     | PyramidStep::BaseRadius
-                    | PyramidStep::Height
-                    | PyramidStep::HeightAfterTopRadius
             );
         }
         self.shape.rectangular()
             && matches!(
                 self.box_step,
-                BoxStep::FirstCorner | BoxStep::OppositeCorner | BoxStep::Height
+                BoxStep::FirstCorner | BoxStep::OppositeCorner
             )
     }
 
+    fn dyn_commit_as_text(&self) -> bool {
+        if self.shape == Shape::Cone {
+            return matches!(
+                self.cone_step,
+                ConeStep::Height | ConeStep::HeightAfterTopRadius
+            );
+        }
+        if self.shape == Shape::Pyramid {
+            return matches!(
+                self.pyramid_step,
+                PyramidStep::Height | PyramidStep::HeightAfterTopRadius
+            );
+        }
+        if self.shape.rectangular() {
+            return matches!(self.box_step, BoxStep::Height);
+        }
+        self.height_step
+    }
+
     fn on_text_input(&mut self, raw: &str) -> Option<CmdResult> {
         if self.shape == Shape::Sphere {
             let token = raw.trim();