add optional tag to circle (#1669)

docs/kcl/std.md

@@ -2654,14 +2654,6 @@ arc(data: ArcData, sketch_group: SketchGroup) -> SketchGroup
 	// The tag.
 	tag: string,
 } |
-{
-	// The end angle.
-	angle_end: number,
-	// The start angle.
-	angle_start: number,
-	// The radius.
-	radius: number,
-} |
 {
 	// The center.
 	center: [number, number],
@@ -2671,14 +2663,6 @@ arc(data: ArcData, sketch_group: SketchGroup) -> SketchGroup
 	tag: string,
 	// The to point.
 	to: [number, number],
-} |
-{
-	// The center.
-	center: [number, number],
-	// The radius.
-	radius: number,
-	// The to point.
-	to: [number, number],
 }
 ```
 * `sketch_group`: `SketchGroup` - A sketch group is a collection of paths. (REQUIRED)
@@ -3078,14 +3062,6 @@ bezierCurve(data: BezierData, sketch_group: SketchGroup) -> SketchGroup
 	tag: string,
 	// The to point.
 	to: [number, number],
-} |
-{
-	// The first control point.
-	control1: [number, number],
-	// The second control point.
-	control2: [number, number],
-	// The to point.
-	to: [number, number],
 }
 ```
 * `sketch_group`: `SketchGroup` - A sketch group is a collection of paths. (REQUIRED)
@@ -3449,171 +3425,67 @@ Sketch a circle on the given plane
 
 
 ```
-circle(center: [number, number], radius: number, surface: SketchSurface) -> SketchGroup
+circle(center: [number, number], radius: number, surface: SketchSurface, tag?: String) -> SketchGroup
 ```
 
 #### Arguments
 
 * `center`: `[number, number]` (REQUIRED)
 * `radius`: `number` (REQUIRED)
-* `surface`: `SketchSurface` - Data for start sketch on. You can start a sketch on a plane or an extrude group. (REQUIRED)
+* `surface`: `SketchSurface` - A sketch group type. (REQUIRED)
 ```
-"XY" |
-"-XY" |
-"XZ" |
-"-XZ" |
-"YZ" |
-"-YZ" |
 {
-	plane: {
+	// The id of the plane.
+	id: uuid,
 	// Origin of the plane.
 	origin: {
 	x: number,
 	y: number,
 	z: number,
 },
+	type: "plane",
+	// Type for a plane.
+	value: "XY" | "XZ" | "YZ" | "Custom",
 	// What should the plane’s X axis be?
-	x_axis: {
-	x: number,
-	y: number,
-	z: number,
-},
-	// What should the plane’s Y axis be?
-	y_axis: {
-	x: number,
-	y: number,
-	z: number,
-},
-	// The z-axis (normal).
-	z_axis: {
-	x: number,
-	y: number,
-	z: number,
-},
-},
-} |
-{
-	// The id of the extrusion end cap
-	endCapId: uuid,
-	// The height of the extrude group.
-	height: number,
-	// The id of the extrude group.
-	id: uuid,
-	// The position of the extrude group.
-	position: [number, number, number],
-	// The rotation of the extrude group.
-	rotation: [number, number, number, number],
-	// The sketch group paths.
-	sketchGroupValues: [{
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "ToPoint",
-} |
-{
-	// arc's direction
-	ccw: string,
-	// the arc's center
-	center: [number, number],
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "TangentialArcTo",
-} |
-{
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "TangentialArc",
-} |
-{
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "Horizontal",
-	// The x coordinate.
-	x: number,
-} |
-{
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "AngledLineTo",
-	// The x coordinate.
-	x: number,
-	// The y coordinate.
-	y: number,
-} |
-{
-	// The from point.
-	from: [number, number],
-	// The name of the path.
-	name: string,
-	// The to point.
-	to: [number, number],
-	type: "Base",
-}],
-	// The id of the extrusion start cap
-	startCapId: uuid,
-	// The extrude surfaces.
-	value: [{
-	// The face id for the extrude plane.
-	faceId: uuid,
-	// The id of the geometry.
-	id: uuid,
-	// The name.
-	name: string,
-	// The position.
-	position: [number, number, number],
-	// The rotation.
-	rotation: [number, number, number, number],
-	// The source range.
-	sourceRange: [number, number],
-	type: "extrudePlane",
-} |
-{
-	// The face id for the extrude plane.
-	faceId: uuid,
-	// The id of the geometry.
-	id: uuid,
-	// The name.
-	name: string,
-	// The position.
-	position: [number, number, number],
-	// The rotation.
-	rotation: [number, number, number, number],
-	// The source range.
-	sourceRange: [number, number],
-	type: "extrudeArc",
-}],
-	// The x-axis of the extrude group base plane in the 3D space
 	xAxis: {
 	x: number,
 	y: number,
 	z: number,
 },
-	// The y-axis of the extrude group base plane in the 3D space
+	// What should the plane’s Y axis be?
 	yAxis: {
 	x: number,
 	y: number,
 	z: number,
 },
-	// The z-axis of the extrude group base plane in the 3D space
+	// The z-axis (normal).
+	zAxis: {
+	x: number,
+	y: number,
+	z: number,
+},
+} |
+{
+	// The id of the face.
+	id: uuid,
+	// The original sketch group id of the object we are sketching on.
+	sketchGroupId: uuid,
+	type: "face",
+	// The tag of the face.
+	value: string,
+	// What should the face’s X axis be?
+	xAxis: {
+	x: number,
+	y: number,
+	z: number,
+},
+	// What should the face’s Y axis be?
+	yAxis: {
+	x: number,
+	y: number,
+	z: number,
+},
+	// The z-axis (normal).
 	zAxis: {
 	x: number,
 	y: number,
@@ -3621,6 +3493,7 @@ circle(center: [number, number], radius: number, surface: SketchSurface) -> Sket
 },
 }
 ```
+* `tag`: `String` (OPTIONAL)
 
 #### Returns
 

src/wasm-lib/kcl/src/ast/types.rs

@@ -1084,11 +1084,17 @@ impl CallExpression {
             }
             FunctionKind::Std(func) => {
                 let function_expression = func.function();
-                if fn_args.len() != function_expression.params.len() {
+                let parts = function_expression.clone().into_parts().map_err(|e| {
+                    KclError::Semantic(KclErrorDetails {
+                        message: format!("Error getting parts of function: {}", e),
+                        source_ranges: vec![self.into()],
+                    })
+                })?;
+                if fn_args.len() < parts.params_required.len() || fn_args.len() > function_expression.params.len() {
                     return Err(KclError::Semantic(KclErrorDetails {
                         message: format!(
                             "this function expected {} arguments, got {}",
-                            function_expression.params.len(),
+                            parts.params_required.len(),
                             fn_args.len(),
                         ),
                         source_ranges: vec![self.into()],
@@ -1097,13 +1103,28 @@ impl CallExpression {
 
                 // Add the arguments to the memory.
                 let mut fn_memory = memory.clone();
-                for (index, param) in function_expression.params.iter().enumerate() {
+                for (index, param) in parts.params_required.iter().enumerate() {
                     fn_memory.add(
                         &param.identifier.name,
                         fn_args.get(index).unwrap().clone(),
                         param.identifier.clone().into(),
                     )?;
                 }
+                // Add the optional arguments to the memory.
+                for (index, param) in parts.params_optional.iter().enumerate() {
+                    if let Some(arg) = fn_args.get(index + parts.params_required.len()) {
+                        fn_memory.add(&param.identifier.name, arg.clone(), param.identifier.clone().into())?;
+                    } else {
+                        fn_memory.add(
+                            &param.identifier.name,
+                            MemoryItem::UserVal(UserVal {
+                                value: serde_json::value::Value::Null,
+                                meta: Default::default(),
+                            }),
+                            param.identifier.clone().into(),
+                        )?;
+                    }
+                }
 
                 // Call the stdlib function
                 let p = func.function().clone().body;

src/wasm-lib/kcl/src/std/shapes.rs

@@ -8,12 +8,13 @@ use crate::{
 };
 
 pub const CIRCLE_FN: &str = r#"
-(center, radius, surface) => {
+(center, radius, surface, tag?) => {
 const sg = startProfileAt([center[0] + radius, center[1]], surface)
     |> arc({
        angle_end: 360,
        angle_start: 0,
-       radius: radius
+       radius: radius,
+       tag: tag
      }, %)
     |> close(%)
   return sg
@@ -89,10 +90,18 @@ impl StdLibFn for Circle {
                     args.push(crate::docs::StdLibFnArg {
                         name: parameter.identifier.name.to_owned(),
                         type_: "SketchSurface".to_string(),
-                        schema: <crate::std::sketch::SketchData>::json_schema(&mut generator),
+                        schema: <crate::executor::SketchSurface>::json_schema(&mut generator),
                         required: true,
                     });
                 }
+                "tag" => {
+                    args.push(crate::docs::StdLibFnArg {
+                        name: parameter.identifier.name.to_owned(),
+                        type_: "String".to_string(),
+                        schema: <String>::json_schema(&mut generator),
+                        required: false,
+                    });
+                }
                 _ => panic!("Unknown parameter: {:?}", parameter.identifier.name),
             }
         }

src/wasm-lib/kcl/src/std/sketch.rs

@@ -1130,18 +1130,7 @@ async fn inner_close(
 #[ts(export)]
 #[serde(rename_all = "camelCase", untagged)]
 pub enum ArcData {
-    /// Angles and radius with a tag.
+    /// Angles and radius with an optional tag.
-    AnglesAndRadiusWithTag {
-        /// The start angle.
-        angle_start: f64,
-        /// The end angle.
-        angle_end: f64,
-        /// The radius.
-        radius: f64,
-        /// The tag.
-        tag: String,
-    },
-    /// Angles and radius.
     AnglesAndRadius {
         /// The start angle.
         angle_start: f64,
@@ -1149,19 +1138,11 @@ pub enum ArcData {
         angle_end: f64,
         /// The radius.
         radius: f64,
-    },
-    /// Center, to and radius with a tag.
-    CenterToRadiusWithTag {
-        /// The center.
-        center: [f64; 2],
-        /// The to point.
-        to: [f64; 2],
-        /// The radius.
-        radius: f64,
         /// The tag.
-        tag: String,
+        #[serde(default)]
+        tag: Option<String>,
     },
-    /// Center, to and radius.
+    /// Center, to and radius with an optional tag.
     CenterToRadius {
         /// The center.
         center: [f64; 2],
@@ -1169,6 +1150,9 @@ pub enum ArcData {
         to: [f64; 2],
         /// The radius.
         radius: f64,
+        /// The tag.
+        #[serde(default)]
+        tag: Option<String>,
     },
 }
 
@@ -1188,7 +1172,7 @@ async fn inner_arc(data: ArcData, sketch_group: Box<SketchGroup>, args: Args) ->
     let from: Point2d = sketch_group.get_coords_from_paths()?;
 
     let (center, angle_start, angle_end, radius, end) = match &data {
-        ArcData::AnglesAndRadiusWithTag {
+        ArcData::AnglesAndRadius {
             angle_start,
             angle_end,
             radius,
@@ -1199,21 +1183,7 @@ async fn inner_arc(data: ArcData, sketch_group: Box<SketchGroup>, args: Args) ->
             let (center, end) = arc_center_and_end(from, a_start, a_end, *radius);
             (center, a_start, a_end, *radius, end)
         }
-        ArcData::AnglesAndRadius {
+        ArcData::CenterToRadius { center, to, radius, .. } => {
-            angle_start,
-            angle_end,
-            radius,
-        } => {
-            let a_start = Angle::from_degrees(*angle_start);
-            let a_end = Angle::from_degrees(*angle_end);
-            let (center, end) = arc_center_and_end(from, a_start, a_end, *radius);
-            (center, a_start, a_end, *radius, end)
-        }
-        ArcData::CenterToRadiusWithTag { center, to, radius, .. } => {
-            let (angle_start, angle_end) = arc_angles(from, center.into(), to.into(), *radius, args.source_range)?;
-            (center.into(), angle_start, angle_end, *radius, to.into())
-        }
-        ArcData::CenterToRadius { center, to, radius } => {
             let (angle_start, angle_end) = arc_angles(from, center.into(), to.into(), *radius, args.source_range)?;
             (center.into(), angle_start, angle_end, *radius, to.into())
         }
@@ -1241,10 +1211,8 @@ async fn inner_arc(data: ArcData, sketch_group: Box<SketchGroup>, args: Args) ->
             from: from.into(),
             to: end.into(),
             name: match data {
-                ArcData::AnglesAndRadiusWithTag { tag, .. } => tag.to_string(),
+                ArcData::AnglesAndRadius { tag, .. } => tag.unwrap_or_default().to_string(),
-                ArcData::AnglesAndRadius { .. } => "".to_string(),
+                ArcData::CenterToRadius { tag, .. } => tag.unwrap_or_default().to_string(),
-                ArcData::CenterToRadiusWithTag { tag, .. } => tag.to_string(),
-                ArcData::CenterToRadius { .. } => "".to_string(),
             },
             geo_meta: GeoMeta {
                 id,
@@ -1456,28 +1424,16 @@ async fn inner_tangential_arc_to(
 /// Data to draw a bezier curve.
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
 #[ts(export)]
-#[serde(rename_all = "camelCase", untagged)]
+#[serde(rename_all = "camelCase")]
-pub enum BezierData {
+pub struct BezierData {
-    /// Points with a tag.
+    /// The to point.
-    PointsWithTag {
+    to: [f64; 2],
-        /// The to point.
+    /// The first control point.
-        to: [f64; 2],
+    control1: [f64; 2],
-        /// The first control point.
+    /// The second control point.
-        control1: [f64; 2],
+    control2: [f64; 2],
-        /// The second control point.
+    /// The tag.
-        control2: [f64; 2],
+    tag: Option<String>,
-        /// The tag.
-        tag: String,
-    },
-    /// Points.
-    Points {
-        /// The to point.
-        to: [f64; 2],
-        /// The first control point.
-        control1: [f64; 2],
-        /// The second control point.
-        control2: [f64; 2],
-    },
 }
 
 /// Draw a bezier curve.
@@ -1499,16 +1455,9 @@ async fn inner_bezier_curve(
 ) -> Result<Box<SketchGroup>, KclError> {
     let from = sketch_group.get_coords_from_paths()?;
 
-    let (to, control1, control2) = match &data {
-        BezierData::PointsWithTag {
-            to, control1, control2, ..
-        } => (to, control1, control2),
-        BezierData::Points { to, control1, control2 } => (to, control1, control2),
-    };
-
     let relative = true;
-    let delta = to;
+    let delta = data.to;
-    let to = [from.x + to[0], from.y + to[1]];
+    let to = [from.x + data.to[0], from.y + data.to[1]];
 
     let id = uuid::Uuid::new_v4();
 
@@ -1518,13 +1467,13 @@ async fn inner_bezier_curve(
             path: sketch_group.id,
             segment: kittycad::types::PathSegment::Bezier {
                 control1: Point3D {
-                    x: control1[0],
+                    x: data.control1[0],
-                    y: control1[1],
+                    y: data.control1[1],
                     z: 0.0,
                 },
                 control2: Point3D {
-                    x: control2[0],
+                    x: data.control2[0],
-                    y: control2[1],
+                    y: data.control2[1],
                     z: 0.0,
                 },
                 end: Point3D {
@@ -1542,11 +1491,7 @@ async fn inner_bezier_curve(
         base: BasePath {
             from: from.into(),
             to,
-            name: if let BezierData::PointsWithTag { tag, .. } = data {
+            name: data.tag.unwrap_or_default().to_string(),
-                tag.to_string()
-            } else {
-                "".to_string()
-            },
             geo_meta: GeoMeta {
                 id,
                 metadata: args.source_range.into(),

src/wasm-lib/tests/executor/main.rs

@@ -1290,3 +1290,29 @@ const part002 = startSketchOn(part001, "end")
         .unwrap();
     twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face_circle.png", &result, 1.0);
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn serial_test_sketch_on_face_circle_tagged() {
+    let code = r#"fn cube = (pos, scale) => {
+  const sg = startSketchOn('XY')
+    |> startProfileAt(pos, %)
+    |> line([0, scale], %)
+    |> line([scale, 0], %)
+    |> line([0, -scale], %)
+
+  return sg
+}
+const part001 = cube([0,0], 20)
+    |> close(%)
+    |> extrude(20, %)
+
+const part002 = startSketchOn(part001, "end")
+  |> circle([0, 0], 5, %, "myCircle") 
+  |> extrude(5, %)
+"#;
+
+    let result = execute_and_snapshot(code, kittycad::types::UnitLength::Mm)
+        .await
+        .unwrap();
+    twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face_circle_tagged.png", &result, 1.0);
+}