Allow point-and-click Substract to take in multiple solids and tools (#7614)

* Allow point-and-click Substract to take in multiple tools
Fixes #7612

* Change target to solids for consistency and make it support multi select too

* Improve err message

* Update src/lang/modifyAst/boolean.ts

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* Update src/lang/modifyAst/boolean.ts

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

* Good bot

* Reduce array to single value if len 1

* Remove console.log

---------

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

e2e/playwright/boolean.spec.ts

@@ -12,7 +12,7 @@ test.describe('Point and click for boolean workflows', () => {
     },
     {
       name: 'subtract',
-      code: 'subtract([extrude001], tools = [extrude006])',
+      code: 'subtract(extrude001, tools = extrude006)',
     },
     {
       name: 'intersect',
@@ -81,6 +81,8 @@ test.describe('Point and click for boolean workflows', () => {
         if (operationName !== 'subtract') {
           // should down shift key to select multiple objects
           await page.keyboard.down('Shift')
+        } else {
+          await cmdBar.progressCmdBar()
         }
 
         // Select second object
@@ -103,8 +105,8 @@ test.describe('Point and click for boolean workflows', () => {
           await cmdBar.expectState({
             stage: 'review',
             headerArguments: {
-              Tool: '1 path',
+              Solids: '1 path',
-              Target: '1 path',
+              Tools: '1 path',
             },
             commandName,
           })

src/lang/modifyAst/boolean.ts

@@ -23,13 +23,13 @@ import type {
   VariableDeclaration,
 } from '@src/lang/wasm'
 import { EXECUTION_TYPE_REAL } from '@src/lib/constants'
-import type { Selection, Selections } from '@src/lib/selections'
+import type { Selections } from '@src/lib/selections'
 import { err } from '@src/lib/trap'
 import { isArray } from '@src/lib/utils'
 
 export async function applySubtractFromTargetOperatorSelections(
-  target: Selection,
+  solids: Selections,
-  tool: Selection,
+  tools: Selections,
   dependencies: {
     kclManager: KclManager
     engineCommandManager: EngineCommandManager
@@ -38,28 +38,28 @@ export async function applySubtractFromTargetOperatorSelections(
   }
 ): Promise<Error | void> {
   const ast = dependencies.kclManager.ast
-  if (!target.artifact || !tool.artifact) {
+  const lastSolidsVars = getLastVariableDeclarationsFromSelections(
-    return new Error('No artifact found')
+    solids,
-  }
+    ast,
-  const orderedChildrenTarget = findAllChildrenAndOrderByPlaceInCode(
-    target.artifact,
     dependencies.kclManager.artifactGraph
   )
-  const orderedChildrenTool = findAllChildrenAndOrderByPlaceInCode(
+  if (err(lastSolidsVars) || lastSolidsVars.length < 1) {
-    tool.artifact,
+    return new Error('Not enough or invalid solids variables found')
+  }
+
+  const lastToolsVars = getLastVariableDeclarationsFromSelections(
+    tools,
+    ast,
     dependencies.kclManager.artifactGraph
   )
-
+  if (err(lastToolsVars) || lastToolsVars.length < 1) {
-  const lastVarTarget = getLastVariable(orderedChildrenTarget, ast)
+    return new Error('Not enough or invalid tools variables found')
-  const lastVarTool = getLastVariable(orderedChildrenTool, ast)
-
-  if (!lastVarTarget || !lastVarTool) {
-    return new Error('No variable found')
   }
+
   const modifiedAst = booleanSubtractAstMod({
     ast,
-    targets: [lastVarTarget?.variableDeclaration?.node],
+    solids: lastSolidsVars,
-    tools: [lastVarTool?.variableDeclaration.node],
+    tools: lastToolsVars,
   })
 
   await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, dependencies)
@@ -75,34 +75,13 @@ export async function applyUnionFromTargetOperatorSelections(
   }
 ): Promise<Error | void> {
   const ast = dependencies.kclManager.ast
-
+  const lastVars = getLastVariableDeclarationsFromSelections(
-  const artifacts: Artifact[] = []
+    solids,
-  for (const selection of solids.graphSelections) {
+    ast,
-    if (selection.artifact) {
+    dependencies.kclManager.artifactGraph
-      artifacts.push(selection.artifact)
-    }
-  }
-
-  if (artifacts.length < 2) {
-    return new Error('Not enough artifacts selected')
-  }
-
-  const orderedChildrenEach = artifacts.map((artifact) =>
-    findAllChildrenAndOrderByPlaceInCode(
-      artifact,
-      dependencies.kclManager.artifactGraph
-    )
   )
-
+  if (err(lastVars) || lastVars.length < 2) {
-  const lastVars: VariableDeclaration[] = []
+    return new Error('Not enough or invalid solids variables found')
-  for (const orderedArtifactLeaves of orderedChildrenEach) {
-    const lastVar = getLastVariable(orderedArtifactLeaves, ast)
-    if (!lastVar) continue
-    lastVars.push(lastVar.variableDeclaration.node)
-  }
-
-  if (lastVars.length < 2) {
-    return new Error('Not enough variables found')
   }
 
   const modifiedAst = booleanUnionAstMod({
@@ -122,23 +101,36 @@ export async function applyIntersectFromTargetOperatorSelections(
   }
 ): Promise<Error | void> {
   const ast = dependencies.kclManager.ast
+  const lastVars = getLastVariableDeclarationsFromSelections(
+    solids,
+    ast,
+    dependencies.kclManager.artifactGraph
+  )
+  if (err(lastVars) || lastVars.length < 2) {
+    return new Error('Not enough or invalid solids variables found')
+  }
 
+  const modifiedAst = booleanIntersectAstMod({
+    ast,
+    solids: lastVars,
+  })
+  await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, dependencies)
+}
+
+function getLastVariableDeclarationsFromSelections(
+  selections: Selections,
+  ast: Node<Program>,
+  artifactGraph: ArtifactGraph
+): Error | VariableDeclaration[] {
   const artifacts: Artifact[] = []
-  for (const selection of solids.graphSelections) {
+  for (const selection of selections.graphSelections) {
     if (selection.artifact) {
       artifacts.push(selection.artifact)
     }
   }
 
-  if (artifacts.length < 2) {
-    return new Error('Not enough artifacts selected')
-  }
-
   const orderedChildrenEach = artifacts.map((artifact) =>
-    findAllChildrenAndOrderByPlaceInCode(
+    findAllChildrenAndOrderByPlaceInCode(artifact, artifactGraph)
-      artifact,
-      dependencies.kclManager.artifactGraph
-    )
   )
 
   const lastVars: VariableDeclaration[] = []
@@ -148,15 +140,7 @@ export async function applyIntersectFromTargetOperatorSelections(
     lastVars.push(lastVar.variableDeclaration.node)
   }
 
-  if (lastVars.length < 2) {
+  return lastVars
-    return new Error('Not enough variables found')
-  }
-
-  const modifiedAst = booleanIntersectAstMod({
-    ast,
-    solids: lastVars,
-  })
-  await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, dependencies)
 }
 
 /** returns all children of a given artifact, and sorts them DESC by start sourceRange
@@ -271,25 +255,27 @@ export function getLastVariable(
 
 export function booleanSubtractAstMod({
   ast,
-  targets,
+  solids,
   tools,
 }: {
   ast: Node<Program>
-  targets: VariableDeclaration[]
+  solids: VariableDeclaration[]
   tools: VariableDeclaration[]
 }): Node<Program> {
   const newAst = structuredClone(ast)
   const newVarName = findUniqueName(newAst, 'solid')
-  const createArrExpr = (varDecs: VariableDeclaration[]) =>
+  const createArrExpr = (varDecs: VariableDeclaration[]) => {
-    createArrayExpression(
+    const names = varDecs.map((varDec) =>
-      varDecs.map((varDec) => createLocalName(varDec.declaration.id.name))
+      createLocalName(varDec.declaration.id.name)
     )
-  const targetsArrayExpression = createArrExpr(targets)
+    return names.length === 1 ? names[0] : createArrayExpression(names)
+  }
+  const solidsArrayExpression = createArrExpr(solids)
   const toolsArrayExpression = createArrExpr(tools)
 
   const newVarDec = createVariableDeclaration(
     newVarName,
-    createCallExpressionStdLibKw('subtract', targetsArrayExpression, [
+    createCallExpressionStdLibKw('subtract', solidsArrayExpression, [
       createLabeledArg('tools', toolsArrayExpression),
     ])
   )

src/lib/commandBarConfigs/modelingCommandConfig.ts

@@ -204,8 +204,8 @@ export type ModelingCommandSchema = {
     variableName: string
   }
   'Boolean Subtract': {
-    target: Selections
+    solids: Selections
-    tool: Selections
+    tools: Selections
   }
   'Boolean Union': {
     solids: Selections
@@ -595,23 +595,21 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
     icon: 'booleanSubtract',
     needsReview: true,
     args: {
-      target: {
+      solids: {
         inputType: 'selection',
         selectionTypes: ['path'],
         selectionFilter: ['object'],
-        multiple: false,
+        multiple: true,
         required: true,
-        skip: true,
         hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
       },
-      tool: {
+      tools: {
         clearSelectionFirst: true,
         inputType: 'selection',
         selectionTypes: ['path'],
         selectionFilter: ['object'],
-        multiple: false,
+        multiple: true,
         required: true,
-        skip: false,
         hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
       },
     },

src/machines/modelingMachine.ts

@@ -3570,17 +3570,17 @@ export const modelingMachine = setup({
           return Promise.reject(new Error(NO_INPUT_PROVIDED_MESSAGE))
         }
 
-        const { target, tool } = input
+        const { solids, tools } = input
         if (
-          !target.graphSelections[0].artifact ||
+          !solids.graphSelections.some((selection) => selection.artifact) ||
-          !tool.graphSelections[0].artifact
+          !tools.graphSelections.some((selection) => selection.artifact)
         ) {
           return Promise.reject(new Error('No artifact in selections found'))
         }
 
-        await applySubtractFromTargetOperatorSelections(
+        const result = await applySubtractFromTargetOperatorSelections(
-          target.graphSelections[0],
+          solids,
-          tool.graphSelections[0],
+          tools,
           {
             kclManager,
             codeManager,
@@ -3588,6 +3588,9 @@ export const modelingMachine = setup({
             editorManager,
           }
         )
+        if (err(result)) {
+          return Promise.reject(result)
+        }
       }
     ),
     boolUnionAstMod: fromPromise(
@@ -3605,12 +3608,15 @@ export const modelingMachine = setup({
           return Promise.reject(new Error('No artifact in selections found'))
         }
 
-        await applyUnionFromTargetOperatorSelections(solids, {
+        const result = await applyUnionFromTargetOperatorSelections(solids, {
           kclManager,
           codeManager,
           engineCommandManager,
           editorManager,
         })
+        if (err(result)) {
+          return Promise.reject(result)
+        }
       }
     ),
     boolIntersectAstMod: fromPromise(
@@ -3628,12 +3634,18 @@ export const modelingMachine = setup({
           return Promise.reject(new Error('No artifact in selections found'))
         }
 
-        await applyIntersectFromTargetOperatorSelections(solids, {
+        const result = await applyIntersectFromTargetOperatorSelections(
-          kclManager,
+          solids,
-          codeManager,
+          {
-          engineCommandManager,
+            kclManager,
-          editorManager,
+            codeManager,
-        })
+            engineCommandManager,
+            editorManager,
+          }
+        )
+        if (err(result)) {
+          return Promise.reject(result)
+        }
       }
     ),