diff --git a/e2e/playwright/boolean.spec.ts b/e2e/playwright/boolean.spec.ts index 2a1379f95..b4509ecd0 100644 --- a/e2e/playwright/boolean.spec.ts +++ b/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', - Target: '1 path', + Solids: '1 path', + Tools: '1 path', }, commandName, }) diff --git a/src/lang/modifyAst/boolean.ts b/src/lang/modifyAst/boolean.ts index 1f5b38a4d..54f48d1b2 100644 --- a/src/lang/modifyAst/boolean.ts +++ b/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, - tool: Selection, + solids: Selections, + tools: Selections, dependencies: { kclManager: KclManager engineCommandManager: EngineCommandManager @@ -38,28 +38,28 @@ export async function applySubtractFromTargetOperatorSelections( } ): Promise { const ast = dependencies.kclManager.ast - if (!target.artifact || !tool.artifact) { - return new Error('No artifact found') - } - const orderedChildrenTarget = findAllChildrenAndOrderByPlaceInCode( - target.artifact, + const lastSolidsVars = getLastVariableDeclarationsFromSelections( + solids, + ast, dependencies.kclManager.artifactGraph ) - const orderedChildrenTool = findAllChildrenAndOrderByPlaceInCode( - tool.artifact, + if (err(lastSolidsVars) || lastSolidsVars.length < 1) { + return new Error('Not enough or invalid solids variables found') + } + + const lastToolsVars = getLastVariableDeclarationsFromSelections( + tools, + ast, dependencies.kclManager.artifactGraph ) - - const lastVarTarget = getLastVariable(orderedChildrenTarget, ast) - const lastVarTool = getLastVariable(orderedChildrenTool, ast) - - if (!lastVarTarget || !lastVarTool) { - return new Error('No variable found') + if (err(lastToolsVars) || lastToolsVars.length < 1) { + return new Error('Not enough or invalid tools variables found') } + const modifiedAst = booleanSubtractAstMod({ ast, - targets: [lastVarTarget?.variableDeclaration?.node], - tools: [lastVarTool?.variableDeclaration.node], + solids: lastSolidsVars, + tools: lastToolsVars, }) await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, dependencies) @@ -75,34 +75,13 @@ export async function applyUnionFromTargetOperatorSelections( } ): Promise { const ast = dependencies.kclManager.ast - - const artifacts: Artifact[] = [] - for (const selection of solids.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( - artifact, - dependencies.kclManager.artifactGraph - ) + const lastVars = getLastVariableDeclarationsFromSelections( + solids, + ast, + dependencies.kclManager.artifactGraph ) - - const lastVars: VariableDeclaration[] = [] - 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') + if (err(lastVars) || lastVars.length < 2) { + return new Error('Not enough or invalid solids variables found') } const modifiedAst = booleanUnionAstMod({ @@ -122,23 +101,36 @@ export async function applyIntersectFromTargetOperatorSelections( } ): Promise { 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, + 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( - artifact, - dependencies.kclManager.artifactGraph - ) + findAllChildrenAndOrderByPlaceInCode(artifact, artifactGraph) ) const lastVars: VariableDeclaration[] = [] @@ -148,15 +140,7 @@ export async function applyIntersectFromTargetOperatorSelections( lastVars.push(lastVar.variableDeclaration.node) } - if (lastVars.length < 2) { - return new Error('Not enough variables found') - } - - const modifiedAst = booleanIntersectAstMod({ - ast, - solids: lastVars, - }) - await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, dependencies) + return lastVars } /** 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 - targets: VariableDeclaration[] + solids: VariableDeclaration[] tools: VariableDeclaration[] }): Node { const newAst = structuredClone(ast) const newVarName = findUniqueName(newAst, 'solid') - const createArrExpr = (varDecs: VariableDeclaration[]) => - createArrayExpression( - varDecs.map((varDec) => createLocalName(varDec.declaration.id.name)) + const createArrExpr = (varDecs: VariableDeclaration[]) => { + const names = varDecs.map((varDec) => + 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), ]) ) diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index 78a21a6bb..3d0814506 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -204,8 +204,8 @@ export type ModelingCommandSchema = { variableName: string } 'Boolean Subtract': { - target: Selections - tool: Selections + solids: 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), }, }, diff --git a/src/machines/modelingMachine.ts b/src/machines/modelingMachine.ts index d409b5d75..ad8183302 100644 --- a/src/machines/modelingMachine.ts +++ b/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 || - !tool.graphSelections[0].artifact + !solids.graphSelections.some((selection) => selection.artifact) || + !tools.graphSelections.some((selection) => selection.artifact) ) { return Promise.reject(new Error('No artifact in selections found')) } - await applySubtractFromTargetOperatorSelections( - target.graphSelections[0], - tool.graphSelections[0], + const result = await applySubtractFromTargetOperatorSelections( + solids, + 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, { - kclManager, - codeManager, - engineCommandManager, - editorManager, - }) + const result = await applyIntersectFromTargetOperatorSelections( + solids, + { + kclManager, + codeManager, + engineCommandManager, + editorManager, + } + ) + if (err(result)) { + return Promise.reject(result) + } } ),