Move helix revolutions into the main helix args (#5942)

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* docs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updaes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>

docs/kcl/index.md

@@ -74,7 +74,6 @@ layout: manual
   * [`getOppositeEdge`](kcl/getOppositeEdge)
   * [`getPreviousAdjacentEdge`](kcl/getPreviousAdjacentEdge)
   * [`helix`](kcl/helix)
-  * [`helixRevolutions`](kcl/helixRevolutions)
   * [`hole`](kcl/hole)
   * [`hollow`](kcl/hollow)
   * [`inch`](kcl/inch)

docs/kcl/types/HelixValue.md

@@ -21,6 +21,7 @@ A helix.
 | `revolutions` |[`number`](/docs/kcl/types/number)| Number of revolutions. | No |
 | `angleStart` |[`number`](/docs/kcl/types/number)| Start angle (in degrees). | No |
 | `ccw` |`boolean`| Is the helix rotation counter clockwise? | No |
+| `cylinderId` |[`string`](/docs/kcl/types/string)| The cylinder the helix was created on. | No |
 | `units` |[`UnitLen`](/docs/kcl/types/UnitLen)| A unit of length. | No |
 
 

rust/kcl-lib/e2e/executor/inputs/helix_defaults.kcl

@@ -1,4 +1,4 @@
 const part001 = startSketchOn(XY)
      |> circle(center= [5, 5], radius= 10)
      |> extrude(length = 10)
-     |> helixRevolutions({revolutions = 16, angleStart = 0}, %)
+     |> helix(revolutions = 16, angleStart = 0, cylinder = %)

rust/kcl-lib/e2e/executor/inputs/helix_defaults_negative_extrude.kcl

@@ -1,4 +1,4 @@
 const part001 = startSketchOn(XY)
      |> circle(center = [5, 5], radius = 10)
      |> extrude(length = -10)
-     |> helixRevolutions({revolutions = 16, angleStart = 0}, %)
+     |> helix(revolutions = 16, angleStart = 0, cylinder = %)

rust/kcl-lib/e2e/executor/inputs/helix_with_length.kcl

@@ -1,4 +1,4 @@
 const part001 = startSketchOn(XY)
      |> circle(center= [5, 5], radius= 10)
      |> extrude(length = 10)
-     |> helixRevolutions({revolutions = 16, angleStart = 0, length = 3}, %)
+     |> helix(revolutions = 16, angleStart = 0, length = 3, cylinder = %)

rust/kcl-lib/src/docs/mod.rs

@@ -127,13 +127,14 @@ impl StdLibFnArg {
         } else {
             ""
         };
-        if self.type_ == "Sketch"
+        if (self.type_ == "Sketch"
             || self.type_ == "[Sketch]"
             || self.type_ == "Solid"
             || self.type_ == "[Solid]"
             || self.type_ == "SketchSurface"
             || self.type_ == "SketchOrSurface"
-            || self.type_ == "SolidOrSketchOrImportedGeometry"
+            || self.type_ == "SolidOrSketchOrImportedGeometry")
+            && (self.required || self.include_in_snippet)
         {
             return Ok(Some((index, format!("{label}${{{}:{}}}", index, "%"))));
         } else if (self.type_ == "TagDeclarator" || self.type_ == "TagNode") && self.required {
@@ -1094,16 +1095,38 @@ mod tests {
 
     #[test]
     #[allow(clippy::literal_string_with_formatting_args)]
-    fn get_autocomplete_snippet_helix_revolutions() {
+    fn get_autocomplete_snippet_union() {
-        let helix_fn: Box<dyn StdLibFn> = Box::new(crate::std::helix::HelixRevolutions);
+        let union_fn: Box<dyn StdLibFn> = Box::new(crate::std::csg::Union);
-        let snippet = helix_fn.to_autocomplete_snippet().unwrap();
+        let snippet = union_fn.to_autocomplete_snippet().unwrap();
+        assert_eq!(snippet, r#"union(${0:%})${}"#);
+    }
+
+    #[test]
+    #[allow(clippy::literal_string_with_formatting_args)]
+    fn get_autocomplete_snippet_subtract() {
+        let subtract_fn: Box<dyn StdLibFn> = Box::new(crate::std::csg::Subtract);
+        let snippet = subtract_fn.to_autocomplete_snippet().unwrap();
+        assert_eq!(snippet, r#"subtract(${0:%}, tools = ${1:%})${}"#);
+    }
+
+    #[test]
+    #[allow(clippy::literal_string_with_formatting_args)]
+    fn get_autocomplete_snippet_intersect() {
+        let intersect_fn: Box<dyn StdLibFn> = Box::new(crate::std::csg::Intersect);
+        let snippet = intersect_fn.to_autocomplete_snippet().unwrap();
+        assert_eq!(snippet, r#"intersect(${0:%})${}"#);
+    }
+
+    #[test]
+    #[allow(clippy::literal_string_with_formatting_args)]
+    fn get_autocomplete_snippet_get_common_edge() {
+        let get_common_edge_fn: Box<dyn StdLibFn> = Box::new(crate::std::edge::GetCommonEdge);
+        let snippet = get_common_edge_fn.to_autocomplete_snippet().unwrap();
         assert_eq!(
             snippet,
-            r#"helixRevolutions({
+            r#"getCommonEdge(faces = [{
-	revolutions = ${0:3.14},
+	value = ${0:"string"},
-	angleStart = ${1:3.14},
+}])${}"#
-	ccw = ${2:false},
-}, ${3:%})${}"#
         );
     }
 

rust/kcl-lib/src/execution/geometry.rs

@@ -152,6 +152,8 @@ pub struct Helix {
     pub angle_start: f64,
     /// Is the helix rotation counter clockwise?
     pub ccw: bool,
+    /// The cylinder the helix was created on.
+    pub cylinder_id: Option<uuid::Uuid>,
     pub units: UnitLen,
     #[serde(skip)]
     pub meta: Vec<Metadata>,

rust/kcl-lib/src/std/args.rs

@@ -1258,23 +1258,6 @@ impl<'a> FromKclValue<'a> for super::sketch::BezierData {
     }
 }
 
-impl<'a> FromKclValue<'a> for super::helix::HelixRevolutionsData {
-    fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
-        let obj = arg.as_object()?;
-        let_field_of!(obj, revolutions);
-        let_field_of!(obj, length?);
-        let_field_of!(obj, ccw?);
-        let ccw = ccw.unwrap_or_default();
-        let angle_start = obj.get("angleStart")?.as_f64()?;
-        Some(Self {
-            revolutions,
-            angle_start,
-            ccw,
-            length,
-        })
-    }
-}
-
 impl<'a> FromKclValue<'a> for FaceTag {
     fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
         let case1 = || match arg.as_str() {

rust/kcl-lib/src/std/helix.rs

@@ -4,8 +4,6 @@ use anyhow::Result;
 use kcl_derive_docs::stdlib;
 use kcmc::{each_cmd as mcmd, length_unit::LengthUnit, shared::Angle, ModelingCmd};
 use kittycad_modeling_cmds as kcmc;
-use schemars::JsonSchema;
-use serde::{Deserialize, Serialize};
 
 use crate::{
     errors::KclError,
@@ -18,11 +16,71 @@ pub async fn helix(exec_state: &mut ExecState, args: Args) -> Result<KclValue, K
     let angle_start = args.get_kw_arg("angleStart")?;
     let revolutions = args.get_kw_arg("revolutions")?;
     let ccw = args.get_kw_arg_opt("ccw")?;
-    let radius = args.get_kw_arg("radius")?;
+    let radius = args.get_kw_arg_opt("radius")?;
-    let axis = args.get_kw_arg("axis")?;
+    let axis = args.get_kw_arg_opt("axis")?;
     let length = args.get_kw_arg_opt("length")?;
+    let cylinder = args.get_kw_arg_opt("cylinder")?;
 
-    let value = inner_helix(revolutions, angle_start, ccw, radius, axis, length, exec_state, args).await?;
+    // Make sure we have a radius if we don't have a cylinder.
+    if radius.is_none() && cylinder.is_none() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Radius is required when creating a helix without a cylinder.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    // Make sure we don't have a radius if we have a cylinder.
+    if radius.is_some() && cylinder.is_some() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Radius is not allowed when creating a helix with a cylinder.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    // Make sure we have an axis if we don't have a cylinder.
+    if axis.is_none() && cylinder.is_none() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Axis is required when creating a helix without a cylinder.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    // Make sure we don't have an axis if we have a cylinder.
+    if axis.is_some() && cylinder.is_some() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Axis is not allowed when creating a helix with a cylinder.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    // Make sure we have a radius if we have an axis.
+    if radius.is_none() && axis.is_some() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Radius is required when creating a helix around an axis.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    // Make sure we have an axis if we have a radius.
+    if axis.is_none() && radius.is_some() {
+        return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+            message: "Axis is required when creating a helix around an axis.".to_string(),
+            source_ranges: vec![args.source_range],
+        }));
+    }
+
+    let value = inner_helix(
+        revolutions,
+        angle_start,
+        ccw,
+        radius,
+        axis,
+        length,
+        cylinder,
+        exec_state,
+        args,
+    )
+    .await?;
     Ok(KclValue::Helix { value })
 }
 
@@ -88,6 +146,23 @@ pub async fn helix(exec_state: &mut ExecState, args: Args) -> Result<KclValue, K
 ///     |> circle( center = [0, 0], radius = 1 )
 ///     |> sweep(path = helixPath)
 /// ```
+///
+///
+///
+/// ```no_run
+/// // Create a helix on a cylinder.
+///
+/// part001 = startSketchOn('XY')
+///   |> circle( center= [5, 5], radius= 10 )
+///   |> extrude(length = 10)
+///
+/// helix(
+///     angleStart = 0,
+///     ccw = true,
+///     revolutions = 16,
+///     cylinder = part001,
+///  )
+/// ```
 #[stdlib {
     name = "helix",
     keywords = true,
@@ -96,9 +171,10 @@ pub async fn helix(exec_state: &mut ExecState, args: Args) -> Result<KclValue, K
         revolutions = { docs = "Number of revolutions."},
         angle_start = { docs = "Start angle (in degrees)."},
         ccw = { docs = "Is the helix rotation counter clockwise? The default is `false`.", include_in_snippet = false},
-        radius = { docs = "Radius of the helix."},
+        radius = { docs = "Radius of the helix.", include_in_snippet = true},
-        axis = { docs = "Axis to use for the helix."},
+        axis = { docs = "Axis to use for the helix.", include_in_snippet = true},
         length = { docs = "Length of the helix. This is not necessary if the helix is created around an edge. If not given the length of the edge is used.", include_in_snippet = true},
+        cylinder = { docs = "Cylinder to create the helix on.", include_in_snippet = false},
     },
     feature_tree_operation = true,
 }]
@@ -107,9 +183,10 @@ async fn inner_helix(
     revolutions: f64,
     angle_start: f64,
     ccw: Option<bool>,
-    radius: f64,
+    radius: Option<f64>,
-    axis: Axis3dOrEdgeReference,
+    axis: Option<Axis3dOrEdgeReference>,
     length: Option<f64>,
+    cylinder: Option<Solid>,
     exec_state: &mut ExecState,
     args: Args,
 ) -> Result<Box<HelixValue>, KclError> {
@@ -120,6 +197,7 @@ async fn inner_helix(
         artifact_id: id.into(),
         revolutions,
         angle_start,
+        cylinder_id: cylinder.as_ref().map(|c| c.id),
         ccw: ccw.unwrap_or(false),
         units: exec_state.length_unit(),
         meta: vec![args.source_range.into()],
@@ -129,113 +207,63 @@ async fn inner_helix(
         return Ok(helix_result);
     }
 
-    match axis {
+    if let Some(cylinder) = cylinder {
-        Axis3dOrEdgeReference::Axis(axis) => {
+        args.batch_modeling_cmd(
-            let (axis, origin) = axis.axis_and_origin()?;
+            id,
+            ModelingCmd::from(mcmd::EntityMakeHelix {
+                cylinder_id: cylinder.id,
+                is_clockwise: !helix_result.ccw,
+                length: LengthUnit(length.unwrap_or(cylinder.height)),
+                revolutions,
+                start_angle: Angle::from_degrees(angle_start),
+            }),
+        )
+        .await?;
+    } else if let (Some(axis), Some(radius)) = (axis, radius) {
+        match axis {
+            Axis3dOrEdgeReference::Axis(axis) => {
+                let (axis, origin) = axis.axis_and_origin()?;
 
-            // Make sure they gave us a length.
+                // Make sure they gave us a length.
-            let Some(length) = length else {
+                let Some(length) = length else {
-                return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+                    return Err(KclError::Semantic(crate::errors::KclErrorDetails {
-                    message: "Length is required when creating a helix around an axis.".to_string(),
+                        message: "Length is required when creating a helix around an axis.".to_string(),
-                    source_ranges: vec![args.source_range],
+                        source_ranges: vec![args.source_range],
-                }));
+                    }));
-            };
+                };
 
-            args.batch_modeling_cmd(
+                args.batch_modeling_cmd(
-                id,
+                    id,
-                ModelingCmd::from(mcmd::EntityMakeHelixFromParams {
+                    ModelingCmd::from(mcmd::EntityMakeHelixFromParams {
-                    radius: LengthUnit(radius),
+                        radius: LengthUnit(radius),
-                    is_clockwise: !helix_result.ccw,
+                        is_clockwise: !helix_result.ccw,
-                    length: LengthUnit(length),
+                        length: LengthUnit(length),
-                    revolutions,
+                        revolutions,
-                    start_angle: Angle::from_degrees(angle_start),
+                        start_angle: Angle::from_degrees(angle_start),
-                    axis,
+                        axis,
-                    center: origin,
+                        center: origin,
-                }),
+                    }),
-            )
+                )
-            .await?;
+                .await?;
-        }
+            }
-        Axis3dOrEdgeReference::Edge(edge) => {
+            Axis3dOrEdgeReference::Edge(edge) => {
-            let edge_id = edge.get_engine_id(exec_state, &args)?;
+                let edge_id = edge.get_engine_id(exec_state, &args)?;
 
-            args.batch_modeling_cmd(
+                args.batch_modeling_cmd(
-                id,
+                    id,
-                ModelingCmd::from(mcmd::EntityMakeHelixFromEdge {
+                    ModelingCmd::from(mcmd::EntityMakeHelixFromEdge {
-                    radius: LengthUnit(radius),
+                        radius: LengthUnit(radius),
-                    is_clockwise: !helix_result.ccw,
+                        is_clockwise: !helix_result.ccw,
-                    length: length.map(LengthUnit),
+                        length: length.map(LengthUnit),
-                    revolutions,
+                        revolutions,
-                    start_angle: Angle::from_degrees(angle_start),
+                        start_angle: Angle::from_degrees(angle_start),
-                    edge_id,
+                        edge_id,
-                }),
+                    }),
-            )
+                )
-            .await?;
+                .await?;
-        }
+            }
-    };
+        };
+    }
 
     Ok(helix_result)
 }
-
-/// Data for helix revolutions.
-#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
-#[ts(export)]
-pub struct HelixRevolutionsData {
-    /// Number of revolutions.
-    pub revolutions: f64,
-    /// Start angle (in degrees).
-    #[serde(rename = "angleStart")]
-    pub angle_start: f64,
-    /// Is the helix rotation counter clockwise?
-    /// The default is `false`.
-    #[serde(default)]
-    pub ccw: bool,
-    /// Length of the helix. If this argument is not provided, the height of
-    /// the solid is used.
-    pub length: Option<f64>,
-}
-
-/// Create a helix on a cylinder.
-pub async fn helix_revolutions(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
-    let (data, solid): (HelixRevolutionsData, Box<Solid>) = args.get_data_and_solid(exec_state)?;
-
-    let value = inner_helix_revolutions(data, solid, exec_state, args).await?;
-    Ok(KclValue::Solid { value })
-}
-
-/// Create a helix on a cylinder.
-///
-/// ```no_run
-/// part001 = startSketchOn('XY')
-///   |> circle( center= [5, 5], radius= 10 )
-///   |> extrude(length = 10)
-///   |> helixRevolutions({
-///     angleStart = 0,
-///     ccw = true,
-///     revolutions = 16,
-///  }, %)
-/// ```
-#[stdlib {
-    name = "helixRevolutions",
-    feature_tree_operation = true,
-}]
-async fn inner_helix_revolutions(
-    data: HelixRevolutionsData,
-    solid: Box<Solid>,
-    exec_state: &mut ExecState,
-    args: Args,
-) -> Result<Box<Solid>, KclError> {
-    let id = exec_state.next_uuid();
-    args.batch_modeling_cmd(
-        id,
-        ModelingCmd::from(mcmd::EntityMakeHelix {
-            cylinder_id: solid.id,
-            is_clockwise: !data.ccw,
-            length: LengthUnit(data.length.unwrap_or(solid.height)),
-            revolutions: data.revolutions,
-            start_angle: Angle::from_degrees(data.angle_start),
-        }),
-    )
-    .await?;
-
-    Ok(solid)
-}

rust/kcl-lib/src/std/mod.rs

@@ -114,7 +114,6 @@ lazy_static! {
         Box::new(crate::std::edge::GetPreviousAdjacentEdge),
         Box::new(crate::std::edge::GetCommonEdge),
         Box::new(crate::std::helix::Helix),
-        Box::new(crate::std::helix::HelixRevolutions),
         Box::new(crate::std::shell::Shell),
         Box::new(crate::std::shell::Hollow),
         Box::new(crate::std::revolve::Revolve),

rust/kcl-lib/tests/helix_ccw/ast.snap

@@ -168,106 +168,99 @@ description: Result of parsing helix_ccw.kcl
               {
                 "arguments": [
                   {
-                    "commentStart": 120,
+                    "type": "LabeledArg",
-                    "end": 0,
+                    "label": {
-                    "properties": [
+                      "commentStart": 117,
-                      {
+                      "end": 0,
-                        "commentStart": 129,
+                      "name": "revolutions",
-                        "end": 0,
+                      "start": 0,
-                        "key": {
+                      "type": "Identifier"
-                          "commentStart": 129,
+                    },
-                          "end": 0,
+                    "arg": {
-                          "name": "revolutions",
+                      "commentStart": 131,
-                          "start": 0,
+                      "end": 0,
-                          "type": "Identifier"
+                      "raw": "16",
-                        },
+                      "start": 0,
-                        "start": 0,
+                      "type": "Literal",
-                        "type": "ObjectProperty",
+                      "type": "Literal",
-                        "value": {
+                      "value": {
-                          "commentStart": 143,
+                        "value": 16.0,
-                          "end": 0,
+                        "suffix": "None"
-                          "raw": "16",
-                          "start": 0,
-                          "type": "Literal",
-                          "type": "Literal",
-                          "value": {
-                            "value": 16.0,
-                            "suffix": "None"
-                          }
-                        }
-                      },
-                      {
-                        "commentStart": 154,
-                        "end": 0,
-                        "key": {
-                          "commentStart": 154,
-                          "end": 0,
-                          "name": "angleStart",
-                          "start": 0,
-                          "type": "Identifier"
-                        },
-                        "start": 0,
-                        "type": "ObjectProperty",
-                        "value": {
-                          "commentStart": 167,
-                          "end": 0,
-                          "raw": "0",
-                          "start": 0,
-                          "type": "Literal",
-                          "type": "Literal",
-                          "value": {
-                            "value": 0.0,
-                            "suffix": "None"
-                          }
-                        }
-                      },
-                      {
-                        "commentStart": 177,
-                        "end": 0,
-                        "key": {
-                          "commentStart": 177,
-                          "end": 0,
-                          "name": "ccw",
-                          "start": 0,
-                          "type": "Identifier"
-                        },
-                        "start": 0,
-                        "type": "ObjectProperty",
-                        "value": {
-                          "commentStart": 183,
-                          "end": 0,
-                          "raw": "true",
-                          "start": 0,
-                          "type": "Literal",
-                          "type": "Literal",
-                          "value": true
-                        }
                       }
-                    ],
+                    }
-                    "start": 0,
-                    "type": "ObjectExpression",
-                    "type": "ObjectExpression"
                   },
                   {
-                    "commentStart": 196,
+                    "type": "LabeledArg",
-                    "end": 0,
+                    "label": {
-                    "start": 0,
+                      "commentStart": 142,
-                    "type": "PipeSubstitution",
+                      "end": 0,
-                    "type": "PipeSubstitution"
+                      "name": "angleStart",
+                      "start": 0,
+                      "type": "Identifier"
+                    },
+                    "arg": {
+                      "commentStart": 155,
+                      "end": 0,
+                      "raw": "0",
+                      "start": 0,
+                      "type": "Literal",
+                      "type": "Literal",
+                      "value": {
+                        "value": 0.0,
+                        "suffix": "None"
+                      }
+                    }
+                  },
+                  {
+                    "type": "LabeledArg",
+                    "label": {
+                      "commentStart": 165,
+                      "end": 0,
+                      "name": "ccw",
+                      "start": 0,
+                      "type": "Identifier"
+                    },
+                    "arg": {
+                      "commentStart": 171,
+                      "end": 0,
+                      "raw": "true",
+                      "start": 0,
+                      "type": "Literal",
+                      "type": "Literal",
+                      "value": true
+                    }
+                  },
+                  {
+                    "type": "LabeledArg",
+                    "label": {
+                      "commentStart": 184,
+                      "end": 0,
+                      "name": "cylinder",
+                      "start": 0,
+                      "type": "Identifier"
+                    },
+                    "arg": {
+                      "commentStart": 195,
+                      "end": 0,
+                      "start": 0,
+                      "type": "PipeSubstitution",
+                      "type": "PipeSubstitution"
+                    }
                   }
                 ],
                 "callee": {
                   "commentStart": 103,
                   "end": 0,
-                  "name": "helixRevolutions",
+                  "name": "helix",
                   "start": 0,
                   "type": "Identifier"
                 },
                 "commentStart": 103,
                 "end": 0,
                 "start": 0,
-                "type": "CallExpression",
+                "type": "CallExpressionKw",
-                "type": "CallExpression"
+                "type": "CallExpressionKw",
+                "unlabeled": null
               }
             ],
             "commentStart": 10,

rust/kcl-lib/tests/helix_ccw/input.kcl

@@ -1,8 +1,8 @@
 part001 = startSketchOn('XY')
   |> circle(center = [5, 5], radius = 10)
   |> extrude(length = 10)
-  |> helixRevolutions({
+  |> helix(
        revolutions = 16,
        angleStart = 0,
-       ccw = true
+       ccw = true,
-     }, %)
+       cylinder = %)

rust/kcl-lib/tests/helix_ccw/ops.snap

@@ -52,45 +52,30 @@ description: Operations executed helix_ccw.kcl
   },
   {
     "labeledArgs": {
-      "data": {
+      "angleStart": {
         "value": {
-          "type": "Object",
+          "type": "Number",
-          "value": {
+          "value": 0.0,
-            "angleStart": {
+          "ty": {
-              "type": "Number",
+            "type": "Default",
-              "value": 0.0,
+            "len": {
-              "ty": {
+              "type": "Mm"
-                "type": "Default",
-                "len": {
-                  "type": "Mm"
-                },
-                "angle": {
-                  "type": "Degrees"
-                }
-              }
             },
-            "ccw": {
+            "angle": {
-              "type": "Bool",
+              "type": "Degrees"
-              "value": true
-            },
-            "revolutions": {
-              "type": "Number",
-              "value": 16.0,
-              "ty": {
-                "type": "Default",
-                "len": {
-                  "type": "Mm"
-                },
-                "angle": {
-                  "type": "Degrees"
-                }
-              }
             }
           }
         },
         "sourceRange": []
       },
-      "solid": {
+      "ccw": {
+        "value": {
+          "type": "Bool",
+          "value": true
+        },
+        "sourceRange": []
+      },
+      "cylinder": {
         "value": {
           "type": "Solid",
           "value": {
@@ -98,11 +83,35 @@ description: Operations executed helix_ccw.kcl
           }
         },
         "sourceRange": []
+      },
+      "revolutions": {
+        "value": {
+          "type": "Number",
+          "value": 16.0,
+          "ty": {
+            "type": "Default",
+            "len": {
+              "type": "Mm"
+            },
+            "angle": {
+              "type": "Degrees"
+            }
+          }
+        },
+        "sourceRange": []
       }
     },
-    "name": "helixRevolutions",
+    "name": "helix",
     "sourceRange": [],
     "type": "StdLibCall",
-    "unlabeledArg": null
+    "unlabeledArg": {
+      "value": {
+        "type": "Solid",
+        "value": {
+          "artifactId": "[uuid]"
+        }
+      },
+      "sourceRange": []
+    }
   }
 ]

rust/kcl-lib/tests/helix_ccw/program_memory.snap

@@ -4,106 +4,14 @@ description: Variables in memory after executing helix_ccw.kcl
 ---
 {
   "part001": {
-    "type": "Solid",
+    "type": "Helix",
     "value": {
-      "type": "Solid",
+      "value": "[uuid]",
-      "id": "[uuid]",
       "artifactId": "[uuid]",
-      "value": [
+      "revolutions": 16.0,
-        {
+      "angleStart": 0.0,
-          "faceId": "[uuid]",
+      "ccw": true,
-          "id": "[uuid]",
+      "cylinderId": "[uuid]",
-          "sourceRange": [],
-          "tag": null,
-          "type": "extrudeArc"
-        }
-      ],
-      "sketch": {
-        "type": "Sketch",
-        "id": "[uuid]",
-        "paths": [
-          {
-            "__geoMeta": {
-              "id": "[uuid]",
-              "sourceRange": []
-            },
-            "ccw": true,
-            "center": [
-              5.0,
-              5.0
-            ],
-            "from": [
-              15.0,
-              5.0
-            ],
-            "radius": 10.0,
-            "tag": null,
-            "to": [
-              15.0,
-              5.0
-            ],
-            "type": "Circle",
-            "units": {
-              "type": "Mm"
-            }
-          }
-        ],
-        "on": {
-          "type": "plane",
-          "id": "[uuid]",
-          "artifactId": "[uuid]",
-          "value": "XY",
-          "origin": {
-            "x": 0.0,
-            "y": 0.0,
-            "z": 0.0
-          },
-          "xAxis": {
-            "x": 1.0,
-            "y": 0.0,
-            "z": 0.0
-          },
-          "yAxis": {
-            "x": 0.0,
-            "y": 1.0,
-            "z": 0.0
-          },
-          "zAxis": {
-            "x": 0.0,
-            "y": 0.0,
-            "z": 1.0
-          },
-          "units": {
-            "type": "Mm"
-          }
-        },
-        "start": {
-          "from": [
-            15.0,
-            5.0
-          ],
-          "to": [
-            15.0,
-            5.0
-          ],
-          "units": {
-            "type": "Mm"
-          },
-          "tag": null,
-          "__geoMeta": {
-            "id": "[uuid]",
-            "sourceRange": []
-          }
-        },
-        "artifactId": "[uuid]",
-        "originalId": "[uuid]",
-        "units": {
-          "type": "Mm"
-        }
-      },
-      "height": 10.0,
-      "startCapId": "[uuid]",
-      "endCapId": "[uuid]",
       "units": {
         "type": "Mm"
       }

rust/kcl-lib/tests/helix_ccw/unparsed.snap

@@ -5,8 +5,9 @@ description: Result of unparsing helix_ccw.kcl
 part001 = startSketchOn(XY)
   |> circle(center = [5, 5], radius = 10)
   |> extrude(length = 10)
-  |> helixRevolutions({
+  |> helix(
        revolutions = 16,
        angleStart = 0,
-       ccw = true
+       ccw = true,
-     }, %)
+       cylinder = %,
+     )

rust/kcl-lib/tests/helix_simple/program_memory.snap

@@ -16,6 +16,7 @@ description: Variables in memory after executing helix_simple.kcl
       "revolutions": 5.0,
       "angleStart": 0.0,
       "ccw": true,
+      "cylinderId": null,
       "units": {
         "type": "Mm"
       }