diff --git a/e2e/playwright/feature-tree-pane.spec.ts b/e2e/playwright/feature-tree-pane.spec.ts index 56d84aa87..7159fc72c 100644 --- a/e2e/playwright/feature-tree-pane.spec.ts +++ b/e2e/playwright/feature-tree-pane.spec.ts @@ -388,4 +388,52 @@ test.describe('Feature Tree pane', () => { }) }) }) + + test(`Delete sketch on offset plane and all profiles from feature tree`, async ({ + context, + page, + homePage, + scene, + editor, + toolbar, + cmdBar, + }) => { + const beforeKclCode = `plane001 = offsetPlane('XY', offset = 5) +sketch001 = startSketchOn(plane001) +profile001 = circle(sketch001, center = [0, 20], radius = 12) +profile002 = startProfileAt([0, 7.25], sketch001) + |> xLine(13.3, %) +profile003 = startProfileAt([0, -4.93], sketch001) + |> line(endAbsolute = [-5.56, 0])` + await context.folderSetupFn(async (dir) => { + const testProject = join(dir, 'test-sample') + await fsp.mkdir(testProject, { recursive: true }) + await fsp.writeFile(join(testProject, 'main.kcl'), beforeKclCode, 'utf-8') + }) + // One dumb hardcoded screen pixel value + const testPoint = { x: 650, y: 250 } + const sketchColor: [number, number, number] = [149, 149, 149] + const planeColor: [number, number, number] = [74, 74, 74] + + await homePage.openProject('test-sample') + // FIXME: @lf94 has a better way to verify execution completion, in a PR rn + await scene.waitForExecutionDone() + + await test.step(`Verify we see the sketch`, async () => { + await scene.expectPixelColor(sketchColor, testPoint, 10) + }) + + await test.step('Delete sketch via feature tree selection', async () => { + const operationButton = await toolbar.getFeatureTreeOperation('Sketch', 0) + await operationButton.click({ button: 'left' }) + await page.keyboard.press('Delete') + await scene.expectPixelColor(planeColor, testPoint, 10) + }) + + await test.step(`Verify the code changed`, async () => { + await editor.expectEditor.toContain('plane001 =') + await editor.expectEditor.not.toContain('sketch001 =') + await editor.expectEditor.not.toContain('profile002 = ') + }) + }) }) diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index 18b42ccdb..a468a4ab3 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -75,6 +75,7 @@ import { import { BodyItem } from '@rust/kcl-lib/bindings/BodyItem' import { findKwArg } from './util' import { deleteEdgeTreatment } from './modifyAst/addEdgeTreatment' +import { codeManager } from 'lib/singletons' export function startSketchOnDefault( node: Node, @@ -1409,18 +1410,39 @@ export async function deleteFromSelection( ({} as any) ): Promise | Error> { const astClone = structuredClone(ast) + let deletionArtifact = selection.artifact + + // Coerce sketch artifacts to their plane first + if (selection.artifact?.type === 'startSketchOnPlane') { + const planeArtifact = getArtifactOfTypes( + { key: selection.artifact.planeId, types: ['plane'] }, + artifactGraph + ) + if (!err(planeArtifact)) { + deletionArtifact = planeArtifact + } + } else if (selection.artifact?.type === 'startSketchOnFace') { + const planeArtifact = getArtifactOfTypes( + { key: selection.artifact.faceId, types: ['plane'] }, + artifactGraph + ) + if (!err(planeArtifact)) { + deletionArtifact = planeArtifact + } + } + if ( - (selection.artifact?.type === 'plane' || - selection.artifact?.type === 'cap' || - selection.artifact?.type === 'wall') && - selection.artifact?.pathIds?.length + (deletionArtifact?.type === 'plane' || + deletionArtifact?.type === 'cap' || + deletionArtifact?.type === 'wall') && + deletionArtifact?.pathIds?.length ) { const plane = - selection.artifact.type === 'plane' - ? expandPlane(selection.artifact, artifactGraph) - : selection.artifact.type === 'wall' - ? expandWall(selection.artifact, artifactGraph) - : expandCap(selection.artifact, artifactGraph) + deletionArtifact.type === 'plane' + ? expandPlane(deletionArtifact, artifactGraph) + : deletionArtifact.type === 'wall' + ? expandWall(deletionArtifact, artifactGraph) + : expandCap(deletionArtifact, artifactGraph) for (const path of plane.paths.sort( (a, b) => b.codeRef.range?.[0] - a.codeRef.range?.[0] )) { @@ -1435,22 +1457,36 @@ export async function deleteFromSelection( } // If it's a cap, we're not going to continue and try to // delete the extrusion - if ( - selection.artifact.type === 'cap' || - selection.artifact.type === 'wall' - ) { + if (deletionArtifact.type === 'cap' || deletionArtifact.type === 'wall') { // Delete the sketch node, which would not work if // we continued down the traditional code path below. // faceCodeRef's pathToNode is empty for some reason // so using source range instead - const codeRef = getFaceCodeRef(selection.artifact) + const codeRef = getFaceCodeRef(deletionArtifact) if (!codeRef) return new Error('Could not find face code ref') const sketchVarDec = getNodePathFromSourceRange(astClone, codeRef.range) const sketchBodyIndex = Number(sketchVarDec[1][0]) astClone.body.splice(sketchBodyIndex, 1) return astClone } + + // If we coerced the artifact from a sketch to a plane, + // this is where we hop off after we delete the sketch variable declaration + if ( + selection.artifact?.type === 'startSketchOnPlane' || + selection.artifact?.type === 'startSketchOnFace' + ) { + const sketchVarDec = getNodePathFromSourceRange( + astClone, + selection.artifact.codeRef.range + ) + const sketchBodyIndex = Number(sketchVarDec[1][0]) + astClone.body.splice(sketchBodyIndex, 1) + return astClone + } } + + // Below is all AST-based deletion logic const varDec = getNodeFromPath( ast, selection?.codeRef?.pathToNode,