Add edit flows for extrude and offset plane operations (#5045)

* Start implementing a "prepareToEdit" callback for extrude

* Start of generic edit flow for operations

* Actually invoking command bar send generically on double-click

* Refactor: break out non-React hook helper to calculate Kcl expression value

* Add unit tests, fmt

* Integrate helper to get calculated KclExpression

* Clean up unused imports, simplify use of `programMemoryFromVariables`

* Implement basic extrude editing

* Refactor: move DefaultPlanesStr to its own lib file

* Add support for editing offset planes

* Add Edit right-click menu option

* Turn off edit flow for sketch for now

* Add e2e tests for sketch and offset plane editing, fix bug found with offset plane editing

* Add failing e2e extrude edit test

* Remove action version of extrude AST mod

* Fix behavior when adding a constant while editing operation, fixing e2e test

* Patch in changes from 61b02b5703

* Remove shell's prepareToEdit

* Add other Surface types to `artifactIsPlaneWithPaths`

* refactor: rename `item` to `operation`

* Allow `prepareToEdit` to fail with a toast, signal sketch-on-offset is unimplemented

* Rework sketch e2e test to test several working and failing cases

* Fix tsc errors related to making `codeRef` optional

* Make basic error messages more friendly

* fmt

* Reset modifyAst.ts to main

* Fix broken artifactGraph unit test

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Remove unused import

* Look at this (photo)Graph *in the voice of Nickelback*

* Make the offset plane insert at the end, not one before

* Fix bug caught by e2e test failure with "Command needs review" logic

* Update src/machines/modelingMachine.ts

Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com>

* Remove console logs per @pierremtb

* Update src/components/CommandBar/CommandBarHeader.tsx

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

* Use better programMemory init thanks @jtran

* Fix tsc post merge of #5068

* Fix logic for `artifactIsPlaneWithPaths` post-merge

* Need to disable the sketch-on-face case now that artifactGraph is in Rust. Will active in a future PR (cc @jtran)

* Re-run CI after snapshots

* Update FeatureTreePane to not use `useCommandsContext`, missed during merge

* Fix merge issue, import location change on edited file

* fix click test step, which I believe is waiting for context scripts to load

* Convert toolbarFixture.exeIndicator to getter

We need to convert all these selectors on fixtures to getters, because
they can go stale if called on the fixture constructor.

* Missed a dumb little thing in toolbarFixture.ts

* Fix goof with merge

* fmt

* Another dumb missed thing during merge

I gotta get used to the LazyGit merge tool I'm not good at it yet

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Conver sceneFixture's exeIndicator to a getter

Locators on fixtures will be frozen from the time of the fixture's
initialization, I'm increasingly convinced

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Post-kwargs E2E test cleanup

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

e2e/playwright/feature-tree-pane.spec.ts

@@ -35,6 +35,30 @@ sketch002 = startSketchOn(plane001)
 extrude001 = extrude(sketch002, length = 10)
 `
 
+const FEAUTRE_TREE_SKETCH_CODE = `sketch001 = startSketchOn('XZ')
+  |> startProfileAt([0, 0], %)
+  |> angledLine([0, 4], %, $rectangleSegmentA001)
+  |> angledLine([
+       segAng(rectangleSegmentA001) - 90,
+       2
+     ], %, $rectangleSegmentB001)
+  |> angledLine([
+       segAng(rectangleSegmentA001),
+       -segLen(rectangleSegmentA001)
+     ], %, $rectangleSegmentC001)
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+  |> close(%)
+extrude001 = extrude(sketch001, length = 10)
+sketch002 = startSketchOn(extrude001, rectangleSegmentB001)
+  |> circle({
+       center = [-1, 2],
+       radius = .5
+     }, %)
+plane001 = offsetPlane('XZ', -5)
+sketch003 = startSketchOn(plane001)
+  |> circle({ center = [0, 0], radius = 5 }, %)
+`
+
 test.describe('Feature Tree pane', () => {
   test(
     'User can go to definition and go to function definition',
@@ -124,4 +148,267 @@ test.describe('Feature Tree pane', () => {
       })
     }
   )
+
+  test(
+    `User can edit sketch (but not on offset plane yet) from the feature tree`,
+    { tag: '@electron' },
+    async ({ context, homePage, scene, editor, toolbar, page }) => {
+      const unavailableToastMessage = page.getByText(
+        'Editing sketches on faces or offset planes through the feature tree is not yet supported'
+      )
+
+      await context.folderSetupFn(async (dir) => {
+        const bracketDir = join(dir, 'test-sample')
+        await fsp.mkdir(bracketDir, { recursive: true })
+        await fsp.writeFile(
+          join(bracketDir, 'main.kcl'),
+          FEAUTRE_TREE_SKETCH_CODE,
+          'utf-8'
+        )
+      })
+
+      await test.step('setup test', async () => {
+        await homePage.expectState({
+          projectCards: [
+            {
+              title: 'test-sample',
+              fileCount: 1,
+            },
+          ],
+          sortBy: 'last-modified-desc',
+        })
+        await homePage.openProject('test-sample')
+        await scene.waitForExecutionDone()
+        await toolbar.openFeatureTreePane()
+      })
+
+      await test.step('On a default plane should work', async () => {
+        await (await toolbar.getFeatureTreeOperation('Sketch', 0)).dblclick()
+        await expect(
+          toolbar.exitSketchBtn,
+          'We should be in sketch mode now'
+        ).toBeVisible()
+        await editor.expectState({
+          highlightedCode: '',
+          diagnostics: [],
+          activeLines: ["sketch001 = startSketchOn('XZ')"],
+        })
+        await toolbar.exitSketchBtn.click()
+      })
+
+      await test.step('On an extrude face should *not* work', async () => {
+        // Tooltip is getting in the way of clicking, so I'm first closing the pane
+        await toolbar.closeFeatureTreePane()
+        await (await toolbar.getFeatureTreeOperation('Sketch', 1)).dblclick()
+        await expect(
+          unavailableToastMessage,
+          'We should see a toast message about this'
+        ).toBeVisible()
+        await unavailableToastMessage.waitFor({ state: 'detached' })
+        // TODO - turn on once we update the artifactGraph in Rust
+        // to include the proper source location for the extrude face
+        // await expect(
+        //   toolbar.exitSketchBtn,
+        //   'We should be in sketch mode now'
+        // ).toBeVisible()
+        // await editor.expectState({
+        //   highlightedCode: '',
+        //   diagnostics: [],
+        //   activeLines: ['|>circle({center=[-1,2],radius=.5},%)'],
+        // })
+        // await toolbar.exitSketchBtn.click()
+      })
+
+      await test.step('On an offset plane should *not* work', async () => {
+        // Tooltip is getting in the way of clicking, so I'm first closing the pane
+        await toolbar.closeFeatureTreePane()
+        await (await toolbar.getFeatureTreeOperation('Sketch', 2)).dblclick()
+        await editor.expectState({
+          highlightedCode: '',
+          diagnostics: [],
+          activeLines: ['|>circle({center=[0,0],radius=5},%)'],
+        })
+        await expect(
+          toolbar.exitSketchBtn,
+          'We should not be in sketch mode now'
+        ).not.toBeVisible()
+        await expect(
+          page.getByText(
+            'Editing sketches on faces or offset planes through the feature tree is not yet supported'
+          ),
+          'We should see a toast message about this'
+        ).toBeVisible()
+      })
+    }
+  )
+  test(`User can edit an extrude operation from the feature tree`, async ({
+    context,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+    page,
+  }) => {
+    const initialInput = '23'
+    const initialCode = `sketch001 = startSketchOn('XZ')
+      |> circle({ center = [0, 0], radius = 5 }, %)
+      renamedExtrude = extrude(sketch001, length = ${initialInput})`
+    const newConstantName = 'distance001'
+    const expectedCode = `sketch001 = startSketchOn('XZ')
+      |> circle({ center = [0, 0], radius = 5 }, %)
+      ${newConstantName} = 23
+      renamedExtrude = extrude(sketch001, length = ${newConstantName})`
+
+    await context.folderSetupFn(async (dir) => {
+      const testDir = join(dir, 'test-sample')
+      await fsp.mkdir(testDir, { recursive: true })
+      await fsp.writeFile(join(testDir, 'main.kcl'), initialCode, 'utf-8')
+    })
+
+    await test.step('setup test', async () => {
+      await homePage.expectState({
+        projectCards: [
+          {
+            title: 'test-sample',
+            fileCount: 1,
+          },
+        ],
+        sortBy: 'last-modified-desc',
+      })
+      await homePage.openProject('test-sample')
+      await scene.waitForExecutionDone()
+      await toolbar.openFeatureTreePane()
+    })
+
+    await test.step('Double click on the extrude operation', async () => {
+      await (await toolbar.getFeatureTreeOperation('Extrude', 0))
+        .first()
+        .dblclick()
+      await editor.expectState({
+        highlightedCode: '',
+        diagnostics: [],
+        activeLines: [
+          `renamedExtrude = extrude(sketch001, length = ${initialInput})`,
+        ],
+      })
+      await cmdBar.expectState({
+        commandName: 'Extrude',
+        stage: 'arguments',
+        currentArgKey: 'distance',
+        currentArgValue: initialInput,
+        headerArguments: {
+          Selection: '1 face',
+          Distance: initialInput,
+        },
+        highlightedHeaderArg: 'distance',
+      })
+    })
+
+    await test.step('Add a named constant for distance argument and submit', async () => {
+      await expect(cmdBar.currentArgumentInput).toBeVisible()
+      const addVariableButton = page.getByRole('button', {
+        name: 'Create new variable',
+      })
+      await addVariableButton.click()
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Selection: '1 face',
+          // The calculated value is shown in the argument summary
+          Distance: initialInput,
+        },
+        commandName: 'Extrude',
+      })
+      await cmdBar.progressCmdBar()
+      await editor.expectState({
+        highlightedCode: '',
+        diagnostics: [],
+        activeLines: [
+          `renamedExtrude = extrude(sketch001, length = ${newConstantName})`,
+        ],
+      })
+      await editor.expectEditor.toContain(expectedCode, {
+        shouldNormalise: true,
+      })
+    })
+  })
+  test(`User can edit an offset plane operation from the feature tree`, async ({
+    context,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    const testCode = (value: string) => `p = offsetPlane('XY', ${value})`
+    const initialInput = '10'
+    const initialCode = testCode(initialInput)
+    const newInput = '5 + 10'
+    const expectedCode = testCode(newInput)
+    await context.folderSetupFn(async (dir) => {
+      const testDir = join(dir, 'test-sample')
+      await fsp.mkdir(testDir, { recursive: true })
+      await fsp.writeFile(join(testDir, 'main.kcl'), initialCode, 'utf-8')
+    })
+
+    await test.step('setup test', async () => {
+      await homePage.expectState({
+        projectCards: [
+          {
+            title: 'test-sample',
+            fileCount: 1,
+          },
+        ],
+        sortBy: 'last-modified-desc',
+      })
+      await homePage.openProject('test-sample')
+      await scene.waitForExecutionDone()
+      await toolbar.openFeatureTreePane()
+    })
+
+    await test.step('Double click on the offset plane operation', async () => {
+      await (await toolbar.getFeatureTreeOperation('Offset Plane', 0))
+        .first()
+        .dblclick()
+      await editor.expectState({
+        highlightedCode: '',
+        diagnostics: [],
+        activeLines: [initialCode],
+      })
+      await cmdBar.expectState({
+        commandName: 'Offset plane',
+        stage: 'arguments',
+        currentArgKey: 'distance',
+        currentArgValue: initialInput,
+        headerArguments: {
+          Plane: '1 plane',
+          Distance: initialInput,
+        },
+        highlightedHeaderArg: 'distance',
+      })
+    })
+
+    await test.step('Edit the distance argument and submit', async () => {
+      await expect(cmdBar.currentArgumentInput).toBeVisible()
+      await cmdBar.currentArgumentInput.locator('.cm-content').fill(newInput)
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Plane: '1 plane',
+          // We show the calculated value in the argument summary
+          Distance: '15',
+        },
+        commandName: 'Offset plane',
+      })
+      await cmdBar.progressCmdBar()
+      await editor.expectState({
+        highlightedCode: '',
+        diagnostics: [],
+        activeLines: [expectedCode],
+      })
+    })
+  })
 })

e2e/playwright/fixtures/cmdBarFixture.ts

@@ -26,11 +26,18 @@ type CmdBarSerialised =
 export class CmdBarFixture {
   public page: Page
   cmdBarOpenBtn!: Locator
+  cmdBarElement!: Locator
 
   constructor(page: Page) {
     this.page = page
     this.cmdBarOpenBtn = page.getByTestId('command-bar-open-button')
+    this.cmdBarElement = page.getByTestId('command-bar')
   }
+
+  get currentArgumentInput() {
+    return this.page.getByTestId('cmd-bar-arg-value')
+  }
+
   reConstruct = (page: Page) => {
     this.page = page
   }

e2e/playwright/fixtures/sceneFixture.ts

@@ -38,7 +38,10 @@ export class SceneFixture {
   public page: Page
   public streamWrapper!: Locator
   public loadingIndicator!: Locator
-  private exeIndicator!: Locator
+
+  get exeIndicator() {
+    return this.page.getByTestId('model-state-indicator-execution-done')
+  }
 
   constructor(page: Page) {
     this.page = page
@@ -64,7 +67,6 @@ export class SceneFixture {
   reConstruct = (page: Page) => {
     this.page = page
 
-    this.exeIndicator = page.getByTestId('model-state-indicator-execution-done')
     this.streamWrapper = page.getByTestId('stream')
     this.loadingIndicator = this.streamWrapper.getByTestId('loading')
   }

e2e/playwright/fixtures/toolbarFixture.ts

@@ -29,7 +29,6 @@ export class ToolbarFixture {
   createFileBtn!: Locator
   fileCreateToast!: Locator
   filePane!: Locator
-  exeIndicator!: Locator
   treeInputField!: Locator
   /** The sidebar button for the Feature Tree pane */
   featureTreeId = 'feature-tree' as const
@@ -62,15 +61,16 @@ export class ToolbarFixture {
     this.filePane = page.locator('#files-pane')
     this.featureTreePane = page.locator('#feature-tree-pane')
     this.fileCreateToast = page.getByText('Successfully created')
-    this.exeIndicator = page.getByTestId(
-      'model-state-indicator-receive-reliable'
-    )
   }
 
   get logoLink() {
     return this.page.getByTestId('app-logo')
   }
 
+  get exeIndicator() {
+    return this.page.getByTestId('model-state-indicator-receive-reliable')
+  }
+
   startSketchPlaneSelection = async () =>
     doAndWaitForImageDiff(this.page, () => this.startSketchBtn.click(), 500)
 
@@ -139,7 +139,8 @@ export class ToolbarFixture {
   }
 
   /**
-   * Get a specific operation button from the Feature Tree pane
+   * Get a specific operation button from the Feature Tree pane.
+   * Index is 0-based.
    */
   async getFeatureTreeOperation(operationName: string, operationIndex: number) {
     await this.openFeatureTreePane()

e2e/playwright/sketch-tests.spec.ts

@@ -15,6 +15,7 @@ test.describe('Sketch tests', { tag: ['@skipWin'] }, () => {
     page,
     context,
     homePage,
+    scene,
   }) => {
     const u = await getUtils(page)
     const selectionsSnippets = {
@@ -75,6 +76,7 @@ test.describe('Sketch tests', { tag: ['@skipWin'] }, () => {
     await page.setBodyDimensions({ width: 1200, height: 500 })
 
     await homePage.goToModelingScene()
+    await scene.waitForExecutionDone()
 
     // wait for execution done
     await u.openDebugPanel()