Add sectional argument and edit flow for point-and-click Sweep (#5480)

* WIP: Expose the sectional argument in the Sweep command flow Fixes #5301 * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Working edit flow * Lint * Allow in place editing, more consistent code * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Remove validation on non-selection arg * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Comment out bad test * Clean up for review * Hack sectional * Made selection args hidden * Fix edit issue in e2e * Clean up * Add face filtering filter for opposite and next adjacent faces * Lint * Fixme back * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * Use updateModelingState in codemod * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * Improve filtering readibility * Fix base test * I liked return but clippy didn't * Working tests, isolating the change to sectional sweep, don't like the api change * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 * Clean up snapshots * A snapshot a day keeps the bugs away! 📷🐛 * A snapshot a day keeps the bugs away! 📷🐛 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2025-03-20 20:42:41 -04:00
parent 12b546ea24
commit 054bb5b500
9 changed files with 332 additions and 51 deletions
--- a/e2e/playwright/point-click.spec.ts
+++ b/e2e/playwright/point-click.spec.ts
@ -1336,7 +1336,7 @@ loft001 = loft([sketch001, sketch002])
    })
  })

-  test(`Sweep point-and-click`, async ({
+  test(`Sweep point-and-click base`, async ({
    context,
    page,
    homePage,
@ -1369,7 +1369,10 @@ sketch002 = startSketchOn('XZ')
      testPoint.x - 50,
      testPoint.y
    )
-    const sweepDeclaration = 'sweep001 = sweep(sketch001, path = sketch002)'
+    const sweepDeclaration =
+      'sweep001 = sweep(sketch001, path = sketch002, sectional = false)'
+    const editedSweepDeclaration =
+      'sweep001 = sweep(sketch001, path = sketch002, sectional = true)'

    await test.step(`Look for sketch001`, async () => {
      await toolbar.closePane('code')
@ -1383,6 +1386,7 @@ sketch002 = startSketchOn('XZ')
        currentArgKey: 'target',
        currentArgValue: '',
        headerArguments: {
+          Sectional: '',
          Target: '',
          Trajectory: '',
        },
@ -1395,6 +1399,7 @@ sketch002 = startSketchOn('XZ')
        currentArgKey: 'trajectory',
        currentArgValue: '',
        headerArguments: {
+          Sectional: '',
          Target: '1 face',
          Trajectory: '',
        },
@ -1404,18 +1409,50 @@ sketch002 = startSketchOn('XZ')
      await clickOnSketch2()
      await page.waitForTimeout(500)
      await cmdBar.progressCmdBar()
-      await toolbar.openPane('code')
-      await page.waitForTimeout(500)
+      await cmdBar.expectState({
+        commandName: 'Sweep',
+        headerArguments: {
+          Target: '1 face',
+          Trajectory: '1 segment',
+          Sectional: '',
+        },
+        stage: 'review',
+      })
+      await cmdBar.progressCmdBar()
    })

    await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
-      // await scene.expectPixelColor([135, 64, 73], testPoint, 15) // FIXME
+      await toolbar.openPane('code')
      await editor.expectEditor.toContain(sweepDeclaration)
-      await editor.expectState({
-        diagnostics: [],
-        activeLines: [sweepDeclaration],
-        highlightedCode: '',
+      await toolbar.closePane('code')
    })
+
+    await test.step('Edit sweep via feature tree selection works', async () => {
+      await toolbar.openPane('feature-tree')
+      const operationButton = await toolbar.getFeatureTreeOperation('Sweep', 0)
+      await operationButton.dblclick({ button: 'left' })
+      await cmdBar.expectState({
+        commandName: 'Sweep',
+        currentArgKey: 'sectional',
+        currentArgValue: '',
+        headerArguments: {
+          Sectional: '',
+        },
+        highlightedHeaderArg: 'sectional',
+        stage: 'arguments',
+      })
+      await cmdBar.selectOption({ name: 'True' }).click()
+      await cmdBar.expectState({
+        commandName: 'Sweep',
+        headerArguments: {
+          Sectional: '',
+        },
+        stage: 'review',
+      })
+      await cmdBar.progressCmdBar()
+      await toolbar.closePane('feature-tree')
+      await toolbar.openPane('code')
+      await editor.expectEditor.toContain(editedSweepDeclaration)
      await toolbar.closePane('code')
    })

@ -1476,6 +1513,7 @@ sketch002 = startSketchOn('XZ')
        currentArgKey: 'target',
        currentArgValue: '',
        headerArguments: {
+          Sectional: '',
          Target: '',
          Trajectory: '',
        },
@ -1488,6 +1526,7 @@ sketch002 = startSketchOn('XZ')
        currentArgKey: 'trajectory',
        currentArgValue: '',
        headerArguments: {
+          Sectional: '',
          Target: '1 face',
          Trajectory: '',
        },
--- a/rust/kcl-lib/src/std/extrude.rs
+++ b/rust/kcl-lib/src/std/extrude.rs
@ -130,6 +130,7 @@ async fn inner_extrude(
                sketch,
                id.into(),
                length,
+                false,
                &NamedCapTags {
                    start: tag_start.as_ref(),
                    end: tag_end.as_ref(),
@ -154,6 +155,7 @@ pub(crate) async fn do_post_extrude<'a>(
    sketch: &Sketch,
    solid_id: ArtifactId,
    length: f64,
+    sectional: bool,
    named_cap_tags: &'a NamedCapTags<'a>,
    exec_state: &mut ExecState,
    args: &Args,
@ -201,6 +203,25 @@ pub(crate) async fn do_post_extrude<'a>(
        vec![]
    };

+    // Face filtering attempt in order to resolve https://github.com/KittyCAD/modeling-app/issues/5328
+    // In case of a sectional sweep, empirically it looks that the first n faces that are yielded from the sweep
+    // are the ones that work with GetOppositeEdge and GetNextAdjacentEdge, aka the n sides in the sweep.
+    // So here we're figuring out that n number as yielded_sides_count here,
+    // making sure that circle() calls count but close() don't (no length)
+    let count_of_first_set_of_faces_if_sectional = if sectional {
+        sketch
+            .paths
+            .iter()
+            .filter(|p| {
+                let is_circle = matches!(p, Path::Circle { .. });
+                let has_length = p.get_base().from != p.get_base().to;
+                is_circle || has_length
+            })
+            .count()
+    } else {
+        usize::MAX
+    };
+
    for (curve_id, face_id) in face_infos
        .iter()
        .filter(|face_info| face_info.cap == ExtrusionFaceCapType::None)
@ -211,6 +232,7 @@ pub(crate) async fn do_post_extrude<'a>(
                None
            }
        })
+        .take(count_of_first_set_of_faces_if_sectional)
    {
        // Batch these commands, because the Rust code doesn't actually care about the outcome.
        // So, there's no need to await them.
--- a/rust/kcl-lib/src/std/loft.rs
+++ b/rust/kcl-lib/src/std/loft.rs
@ -174,6 +174,7 @@ async fn inner_loft(
            &sketch,
            id.into(),
            0.0,
+            false,
            &super::extrude::NamedCapTags {
                start: tag_start.as_ref(),
                end: tag_end.as_ref(),
--- a/rust/kcl-lib/src/std/revolve.rs
+++ b/rust/kcl-lib/src/std/revolve.rs
@ -299,6 +299,7 @@ async fn inner_revolve(
                sketch,
                id.into(),
                0.0,
+                false,
                &super::extrude::NamedCapTags {
                    start: tag_start.as_ref(),
                    end: tag_end.as_ref(),
--- a/rust/kcl-lib/src/std/sweep.rs
+++ b/rust/kcl-lib/src/std/sweep.rs
@ -183,6 +183,7 @@ async fn inner_sweep(
                sketch,
                id.into(),
                0.0,
+                sectional.unwrap_or(false),
                &super::extrude::NamedCapTags {
                    start: tag_start.as_ref(),
                    end: tag_end.as_ref(),
--- a/src/lang/modifyAst.ts
+++ b/src/lang/modifyAst.ts
@ -515,30 +515,54 @@ export function addShell({
  }
 }

-export function addSweep(
-  node: Node<Program>,
-  profileDeclarator: VariableDeclarator,
-  pathDeclarator: VariableDeclarator
-): {
+export function addSweep({
+  node,
+  targetDeclarator,
+  trajectoryDeclarator,
+  sectional,
+  variableName,
+  insertIndex,
+}: {
+  node: Node<Program>
+  targetDeclarator: VariableDeclarator
+  trajectoryDeclarator: VariableDeclarator
+  sectional: boolean
+  variableName?: string
+  insertIndex?: number
+}): {
  modifiedAst: Node<Program>
  pathToNode: PathToNode
 } {
  const modifiedAst = structuredClone(node)
-  const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SWEEP)
-  const sweep = createCallExpressionStdLibKw(
+  const name =
+    variableName ?? findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SWEEP)
+  const call = createCallExpressionStdLibKw(
    'sweep',
-    createIdentifier(profileDeclarator.id.name),
-    [createLabeledArg('path', createIdentifier(pathDeclarator.id.name))]
+    createIdentifier(targetDeclarator.id.name),
+    [
+      createLabeledArg('path', createIdentifier(trajectoryDeclarator.id.name)),
+      createLabeledArg('sectional', createLiteral(sectional)),
+    ]
  )
-  const declaration = createVariableDeclaration(name, sweep)
-  modifiedAst.body.push(declaration)
+  const variable = createVariableDeclaration(name, call)
+  const insertAt =
+    insertIndex !== undefined
+      ? insertIndex
+      : modifiedAst.body.length
+      ? modifiedAst.body.length
+      : 0
+
+  modifiedAst.body.length
+    ? modifiedAst.body.splice(insertAt, 0, variable)
+    : modifiedAst.body.push(variable)
+  const argIndex = 0
  const pathToNode: PathToNode = [
    ['body', ''],
-    [modifiedAst.body.length - 1, 'index'],
+    [insertAt, 'index'],
    ['declaration', 'VariableDeclaration'],
    ['init', 'VariableDeclarator'],
    ['arguments', 'CallExpressionKw'],
-    [0, ARG_INDEX_FIELD],
+    [argIndex, ARG_INDEX_FIELD],
    ['arg', LABELED_ARG_FIELD],
  ]

--- a/src/lib/commandBarConfigs/modelingCommandConfig.ts
+++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts
@ -55,8 +55,12 @@ export type ModelingCommandSchema = {
    distance: KclCommandValue
  }
  Sweep: {
+    // Enables editing workflow
+    nodeToEdit?: PathToNode
+    // Arguments
    target: Selections
    trajectory: Selections
+    sectional: boolean
  }
  Loft: {
    selection: Selections
@ -357,22 +361,40 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
    description:
      'Create a 3D body by moving a sketch region along an arbitrary path.',
    icon: 'sweep',
-    needsReview: false,
+    needsReview: true,
    args: {
+      nodeToEdit: {
+        description:
+          'Path to the node in the AST to edit. Never shown to the user.',
+        skip: true,
+        inputType: 'text',
+        required: false,
+      },
      target: {
        inputType: 'selection',
        selectionTypes: ['solid2d'],
        required: true,
        skip: true,
        multiple: false,
+        hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
      },
      trajectory: {
        inputType: 'selection',
-        selectionTypes: ['segment', 'path'],
+        selectionTypes: ['segment'],
        required: true,
-        skip: false,
+        skip: true,
        multiple: false,
        validation: sweepValidator,
+        hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
+      },
+      sectional: {
+        inputType: 'options',
+        required: true,
+        options: [
+          { name: 'False', value: false },
+          { name: 'True', value: true },
+        ],
+        // No validation possible here until we have rollback
      },
    },
  },
--- a/src/lib/operations.ts
+++ b/src/lib/operations.ts
@ -311,6 +311,143 @@ const prepareToEditOffsetPlane: PrepareToEditCallback = async ({
  }
 }

+const prepareToEditSweep: PrepareToEditCallback = async ({
+  artifact,
+  operation,
+}) => {
+  const baseCommand = {
+    name: 'Sweep',
+    groupId: 'modeling',
+  }
+  if (
+    operation.type !== 'StdLibCall' ||
+    !operation.labeledArgs ||
+    !operation.unlabeledArg ||
+    !('sectional' in operation.labeledArgs) ||
+    !operation.labeledArgs.sectional
+  ) {
+    return baseCommand
+  }
+  if (!artifact || !('pathId' in artifact) || operation.type !== 'StdLibCall') {
+    return baseCommand
+  }
+
+  // We have to go a little roundabout to get from the original artifact
+  // to the solid2DId that we need to pass to the Sweep command, just like Extrude.
+  const pathArtifact = getArtifactOfTypes(
+    {
+      key: artifact.pathId,
+      types: ['path'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (
+    err(pathArtifact) ||
+    pathArtifact.type !== 'path' ||
+    !pathArtifact.solid2dId
+  ) {
+    return baseCommand
+  }
+
+  const targetArtifact = getArtifactOfTypes(
+    {
+      key: pathArtifact.solid2dId,
+      types: ['solid2d'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(targetArtifact) || targetArtifact.type !== 'solid2d') {
+    return baseCommand
+  }
+
+  const target = {
+    graphSelections: [
+      {
+        artifact: targetArtifact,
+        codeRef: pathArtifact.codeRef,
+      },
+    ],
+    otherSelections: [],
+  }
+
+  // Same roundabout but twice for 'path' aka trajectory: sketch -> path -> segment
+  if (!('path' in operation.labeledArgs) || !operation.labeledArgs.path) {
+    return baseCommand
+  }
+
+  if (operation.labeledArgs.path.value.type !== 'Sketch') {
+    return baseCommand
+  }
+
+  const trajectoryPathArtifact = getArtifactOfTypes(
+    {
+      key: operation.labeledArgs.path.value.value.artifactId,
+      types: ['path'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(trajectoryPathArtifact) || trajectoryPathArtifact.type !== 'path') {
+    return baseCommand
+  }
+
+  const trajectoryArtifact = getArtifactOfTypes(
+    {
+      key: trajectoryPathArtifact.segIds[0],
+      types: ['segment'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(trajectoryArtifact) || trajectoryArtifact.type !== 'segment') {
+    return baseCommand
+  }
+
+  const trajectory = {
+    graphSelections: [
+      {
+        artifact: trajectoryArtifact,
+        codeRef: trajectoryArtifact.codeRef,
+      },
+    ],
+    otherSelections: [],
+  }
+
+  // sectional options boolean arg
+  if (
+    !('sectional' in operation.labeledArgs) ||
+    !operation.labeledArgs.sectional
+  ) {
+    return baseCommand
+  }
+
+  const sectional =
+    codeManager.code.slice(
+      operation.labeledArgs.sectional.sourceRange[0],
+      operation.labeledArgs.sectional.sourceRange[1]
+    ) === 'true'
+
+  // Assemble the default argument values for the Offset Plane command,
+  // with `nodeToEdit` set, which will let the Offset Plane actor know
+  // to edit the node that corresponds to the StdLibCall.
+  const argDefaultValues: ModelingCommandSchema['Sweep'] = {
+    target: target,
+    trajectory,
+    sectional,
+    nodeToEdit: getNodePathFromSourceRange(
+      kclManager.ast,
+      sourceRangeFromRust(operation.sourceRange)
+    ),
+  }
+
+  return {
+    ...baseCommand,
+    argDefaultValues,
+  }
+}
+
 const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
  const baseCommand = {
    name: 'Helix',
@ -511,6 +648,7 @@ export const stdLibMap: Record<string, StdLibCallInfo> = {
  sweep: {
    label: 'Sweep',
    icon: 'sweep',
+    prepareToEdit: prepareToEditSweep,
    supportsAppearance: true,
  },
 }
--- a/src/machines/modelingMachine.ts
+++ b/src/machines/modelingMachine.ts
@ -78,7 +78,7 @@ import {
 import { ModelingCommandSchema } from 'lib/commandBarConfigs/modelingCommandConfig'
 import { err, reportRejection, trap } from 'lib/trap'
 import { DefaultPlaneStr } from 'lib/planes'
-import { uuidv4 } from 'lib/utils'
+import { isArray, uuidv4 } from 'lib/utils'
 import { Coords2d } from 'lang/std/sketch'
 import { deleteSegment } from 'clientSideScene/ClientSideSceneComp'
 import toast from 'react-hot-toast'
@ -1994,55 +1994,88 @@ export const modelingMachine = setup({
        if (!input) return new Error('No input provided')
        // Extract inputs
        const ast = kclManager.ast
-        const { target, trajectory } = input
+        const { target, trajectory, sectional, nodeToEdit } = input
+        let variableName: string | undefined = undefined
+        let insertIndex: number | undefined = undefined

-        // Find the profile declaration
+        // If this is an edit flow, first we're going to remove the old one
+        if (nodeToEdit !== undefined && typeof nodeToEdit[1][0] === 'number') {
+          // Extract the plane name from the node to edit
+          const variableNode = getNodeFromPath<VariableDeclaration>(
+            ast,
+            nodeToEdit,
+            'VariableDeclaration'
+          )
+
+          if (err(variableNode)) {
+            console.error('Error extracting name')
+          } else {
+            variableName = variableNode.node.declaration.id.name
+          }
+
+          // Removing the old statement
+          const newBody = [...ast.body]
+          newBody.splice(nodeToEdit[1][0], 1)
+          ast.body = newBody
+          insertIndex = nodeToEdit[1][0]
+        }
+
+        // Find the target declaration
        const targetNodePath = getNodePathFromSourceRange(
          ast,
          target.graphSelections[0].codeRef.range
        )
-        const targetNode = getNodeFromPath<VariableDeclarator>(
-          ast,
-          targetNodePath,
-          'VariableDeclarator'
-        )
+        // Gotchas, not sure why
+        // - it seems like in some cases we get a list on edit, especially the state that e2e hits
+        // - looking for a VariableDeclaration seems more robust than VariableDeclarator
+        const targetNode = getNodeFromPath<
+          VariableDeclaration | VariableDeclaration[]
+        >(ast, targetNodePath, 'VariableDeclaration')
        if (err(targetNode)) {
          return new Error("Couldn't parse profile selection")
        }
-        const targetDeclarator = targetNode.node

-        // Find the path declaration
+        const targetDeclarator = isArray(targetNode.node)
+          ? targetNode.node[0].declaration
+          : targetNode.node.declaration
+
+        // Find the trajectory (or path) declaration
        const trajectoryNodePath = getNodePathFromSourceRange(
          ast,
          trajectory.graphSelections[0].codeRef.range
        )
-        const trajectoryNode = getNodeFromPath<VariableDeclarator>(
+        // Also looking for VariableDeclaration for consistency here
+        const trajectoryNode = getNodeFromPath<VariableDeclaration>(
          ast,
          trajectoryNodePath,
-          'VariableDeclarator'
+          'VariableDeclaration'
        )
        if (err(trajectoryNode)) {
          return new Error("Couldn't parse path selection")
        }
-        const trajectoryDeclarator = trajectoryNode.node
+
+        const trajectoryDeclarator = trajectoryNode.node.declaration

        // Perform the sweep
-        const sweepRes = addSweep(ast, targetDeclarator, trajectoryDeclarator)
-        const updateAstResult = await kclManager.updateAst(
-          sweepRes.modifiedAst,
-          true,
+        const { modifiedAst, pathToNode } = addSweep({
+          node: ast,
+          targetDeclarator,
+          trajectoryDeclarator,
+          sectional,
+          variableName,
+          insertIndex,
+        })
+        await updateModelingState(
+          modifiedAst,
          {
-            focusPath: [sweepRes.pathToNode],
+            kclManager,
+            editorManager,
+            codeManager,
+          },
+          {
+            focusPath: [pathToNode],
          }
        )
-
-        await codeManager.updateEditorWithAstAndWriteToFile(
-          updateAstResult.newAst
-        )
-
-        if (updateAstResult?.selections) {
-          editorManager.selectRange(updateAstResult?.selections)
-        }
      }
    ),
    loftAstMod: fromPromise(