Add fixme to a file-tree test (#5941)

* Enable tests to run on pushes to 'all-e2e' branch

* Document branch behavior

.github/workflows/e2e-tests.yml

@@ -1,7 +1,9 @@
 name: E2E Tests
 on:
   push:
-    branches: [ main ]
+    branches:
+      - main
+      - all-e2e # this bypasses `fixme()` using `orRunWhenFullSuiteEnabled()`
   pull_request:
   schedule:
     - cron: 0 * * * *  # hourly
@@ -15,7 +17,6 @@ permissions:
   pull-requests: write
   actions: read
 
-
 jobs:
 
   conditions:
@@ -67,14 +68,16 @@ jobs:
       - name: Display conditions
         shell: bash
         run: |
-          # For debugging purposes.
+          # For debugging purposes
           set -euo pipefail
+          echo "GITHUB_REF: $GITHUB_REF"
+          echo "GITHUB_HEAD_REF: $GITHUB_HEAD_REF"
+          echo "GITHUB_BASE_REF: $GITHUB_BASE_REF"
           echo "significant: ${{ steps.path-changes.outputs.significant }}"
           echo "should-run: ${{ steps.should-run.outputs.should-run }}"
 
-
   prepare-wasm:
-    # seperate job on Ubuntu to build or fetch the wasm blob once on the fastest runner
+    # separate job on Ubuntu to build or fetch the wasm blob once on the fastest runner
     runs-on: runs-on=${{ github.run_id }}/family=i7ie.2xlarge/image=ubuntu22-full-x64
     needs: conditions
     steps:
@@ -159,7 +162,6 @@ jobs:
           name: prepared-wasm
           path: |
             rust/kcl-wasm-lib/pkg/kcl_wasm_lib*
-  
 
   snapshots:
     name: playwright:snapshots:ubuntu
@@ -243,7 +245,7 @@ jobs:
           retention-days: 30
           overwrite: true
 
-      - name: check for changes
+      - name: Check for changes
         if: ${{ needs.conditions.outputs.should-run == 'true' && github.ref != 'refs/heads/main' }}
         shell: bash
         id: git-check

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 |
 
 

e2e/playwright/file-tree.spec.ts

@@ -15,6 +15,7 @@ test.describe('integrations tests', () => {
     'Creating a new file or switching file while in sketchMode should exit sketchMode',
     { tag: '@electron' },
     async ({ page, context, homePage, scene, editor, toolbar, cmdBar }) => {
+      test.fixme(orRunWhenFullSuiteEnabled())
       await context.folderSetupFn(async (dir) => {
         const bracketDir = join(dir, 'test-sample')
         await fsp.mkdir(bracketDir, { recursive: true })

e2e/playwright/test-utils.ts

@@ -56,7 +56,8 @@ export const editorSelector = '[role="textbox"][data-language="kcl"]'
 type PaneId = 'variables' | 'code' | 'files' | 'logs'
 
 export function orRunWhenFullSuiteEnabled() {
-  return process.env.GITHUB_HEAD_REF !== 'all-e2e'
+  const branch = process.env.GITHUB_REF?.replace('refs/heads/', '')
+  return branch !== 'all-e2e'
 }
 
 async function waitForPageLoadWithRetry(page: Page) {

e2e/playwright/testing-settings.spec.ts

@@ -746,6 +746,7 @@ test.describe('Testing settings', () => {
     })
     await page.setBodyDimensions({ width: 1200, height: 500 })
     await homePage.goToModelingScene()
+    await expect(toolbar.startSketchBtn).toBeEnabled({ timeout: 15_000 })
     await scene.settled(cmdBar)
     await page.waitForTimeout(1000)
 

rust/Cargo.lock

@@ -1780,7 +1780,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-bumper"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "anyhow",
  "clap",
@@ -1791,7 +1791,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-derive-docs"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "Inflector",
  "anyhow",
@@ -1810,7 +1810,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-directory-test-macro"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1819,7 +1819,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-language-server"
-version = "0.2.52"
+version = "0.2.53"
 dependencies = [
  "anyhow",
  "clap",
@@ -1840,7 +1840,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-language-server-release"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "anyhow",
  "clap",
@@ -1860,7 +1860,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-lib"
-version = "0.2.52"
+version = "0.2.53"
 dependencies = [
  "anyhow",
  "approx 0.5.1",
@@ -1928,7 +1928,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-python-bindings"
-version = "0.3.52"
+version = "0.3.53"
 dependencies = [
  "anyhow",
  "kcl-lib",
@@ -1943,7 +1943,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-test-server"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "anyhow",
  "hyper 0.14.32",
@@ -1956,7 +1956,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-to-core"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1970,7 +1970,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-wasm-lib"
-version = "0.1.52"
+version = "0.1.53"
 dependencies = [
  "bson",
  "console_error_panic_hook",

rust/kcl-bumper/Cargo.toml

@@ -1,7 +1,7 @@
 
 [package]
 name = "kcl-bumper"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 repository = "https://github.com/KittyCAD/modeling-api"
 rust-version = "1.76"

rust/kcl-derive-docs/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-derive-docs"
 description = "A tool for generating documentation from Rust derive macros"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

rust/kcl-directory-test-macro/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-directory-test-macro"
 description = "A tool for generating tests from a directory of kcl files"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

rust/kcl-language-server-release/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "kcl-language-server-release"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 authors = ["KittyCAD Inc <kcl@kittycad.io>"]
 publish = false

rust/kcl-language-server/Cargo.toml

@@ -2,7 +2,7 @@
 name = "kcl-language-server"
 description = "A language server for KCL."
 authors = ["KittyCAD Inc <kcl@kittycad.io>"]
-version = "0.2.52"
+version = "0.2.53"
 edition = "2021"
 license = "MIT"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

rust/kcl-lib/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-lib"
 description = "KittyCAD Language implementation and tools"
-version = "0.2.52"
+version = "0.2.53"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

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() {
-        let helix_fn: Box<dyn StdLibFn> = Box::new(crate::std::helix::HelixRevolutions);
-        let snippet = helix_fn.to_autocomplete_snippet().unwrap();
+    fn get_autocomplete_snippet_union() {
+        let union_fn: Box<dyn StdLibFn> = Box::new(crate::std::csg::Union);
+        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({
-	revolutions = ${0:3.14},
-	angleStart = ${1:3.14},
-	ccw = ${2:false},
-}, ${3:%})${}"#
+            r#"getCommonEdge(faces = [{
+	value = ${0:"string"},
+}])${}"#
         );
     }
 

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/parsing/mod.rs

@@ -60,11 +60,8 @@ pub fn parse_tokens(mut tokens: TokenStream) -> ParseResult {
         return Node::<Program>::default().into();
     }
 
-    // Check all the tokens are whitespace or comments.
-    if tokens
-        .iter()
-        .all(|t| t.token_type.is_whitespace() || t.token_type.is_comment())
-    {
+    // Check all the tokens are whitespace.
+    if tokens.iter().all(|t| t.token_type.is_whitespace()) {
         return Node::<Program>::default().into();
     }
 

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 axis = args.get_kw_arg("axis")?;
+    let radius = args.get_kw_arg_opt("radius")?;
+    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."},
-        axis = { docs = "Axis to use for the helix."},
+        radius = { docs = "Radius of the helix.", include_in_snippet = true},
+        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,
-    axis: Axis3dOrEdgeReference,
+    radius: Option<f64>,
+    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 {
-        Axis3dOrEdgeReference::Axis(axis) => {
-            let (axis, origin) = axis.axis_and_origin()?;
+    if let Some(cylinder) = cylinder {
+        args.batch_modeling_cmd(
+            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.
-            let Some(length) = length else {
-                return Err(KclError::Semantic(crate::errors::KclErrorDetails {
-                    message: "Length is required when creating a helix around an axis.".to_string(),
-                    source_ranges: vec![args.source_range],
-                }));
-            };
+                // Make sure they gave us a length.
+                let Some(length) = length else {
+                    return Err(KclError::Semantic(crate::errors::KclErrorDetails {
+                        message: "Length is required when creating a helix around an axis.".to_string(),
+                        source_ranges: vec![args.source_range],
+                    }));
+                };
 
-            args.batch_modeling_cmd(
-                id,
-                ModelingCmd::from(mcmd::EntityMakeHelixFromParams {
-                    radius: LengthUnit(radius),
-                    is_clockwise: !helix_result.ccw,
-                    length: LengthUnit(length),
-                    revolutions,
-                    start_angle: Angle::from_degrees(angle_start),
-                    axis,
-                    center: origin,
-                }),
-            )
-            .await?;
-        }
-        Axis3dOrEdgeReference::Edge(edge) => {
-            let edge_id = edge.get_engine_id(exec_state, &args)?;
+                args.batch_modeling_cmd(
+                    id,
+                    ModelingCmd::from(mcmd::EntityMakeHelixFromParams {
+                        radius: LengthUnit(radius),
+                        is_clockwise: !helix_result.ccw,
+                        length: LengthUnit(length),
+                        revolutions,
+                        start_angle: Angle::from_degrees(angle_start),
+                        axis,
+                        center: origin,
+                    }),
+                )
+                .await?;
+            }
+            Axis3dOrEdgeReference::Edge(edge) => {
+                let edge_id = edge.get_engine_id(exec_state, &args)?;
 
-            args.batch_modeling_cmd(
-                id,
-                ModelingCmd::from(mcmd::EntityMakeHelixFromEdge {
-                    radius: LengthUnit(radius),
-                    is_clockwise: !helix_result.ccw,
-                    length: length.map(LengthUnit),
-                    revolutions,
-                    start_angle: Angle::from_degrees(angle_start),
-                    edge_id,
-                }),
-            )
-            .await?;
-        }
-    };
+                args.batch_modeling_cmd(
+                    id,
+                    ModelingCmd::from(mcmd::EntityMakeHelixFromEdge {
+                        radius: LengthUnit(radius),
+                        is_clockwise: !helix_result.ccw,
+                        length: length.map(LengthUnit),
+                        revolutions,
+                        start_angle: Angle::from_degrees(angle_start),
+                        edge_id,
+                    }),
+                )
+                .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/src/unparser.rs

@@ -2039,6 +2039,15 @@ thing = 'foo'
         );
     }
 
+    #[test]
+    fn test_recast_only_line_comments() {
+        let code = r#"// comment at start
+"#;
+        let program = crate::parsing::top_level_parse(code).unwrap();
+
+        assert_eq!(program.recast(&Default::default(), 0), code);
+    }
+
     #[test]
     fn test_recast_comment_at_start() {
         let test_program = r#"

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

@@ -168,106 +168,99 @@ description: Result of parsing helix_ccw.kcl
               {
                 "arguments": [
                   {
-                    "commentStart": 120,
-                    "end": 0,
-                    "properties": [
-                      {
-                        "commentStart": 129,
-                        "end": 0,
-                        "key": {
-                          "commentStart": 129,
-                          "end": 0,
-                          "name": "revolutions",
-                          "start": 0,
-                          "type": "Identifier"
-                        },
-                        "start": 0,
-                        "type": "ObjectProperty",
-                        "value": {
-                          "commentStart": 143,
-                          "end": 0,
-                          "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
-                        }
+                    "type": "LabeledArg",
+                    "label": {
+                      "commentStart": 117,
+                      "end": 0,
+                      "name": "revolutions",
+                      "start": 0,
+                      "type": "Identifier"
+                    },
+                    "arg": {
+                      "commentStart": 131,
+                      "end": 0,
+                      "raw": "16",
+                      "start": 0,
+                      "type": "Literal",
+                      "type": "Literal",
+                      "value": {
+                        "value": 16.0,
+                        "suffix": "None"
                       }
-                    ],
-                    "start": 0,
-                    "type": "ObjectExpression",
-                    "type": "ObjectExpression"
+                    }
                   },
                   {
-                    "commentStart": 196,
-                    "end": 0,
-                    "start": 0,
-                    "type": "PipeSubstitution",
-                    "type": "PipeSubstitution"
+                    "type": "LabeledArg",
+                    "label": {
+                      "commentStart": 142,
+                      "end": 0,
+                      "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": "CallExpression"
+                "type": "CallExpressionKw",
+                "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",
-          "value": {
-            "angleStart": {
-              "type": "Number",
-              "value": 0.0,
-              "ty": {
-                "type": "Default",
-                "len": {
-                  "type": "Mm"
-                },
-                "angle": {
-                  "type": "Degrees"
-                }
-              }
+          "type": "Number",
+          "value": 0.0,
+          "ty": {
+            "type": "Default",
+            "len": {
+              "type": "Mm"
             },
-            "ccw": {
-              "type": "Bool",
-              "value": true
-            },
-            "revolutions": {
-              "type": "Number",
-              "value": 16.0,
-              "ty": {
-                "type": "Default",
-                "len": {
-                  "type": "Mm"
-                },
-                "angle": {
-                  "type": "Degrees"
-                }
-              }
+            "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",
-      "id": "[uuid]",
+      "value": "[uuid]",
       "artifactId": "[uuid]",
-      "value": [
-        {
-          "faceId": "[uuid]",
-          "id": "[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]",
+      "revolutions": 16.0,
+      "angleStart": 0.0,
+      "ccw": true,
+      "cylinderId": "[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"
       }

rust/kcl-python-bindings/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "kcl-python-bindings"
-version = "0.3.52"
+version = "0.3.53"
 edition = "2021"
 repository = "https://github.com/kittycad/modeling-app"
 

rust/kcl-test-server/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-test-server"
 description = "A test server for KCL"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 license = "MIT"
 

rust/kcl-to-core/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-to-core"
 description = "Utility methods to convert kcl to engine core executable tests"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

rust/kcl-wasm-lib/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "kcl-wasm-lib"
-version = "0.1.52"
+version = "0.1.53"
 edition = "2021"
 repository = "https://github.com/KittyCAD/modeling-app"
 rust-version = "1.83"

src/components/FileMachineProvider.tsx

@@ -34,7 +34,6 @@ import { settingsActor, useSettings } from 'machines/appMachine'
 import { createRouteCommands } from 'lib/commandBarConfigs/routeCommandConfig'
 import { useToken } from 'machines/appMachine'
 import { createNamedViewsCommand } from 'lib/commandBarConfigs/namedViewsConfig'
-import { reportRejection } from 'lib/trap'
 
 type MachineContext<T extends AnyStateMachine> = {
   state: StateFrom<T>
@@ -61,20 +60,6 @@ export const FileMachineProvider = ({
     []
   )
 
-  // Write code mirror content to disk when the page is trying to reroute
-  // Our logic for codeManager.writeToFile has an artificial 1000ms timeout which
-  // won't run quickly enough so users can make an edit, exit the page and lose their
-  // progress within that 1000ms window.
-  useEffect(() => {
-    const preventUnload = (event: BeforeUnloadEvent) => {
-      codeManager.writeToFileNoTimeout().catch(reportRejection)
-    }
-    window.addEventListener('beforeunload', preventUnload)
-    return () => {
-      window.removeEventListener('beforeunload', preventUnload)
-    }
-  }, [])
-
   useEffect(() => {
     // TODO: Engine feature is not deployed
     if (DEV) {

src/lang/codeManager.ts

@@ -164,32 +164,6 @@ export default class CodeManager {
     }
   }
 
-  // When we unload the page via changing routes we want to instantly write to disk to save their progress
-  // There is a race condition in the system. writeToFile takes 1000ms to run, if they make an edit and leave within the 1000ms
-  // window they won't get their content saved. Use this to always save their file before rerouting
-  async writeToFileNoTimeout() {
-    if (isDesktop()) {
-      return new Promise((resolve, reject) => {
-        if (!this._currentFilePath)
-          return reject(new Error('currentFilePath not set'))
-
-        // Wait one event loop to give a chance for params to be set
-        // Save the file to disk
-        window.electron
-          .writeFile(this._currentFilePath, this.code ?? '')
-          .then(resolve)
-          .catch((err: Error) => {
-            // TODO: add tracing per GH issue #254 (https://github.com/KittyCAD/modeling-app/issues/254)
-            console.error('error saving file', err)
-            toast.error('Error saving file, please check file permissions')
-            reject(err)
-          })
-      })
-    } else {
-      safeLSSetItem(PERSIST_CODE_KEY, this.code)
-    }
-  }
-
   async updateEditorWithAstAndWriteToFile(ast: Program) {
     // We clear the AST when there it cannot be parsed, so if we are trying to write an empty AST, its
     // probably because of an earlier error. That's a bad state to be in and it's not going to be