Point-and-click transforms & appearance refactor with opt args (#7635)

* WIP: `global` optional arg for all point-and-click transforms and add Scale
Closes #7615 and #7634

* WIP transforms like sweep

* WIP: Allow all sweeps to work on variable-less profiles
Fixes #7657

* WIP

* WIP

* Fix test

* Fix circ dep

* Add unit tests for getVariableExprsFromSelection

* Codespell

* Add unit tests for createVariableExpressionsArray

* Add unit test createPathToNodeForLastVariable

* WIP new util function

* Add other test case

* We going

* Lint & complete addExtrude tests

* Add addSweep test

* Add basic addLoft and addRevolve tests, will have to see how to distribute coverage

* Progress integrating new stuff from sweeps

* Add support for imports

* Update rotate and scale to match

* WIP edit flows

* Fixing up edit flows for transforms, and WIP for simpler feature tree action

* Clean pu

* Clean up

* Big time clean up and bringing clone on for the ride

* Quick variable break out for clarity

* Add getSketchSelectionsFromOperation test

* retrieveSelectionsFromOpArg generic fn and tests

* Lint

* Extend getVariableExprsFromSelection to include child look up

* Add test to catch potentially wrong double pipe substitutions

* Fix circular dep and test

* Fix bad imports

* Fix test, still need to fix circ dep

* Fix circular dep

* Make transforms components optional args

* Move some translate tests from pw to unit

* Unit tests for rotate (moved most pw tests) and scale

* Bring back variable name for clone and clean up tests

* Remove playwright tests and beefing up vitest integration tests

* Back to a px click for loft selection :sad:, other fixes

* WIP p&c tests

* WIP

* Fix clone test

* Delete irrelevant test

* Change lotta things

* Wire up appearance as point-and-click operation just like other transforms

* Wire up opt args for Appearance

* WIP tests

* Set composite: false to work around tsc limitation

* New unit test, still a bug to fix with appearance edit flow in e2e test

* Fix e2e test

Author: pierremtb

e2e/playwright/point-click-assemblies.spec.ts

@@ -25,6 +25,7 @@ import {
   stdLibMap,
 } from '@src/lib/operations'
 import {
+  commandBarActor,
   editorManager,
   kclManager,
   rustContext,
@@ -73,6 +74,36 @@ export const FeatureTreePane = () => {
         scrollToError: () => {
           editorManager.scrollToFirstErrorDiagnosticIfExists()
         },
+        sendTranslateCommand: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Translate', groupId: 'modeling' },
+          })
+        },
+        sendRotateCommand: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Rotate', groupId: 'modeling' },
+          })
+        },
+        sendScaleCommand: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Scale', groupId: 'modeling' },
+          })
+        },
+        sendCloneCommand: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Clone', groupId: 'modeling' },
+          })
+        },
+        sendAppearanceCommand: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Appearance', groupId: 'modeling' },
+          })
+        },
         sendSelectionEvent: ({ context }) => {
           if (!context.targetSourceRange) {
             return
@@ -379,10 +410,6 @@ const OperationItem = (props: {
     }
   }
 
-  /**
-   * For now we can only enter the "edit" flow for the startSketchOn operation.
-   * TODO: https://github.com/KittyCAD/modeling-app/issues/4442
-   */
   function enterEditFlow() {
     if (
       props.item.type === 'StdLibCall' ||
@@ -434,6 +461,18 @@ const OperationItem = (props: {
     }
   }
 
+  function enterScaleFlow() {
+    if (props.item.type === 'StdLibCall' || props.item.type === 'GroupBegin') {
+      props.send({
+        type: 'enterScaleFlow',
+        data: {
+          targetSourceRange: sourceRangeFromRust(props.item.sourceRange),
+          currentOperation: props.item,
+        },
+      })
+    }
+  }
+
   function enterCloneFlow() {
     if (props.item.type === 'StdLibCall' || props.item.type === 'GroupBegin') {
       props.send({
@@ -565,7 +604,7 @@ const OperationItem = (props: {
                 !stdLibMap[props.item.name]?.supportsTransform
               }
             >
-              Set translate
+              Translate
             </ContextMenuItem>,
             <ContextMenuItem
               onClick={enterRotateFlow}
@@ -575,7 +614,17 @@ const OperationItem = (props: {
                 !stdLibMap[props.item.name]?.supportsTransform
               }
             >
-              Set rotate
+              Rotate
+            </ContextMenuItem>,
+            <ContextMenuItem
+              onClick={enterScaleFlow}
+              data-testid="context-menu-set-scale"
+              disabled={
+                props.item.type !== 'GroupBegin' &&
+                !stdLibMap[props.item.name]?.supportsTransform
+              }
+            >
+              Scale
             </ContextMenuItem>,
             <ContextMenuItem
               onClick={enterCloneFlow}

e2e/playwright/point-click.spec.ts

@@ -1058,7 +1058,7 @@ profile001 = circle(sketch001, center = [0, 0], radius = 1)
     const call = createCallExpressionStdLibKw('extrude', exprs, [
       createLabeledArg('length', createLiteral(5)),
     ])
-    const pathToNode = setCallInAst(ast, call)
+    const pathToNode = setCallInAst({ ast, call, variableIfNewDecl: 'extrude' })
     if (err(pathToNode)) {
       throw pathToNode
     }
@@ -1092,7 +1092,11 @@ profile001 = circle(sketch001, center = [0, 0], radius = 1)
     const call = createCallExpressionStdLibKw('extrude', exprs, [
       createLabeledArg('length', createLiteral(5)),
     ])
-    const pathToNode = setCallInAst(ast, call, undefined, vars.pathIfPipe)
+    const pathToNode = setCallInAst({
+      ast,
+      call,
+      pathIfNewPipe: vars.pathIfPipe,
+    })
     if (err(pathToNode)) {
       throw pathToNode
     }
@@ -1126,7 +1130,12 @@ profile001 = circle(sketch001, center = [0, 0], radius = 1)
     const call = createCallExpressionStdLibKw('extrude', exprs, [
       createLabeledArg('length', createLiteral(5)),
     ])
-    const pathToNode = setCallInAst(ast, call, undefined, vars.pathIfPipe)
+    const pathToNode = setCallInAst({
+      ast,
+      call,
+      pathIfNewPipe: vars.pathIfPipe,
+      variableIfNewDecl: 'extrude',
+    })
     if (err(pathToNode)) {
       throw pathToNode
     }

src/components/ModelingSidebar/ModelingPanes/FeatureTreePane.tsx

@@ -6,6 +6,7 @@ import type { NonCodeMeta } from '@rust/kcl-lib/bindings/NonCodeMeta'
 import {
   createArrayExpression,
   createCallExpressionStdLibKw,
+  createExpressionStatement,
   createIdentifier,
   createImportAsSelector,
   createImportStatement,
@@ -596,39 +597,6 @@ export function addHelix({
   }
 }
 
-/**
- * Add clone statement
- */
-export function addClone({
-  ast,
-  geometryName,
-  variableName,
-}: {
-  ast: Node<Program>
-  geometryName: string
-  variableName: string
-}): { modifiedAst: Node<Program>; pathToNode: PathToNode } {
-  const modifiedAst = structuredClone(ast)
-  const variable = createVariableDeclaration(
-    variableName,
-    createCallExpressionStdLibKw('clone', createLocalName(geometryName), [])
-  )
-
-  modifiedAst.body.push(variable)
-  const insertAt = modifiedAst.body.length - 1
-  const pathToNode: PathToNode = [
-    ['body', ''],
-    [insertAt, 'index'],
-    ['declaration', 'VariableDeclaration'],
-    ['init', 'VariableDeclarator'],
-  ]
-
-  return {
-    modifiedAst,
-    pathToNode,
-  }
-}
-
 /**
  * Return a modified clone of an AST with a named constant inserted into the body
  */
@@ -1189,65 +1157,76 @@ export function createPathToNodeForLastVariable(
   return pathToCall
 }
 
-export function setCallInAst(
-  ast: Node<Program>,
-  call: Node<CallExpressionKw>,
-  nodeToEdit?: PathToNode,
-  pathIfPipe?: PathToNode
-): Error | PathToNode {
+export function setCallInAst({
+  ast,
+  call,
+  pathToEdit,
+  pathIfNewPipe,
+  variableIfNewDecl,
+}: {
+  ast: Node<Program>
+  call: Node<CallExpressionKw>
+  pathToEdit?: PathToNode
+  pathIfNewPipe?: PathToNode
+  variableIfNewDecl?: string
+}): Error | PathToNode {
   let pathToNode: PathToNode | undefined
-  if (nodeToEdit) {
+  if (pathToEdit) {
     const result = getNodeFromPath<CallExpressionKw>(
       ast,
-      nodeToEdit,
+      pathToEdit,
       'CallExpressionKw'
     )
     if (err(result)) {
       return result
     }
 
     Object.assign(result.node, call)
-    pathToNode = nodeToEdit
-  } else {
-    if (!call.unlabeled && pathIfPipe) {
-      const pipe = getNodeFromPath<PipeExpression>(
+    pathToNode = pathToEdit
+  } else if (pathIfNewPipe) {
+    const pipe = getNodeFromPath<PipeExpression>(
+      ast,
+      pathIfNewPipe,
+      'PipeExpression'
+    )
+    if (err(pipe)) {
+      return pipe
+    }
+    if (pipe.node.type === 'PipeExpression') {
+      pipe.node.body.push(call)
+    } else if (pipe.node.type === 'CallExpressionKw') {
+      const expression = getNodeFromPath<ExpressionStatement>(
         ast,
-        pathIfPipe,
-        'PipeExpression'
+        pathIfNewPipe,
+        'ExpressionStatement'
       )
-      if (err(pipe)) {
-        return pipe
+      if (err(expression) || expression.node.type !== 'ExpressionStatement') {
+        return new Error('Could not retrieve ExpressionStatement')
       }
 
-      if (pipe.node.type === 'PipeExpression') {
-        pipe.node.body.push(call)
-      } else if (pipe.node.type === 'CallExpressionKw') {
-        const expression = getNodeFromPath<ExpressionStatement>(
-          ast,
-          pathIfPipe,
-          'ExpressionStatement'
-        )
-        if (err(expression) || expression.node.type !== 'ExpressionStatement') {
-          return new Error('Could not retrieve ExpressionStatement')
-        }
-
-        expression.node.expression = createPipeExpression([
-          expression.node.expression,
-          call,
-        ])
-      } else {
-        return new Error(
-          'Expected pipeIfPipe to be a PipeExpression or CallExpressionKw'
-        )
-      }
-      pathToNode = pathIfPipe
+      expression.node.expression = createPipeExpression([
+        expression.node.expression,
+        call,
+      ])
     } else {
-      const name = findUniqueName(ast, call.callee.name.name)
-      const declaration = createVariableDeclaration(name, call)
-      ast.body.push(declaration)
-      const toFirstKwarg = call.arguments.length > 0
-      pathToNode = createPathToNodeForLastVariable(ast, toFirstKwarg)
+      return new Error(
+        'Expected pipeIfPipe to be a PipeExpression or CallExpressionKw'
+      )
     }
+    pathToNode = pathIfNewPipe
+  } else if (variableIfNewDecl) {
+    const name = findUniqueName(ast, variableIfNewDecl)
+    const declaration = createVariableDeclaration(name, call)
+    ast.body.push(declaration)
+    const toFirstKwarg = call.arguments.length > 0
+    pathToNode = createPathToNodeForLastVariable(ast, toFirstKwarg)
+  } else {
+    ast.body.push(createExpressionStatement(call))
+    pathToNode = [
+      ['body', ''],
+      [ast.body.length - 1, 'index'],
+      ['expression', 'ExpressionStatement'],
+    ]
   }
 
   return pathToNode

src/lang/modifyAst.test.ts

@@ -32,6 +32,7 @@ import {
   getSweepArtifactFromSelection,
 } from '@src/lang/std/artifactGraph'
 import type { OpArg, OpKclValue } from '@rust/kcl-lib/bindings/Operation'
+import { KCL_DEFAULT_CONSTANT_PREFIXES } from '@src/lib/constants'
 
 export function addShell({
   ast,
@@ -103,12 +104,13 @@ export function addShell({
 
   // 3. If edit, we assign the new function call declaration to the existing node,
   // otherwise just push to the end
-  const pathToNode = setCallInAst(
-    modifiedAst,
+  const pathToNode = setCallInAst({
+    ast: modifiedAst,
     call,
-    nodeToEdit,
-    vars.pathIfPipe
-  )
+    pathToEdit: nodeToEdit,
+    pathIfNewPipe: vars.pathIfPipe,
+    variableIfNewDecl: KCL_DEFAULT_CONSTANT_PREFIXES.SHELL,
+  })
   if (err(pathToNode)) {
     return pathToNode
   }