Allow users to delete sketches on offset planes via feature tree (#5641)

* Add sketch-on-offset plane deletion cleanup logic

* Add an E2E test for this behavior

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 = ')
+    })
+  })
 })

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<Program>,
@@ -1409,18 +1410,39 @@ export async function deleteFromSelection(
     ({} as any)
 ): Promise<Node<Program> | 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' ||
+    (deletionArtifact?.type === 'plane' ||
-      selection.artifact?.type === 'cap' ||
+      deletionArtifact?.type === 'cap' ||
-      selection.artifact?.type === 'wall') &&
+      deletionArtifact?.type === 'wall') &&
-    selection.artifact?.pathIds?.length
+    deletionArtifact?.pathIds?.length
   ) {
     const plane =
-      selection.artifact.type === 'plane'
+      deletionArtifact.type === 'plane'
-        ? expandPlane(selection.artifact, artifactGraph)
+        ? expandPlane(deletionArtifact, artifactGraph)
-        : selection.artifact.type === 'wall'
+        : deletionArtifact.type === 'wall'
-        ? expandWall(selection.artifact, artifactGraph)
+        ? expandWall(deletionArtifact, artifactGraph)
-        : expandCap(selection.artifact, 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 (
+    if (deletionArtifact.type === 'cap' || deletionArtifact.type === 'wall') {
-      selection.artifact.type === 'cap' ||
-      selection.artifact.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<VariableDeclarator>(
     ast,
     selection?.codeRef?.pathToNode,