Disable extrude button if there is no extrudable geometry (#2730)

Disable extrude button if there is no extrudeable geometry

e2e/playwright/flow-tests.spec.ts

@@ -1798,6 +1798,74 @@ test.describe('Testing selections', () => {
     await page.waitForTimeout(100)
     await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
   })
+  test("Extrude button should be disabled if there's no extrudable geometry when nothing is selected", async ({
+    page,
+  }) => {
+    const u = await getUtils(page)
+    await page.addInitScript(async () => {
+      localStorage.setItem(
+        'persistCode',
+        `const sketch001 = startSketchOn('XZ')
+  |> startProfileAt([3.29, 7.86], %)
+  |> line([2.48, 2.44], %)
+  |> line([2.66, 1.17], %)
+  |> line([3.75, 0.46], %)
+  |> line([4.99, -0.46], %, 'seg01')
+  |> line([3.3, -2.12], %)
+  |> line([2.16, -3.33], %)
+  |> line([0.85, -3.08], %)
+  |> line([-0.18, -3.36], %)
+  |> line([-3.86, -2.73], %)
+  |> line([-17.67, 0.85], %)
+  |> close(%)
+const extrude001 = extrude(10, sketch001)
+  `
+      )
+    })
+    await page.setViewportSize({ width: 1000, height: 500 })
+    await page.goto('/')
+    await u.waitForAuthSkipAppStart()
+
+    // wait for execution done
+    await u.openDebugPanel()
+    await u.expectCmdLog('[data-message-type="execution-done"]')
+    await u.closeDebugPanel()
+
+    const selectUnExtrudable = () =>
+      page.getByText(`line([4.99, -0.46], %, 'seg01')`).click()
+    const clickEmpty = () => page.mouse.click(700, 460)
+    await selectUnExtrudable()
+    // expect extrude button to be disabled
+    await expect(page.getByRole('button', { name: 'Extrude' })).toBeDisabled()
+
+    await clickEmpty()
+
+    // expect active line to contain nothing
+    await expect(page.locator('.cm-activeLine')).toHaveText('')
+    // and extrude to still be disabled
+    await expect(page.getByRole('button', { name: 'Extrude' })).toBeDisabled()
+
+    const codeToAdd = `${await u.codeLocator.allInnerTexts()}
+const sketch002 = startSketchOn(extrude001, 'seg01')
+  |> startProfileAt([-12.94, 6.6], %)
+  |> line([2.45, -0.2], %)
+  |> line([-2, -1.25], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+`
+    await u.codeLocator.fill(codeToAdd)
+
+    await selectUnExtrudable()
+    // expect extrude button to be disabled
+    await expect(page.getByRole('button', { name: 'Extrude' })).toBeDisabled()
+
+    await clickEmpty()
+    await expect(page.locator('.cm-activeLine')).toHaveText('')
+    // there's not extrudable geometry, so button should be enabled
+    await expect(
+      page.getByRole('button', { name: 'Extrude' })
+    ).not.toBeDisabled()
+  })
 
   test('Testing selections (and hovers) work on sketches when NOT in sketch mode', async ({
     page,

src/components/ModelingMachineProvider.tsx

@@ -63,6 +63,7 @@ import {
 import {
   getNodeFromPath,
   getNodePathFromSourceRange,
+  hasExtrudableGeometry,
   isSingleCursorInPipe,
 } from 'lang/queryAst'
 import { TEST } from 'env'
@@ -447,8 +448,13 @@ export const ModelingMachineProvider = ({
           if (
             selectionRanges.codeBasedSelections.length === 0 ||
             isSelectionLastLine(selectionRanges, codeManager.code)
-          )
+          ) {
-            return true
+            // they have no selection, we should enable the button
+            // so they can select the face through the cmdbar
+            // BUT only if there's extrudable geometry
+            if (hasExtrudableGeometry(kclManager.ast)) return true
+            return false
+          }
           if (!isPipe) return false
 
           return canExtrudeSelection(selectionRanges)

src/lang/queryAst.test.ts

@@ -7,6 +7,8 @@ import {
   doesPipeHaveCallExp,
   hasExtrudeSketchGroup,
   findUsesOfTagInPipe,
+  hasSketchPipeBeenExtruded,
+  hasExtrudableGeometry,
 } from './queryAst'
 import { enginelessExecutor } from '../lib/testHelpers'
 import {
@@ -396,3 +398,90 @@ describe('Testing findUsesOfTagInPipe', () => {
     expect(result).toHaveLength(0)
   })
 })
+
+describe('Testing hasSketchPipeBeenExtruded', () => {
+  const exampleCode = `const sketch001 = startSketchOn('XZ')
+  |> startProfileAt([3.29, 7.86], %)
+  |> line([2.48, 2.44], %)
+  |> line([2.66, 1.17], %)
+  |> line([3.75, 0.46], %)
+  |> line([4.99, -0.46], %, 'seg01')
+  |> line([3.3, -2.12], %)
+  |> line([2.16, -3.33], %)
+  |> line([0.85, -3.08], %)
+  |> line([-0.18, -3.36], %)
+  |> line([-3.86, -2.73], %)
+  |> line([-17.67, 0.85], %)
+  |> close(%)
+const extrude001 = extrude(10, sketch001)
+const sketch002 = startSketchOn(extrude001, 'seg01')
+  |> startProfileAt([-12.94, 6.6], %)
+  |> line([2.45, -0.2], %)
+  |> line([-2, -1.25], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+`
+  it('finds sketch001 pipe to be extruded', async () => {
+    const ast = parse(exampleCode)
+    const lineOfInterest = `line([4.99, -0.46], %, 'seg01')`
+    const characterIndex =
+      exampleCode.indexOf(lineOfInterest) + lineOfInterest.length
+    const extruded = hasSketchPipeBeenExtruded(
+      {
+        range: [characterIndex, characterIndex],
+        type: 'default',
+      },
+      ast
+    )
+    expect(extruded).toBeTruthy()
+  })
+  it('find sketch002 NOT pipe to be extruded', async () => {
+    const ast = parse(exampleCode)
+    const lineOfInterest = `line([2.45, -0.2], %)`
+    const characterIndex =
+      exampleCode.indexOf(lineOfInterest) + lineOfInterest.length
+    const extruded = hasSketchPipeBeenExtruded(
+      {
+        range: [characterIndex, characterIndex],
+        type: 'default',
+      },
+      ast
+    )
+    expect(extruded).toBeFalsy()
+  })
+})
+
+describe('Testing hasExtrudableGeometry', () => {
+  it('finds sketch001 pipe to be extruded', async () => {
+    const exampleCode = `const sketch001 = startSketchOn('XZ')
+  |> startProfileAt([3.29, 7.86], %)
+  |> line([2.48, 2.44], %)
+  |> line([-3.86, -2.73], %)
+  |> line([-17.67, 0.85], %)
+  |> close(%)
+const extrude001 = extrude(10, sketch001)
+const sketch002 = startSketchOn(extrude001, 'seg01')
+  |> startProfileAt([-12.94, 6.6], %)
+  |> line([2.45, -0.2], %)
+  |> line([-2, -1.25], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+`
+    const ast = parse(exampleCode)
+    const extrudable = hasExtrudableGeometry(ast)
+    expect(extrudable).toBeTruthy()
+  })
+  it('find sketch002 NOT pipe to be extruded', async () => {
+    const exampleCode = `const sketch001 = startSketchOn('XZ')
+  |> startProfileAt([3.29, 7.86], %)
+  |> line([2.48, 2.44], %)
+  |> line([-3.86, -2.73], %)
+  |> line([-17.67, 0.85], %)
+  |> close(%)
+const extrude001 = extrude(10, sketch001)
+`
+    const ast = parse(exampleCode)
+    const extrudable = hasExtrudableGeometry(ast)
+    expect(extrudable).toBeFalsy()
+  })
+})

src/lang/queryAst.ts

@@ -720,3 +720,78 @@ export function findUsesOfTagInPipe(
   })
   return dependentRanges
 }
+
+export function hasSketchPipeBeenExtruded(selection: Selection, ast: Program) {
+  const path = getNodePathFromSourceRange(ast, selection.range)
+  const { node: pipeExpression } = getNodeFromPath<PipeExpression>(
+    ast,
+    path,
+    'PipeExpression'
+  )
+  if (pipeExpression.type !== 'PipeExpression') return false
+  const varDec = getNodeFromPath<VariableDeclarator>(
+    ast,
+    path,
+    'VariableDeclarator'
+  ).node
+  let extruded = false
+  traverse(ast as any, {
+    enter(node) {
+      if (
+        node.type === 'CallExpression' &&
+        node.callee.type === 'Identifier' &&
+        node.callee.name === 'extrude' &&
+        node.arguments?.[1]?.type === 'Identifier' &&
+        node.arguments[1].name === varDec.id.name
+      ) {
+        extruded = true
+      }
+    },
+  })
+  return extruded
+}
+
+/** File must contain at least one sketch that has not been extruded already */
+export function hasExtrudableGeometry(ast: Program) {
+  const theMap: any = {}
+  traverse(ast as any, {
+    enter(node) {
+      if (
+        node.type === 'VariableDeclarator' &&
+        node.init?.type === 'PipeExpression'
+      ) {
+        let hasStartProfileAt = false
+        let hasStartSketchOn = false
+        let hasClose = false
+        for (const pipe of node.init.body) {
+          if (
+            pipe.type === 'CallExpression' &&
+            pipe.callee.name === 'startProfileAt'
+          ) {
+            hasStartProfileAt = true
+          }
+          if (
+            pipe.type === 'CallExpression' &&
+            pipe.callee.name === 'startSketchOn'
+          ) {
+            hasStartSketchOn = true
+          }
+          if (pipe.type === 'CallExpression' && pipe.callee.name === 'close') {
+            hasClose = true
+          }
+        }
+        if (hasStartProfileAt && hasStartSketchOn && hasClose) {
+          theMap[node.id.name] = true
+        }
+      } else if (
+        node.type === 'CallExpression' &&
+        node.callee.name === 'extrude' &&
+        node.arguments[1]?.type === 'Identifier' &&
+        theMap?.[node?.arguments?.[1]?.name]
+      ) {
+        delete theMap[node.arguments[1].name]
+      }
+    },
+  })
+  return Object.keys(theMap).length > 0
+}

src/lib/selections.ts

@@ -16,6 +16,7 @@ import { Program } from 'lang/wasm'
 import {
   doesPipeHaveCallExp,
   getNodeFromPath,
+  hasSketchPipeBeenExtruded,
   isSingleCursorInPipe,
 } from 'lang/queryAst'
 import { CommandArgument } from './commandTypes'
@@ -387,6 +388,7 @@ export function canExtrudeSelection(selection: Selections) {
   )
   return (
     !!isSketchPipe(selection) &&
+    commonNodes.every((n) => !hasSketchPipeBeenExtruded(n.selection, n.ast)) &&
     commonNodes.every((n) => nodeHasClose(n)) &&
     commonNodes.every((n) => !nodeHasExtrude(n))
   )