Add bidirectional and twist optional args to point-and-click Extrude (#7496)

* WIP: Add bidirectional args to point-and-click Extrude
Will eventually close #7495

* Wire up edit flow for symmetric

* Show skip true args in header in review phase

* Add bidirectionalLength

* Make currentArg always part of header

* WIP

* Add twistAng

* Proper optional args line in review

* Labels in progress button and option arg section heading

* Clean up extrude specific changes

* Clean up to separate from #7506

* Clean up to separate from #7506

* More clean up across branches

* More UI polish

* Remove options bool icon

* Fix labels for tests

* Upgrade e2e tests to cmdBar fixtures with fixes

* More fixes

* Fixed up more tests related to sweep behavior change

* Fix nodeToEdit not having hidden: true on Shell

* Add typecheck

* WIP: footer buttons

* back to reg width

* Clean up

* Update snapshots

* Update snapshots

* Clean up

* Fix tests and remove label

* Refactor

* Fix offset plane test

* Add CommandBarDivider

* Fix step back

* Update snapshots

* Add comment

* Fix it, thanks bot

* Clean up and inline optional heading

* Little case tweak

* Update src/components/CommandBar/CommandBarReview.tsx

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* Rename to CommandBarHeaderFooter

* Rename to CommandBarHeaderFooter

* Clean things up and fix edit

* Add test

* Revert something quick

* Reorg args to match kcl order

* Clean up edit arg retrieval error checks

* Lint

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

e2e/playwright/fixtures/cmdBarFixture.ts

@@ -187,6 +187,13 @@ export class CmdBarFixture {
     return this.page.getByRole('option', options)
   }
 
+  /**
+   * Select an optional argument from the command bar during review
+   */
+  clickOptionalArgument = async (argName: string) => {
+    await this.page.getByTestId(`cmd-bar-add-optional-arg-${argName}`).click()
+  }
+
   /**
    * Clicks the Create new variable button for kcl input
    */

e2e/playwright/point-click.spec.ts

@@ -4923,4 +4923,154 @@ extrude001 = extrude(profile001 length = 1)`
       await editor.expectEditor.toContain(badCode, { shouldNormalise: true })
     })
   })
+
+  test('Point-and-click extrude with optional args', async ({
+    context,
+    page,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    const squareProfileCode = `length001 = 100
+sketch001 = startSketchOn(XY)
+profile001 = startProfile(sketch001, at = [0, 0])
+  |> yLine(length = length001)
+  |> xLine(length = length001)
+  |> yLine(length = -length001)
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+  |> close()
+    `
+    await context.addInitScript((initialCode) => {
+      localStorage.setItem('persistCode', initialCode)
+    }, squareProfileCode)
+    await homePage.goToModelingScene()
+    await scene.settled(cmdBar)
+
+    await test.step('Select through code', async () => {
+      await editor.selectText('startProfile(sketch001, at = [0, 0])')
+    })
+
+    await test.step('Go through command bar flow', async () => {
+      await toolbar.extrudeButton.click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'sketches',
+        currentArgValue: '',
+        headerArguments: {
+          Profiles: '',
+          Length: '',
+        },
+        highlightedHeaderArg: 'Profiles',
+        commandName: 'Extrude',
+      })
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'length',
+        currentArgValue: '5',
+        headerArguments: {
+          Profiles: '1 profile',
+          Length: '',
+        },
+        highlightedHeaderArg: 'length',
+        commandName: 'Extrude',
+      })
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Profiles: '1 profile',
+          Length: '5',
+        },
+        commandName: 'Extrude',
+      })
+      await cmdBar.clickOptionalArgument('bidirectionalLength')
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'bidirectionalLength',
+        currentArgValue: '',
+        headerArguments: {
+          Profiles: '1 profile',
+          Length: '5',
+          BidirectionalLength: '',
+        },
+        highlightedHeaderArg: 'bidirectionalLength',
+        commandName: 'Extrude',
+      })
+      await page.keyboard.insertText('10')
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Profiles: '1 profile',
+          Length: '5',
+          BidirectionalLength: '10',
+        },
+        commandName: 'Extrude',
+      })
+      await cmdBar.submit()
+    })
+
+    await test.step('Check that the code has changed', async () => {
+      await scene.settled(cmdBar)
+      await editor.expectEditor.toContain(
+        `extrude001 = extrude(profile001, length = 5, bidirectionalLength = 10)`,
+        { shouldNormalise: true }
+      )
+    })
+
+    await test.step('Go through the edit flow via feature tree', async () => {
+      await toolbar.openPane('feature-tree')
+      const op = await toolbar.getFeatureTreeOperation('Extrude', 0)
+      await op.dblclick()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'length',
+        currentArgValue: '5',
+        headerArguments: {
+          Length: '5',
+          BidirectionalLength: '10',
+        },
+        highlightedHeaderArg: 'length',
+        commandName: 'Extrude',
+      })
+      await page.keyboard.insertText('10')
+      await cmdBar.progressCmdBar()
+      await page.getByRole('button', { name: 'BidirectionalLength' }).click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'bidirectionalLength',
+        currentArgValue: '10',
+        headerArguments: {
+          Length: '10',
+          BidirectionalLength: '10',
+        },
+        highlightedHeaderArg: 'bidirectionalLength',
+        commandName: 'Extrude',
+      })
+      await page.keyboard.insertText('20')
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Length: '10',
+          BidirectionalLength: '20',
+        },
+        commandName: 'Extrude',
+      })
+      await cmdBar.submit()
+    })
+
+    await test.step('Check that the code has changed again', async () => {
+      await scene.settled(cmdBar)
+      await toolbar.closePane('feature-tree')
+      await toolbar.openPane('code')
+      await editor.expectEditor.toContain(
+        `extrude001 = extrude(profile001, length = 10, bidirectionalLength = 20)`,
+        { shouldNormalise: true }
+      )
+    })
+  })
 })

src/components/CommandBar/CommandBarReview.tsx

@@ -102,7 +102,7 @@ function CommandBarReview({ stepBack }: { stepBack: () => void }) {
               ([argName, arg]) => {
                 return (
                   <button
-                    data-testid="cmd-bar-add-optional-arg"
+                    data-testid={`cmd-bar-add-optional-arg-${argName}`}
                     type="button"
                     onClick={() => {
                       commandBarActor.send({

src/lang/modifyAst/addSweep.ts

@@ -37,11 +37,17 @@ export function addExtrude({
   ast,
   sketches,
   length,
+  symmetric,
+  bidirectionalLength,
+  twistAngle,
   nodeToEdit,
 }: {
   ast: Node<Program>
   sketches: Selections
   length: KclCommandValue
+  symmetric?: boolean
+  bidirectionalLength?: KclCommandValue
+  twistAngle?: KclCommandValue
   nodeToEdit?: PathToNode
 }):
   | {
@@ -63,15 +69,48 @@ export function addExtrude({
     return sketchesExprList
   }
 
+  // Extra labeled args expressions
+  const symmetricExpr = symmetric
+    ? [createLabeledArg('symmetric', createLiteral(symmetric))]
+    : []
+  const bidirectionalLengthExpr = bidirectionalLength
+    ? [
+        createLabeledArg(
+          'bidirectionalLength',
+          valueOrVariable(bidirectionalLength)
+        ),
+      ]
+    : []
+  const twistAngleExpr = twistAngle
+    ? [createLabeledArg('twistAngle', valueOrVariable(twistAngle))]
+    : []
+
   const sketchesExpr = createSketchExpression(sketchesExprList)
   const call = createCallExpressionStdLibKw('extrude', sketchesExpr, [
     createLabeledArg('length', valueOrVariable(length)),
+    ...symmetricExpr,
+    ...bidirectionalLengthExpr,
+    ...twistAngleExpr,
   ])
 
   // Insert variables for labeled arguments if provided
   if ('variableName' in length && length.variableName) {
     insertVariableAndOffsetPathToNode(length, modifiedAst, nodeToEdit)
   }
+  if (
+    bidirectionalLength &&
+    'variableName' in bidirectionalLength &&
+    bidirectionalLength.variableName
+  ) {
+    insertVariableAndOffsetPathToNode(
+      bidirectionalLength,
+      modifiedAst,
+      nodeToEdit
+    )
+  }
+  if (twistAngle && 'variableName' in twistAngle && twistAngle.variableName) {
+    insertVariableAndOffsetPathToNode(twistAngle, modifiedAst, nodeToEdit)
+  }
 
   // 3. If edit, we assign the new function call declaration to the existing node,
   // otherwise just push to the end

src/lib/commandBarConfigs/modelingCommandConfig.ts

@@ -75,6 +75,9 @@ export type ModelingCommandSchema = {
     // KCL stdlib arguments
     sketches: Selections
     length: KclCommandValue
+    symmetric?: boolean
+    bidirectionalLength?: KclCommandValue
+    twistAngle?: KclCommandValue
   }
   Sweep: {
     // Enables editing workflow
@@ -408,6 +411,22 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
         defaultValue: KCL_DEFAULT_LENGTH,
         required: true,
       },
+      symmetric: {
+        inputType: 'options',
+        required: false,
+        options: [
+          { name: 'False', value: false },
+          { name: 'True', value: true },
+        ],
+      },
+      bidirectionalLength: {
+        inputType: 'kcl',
+        required: false,
+      },
+      twistAngle: {
+        inputType: 'kcl',
+        required: false,
+      },
     },
   },
   Sweep: {

src/lib/operations.ts

@@ -25,7 +25,7 @@ import type {
   HelixModes,
   ModelingCommandSchema,
 } from '@src/lib/commandBarConfigs/modelingCommandConfig'
-import type { KclExpression } from '@src/lib/commandTypes'
+import type { KclCommandValue, KclExpression } from '@src/lib/commandTypes'
 import {
   stringToKclExpression,
   retrieveArgFromPipedCallExpression,
@@ -134,12 +134,63 @@ const prepareToEditExtrude: PrepareToEditCallback = async ({ operation }) => {
     return { reason: "Couldn't retrieve length argument" }
   }
 
+  // symmetric argument from a string to boolean
+  let symmetric: boolean | undefined
+  if ('symmetric' in operation.labeledArgs && operation.labeledArgs.symmetric) {
+    symmetric =
+      codeManager.code.slice(
+        operation.labeledArgs.symmetric.sourceRange[0],
+        operation.labeledArgs.symmetric.sourceRange[1]
+      ) === 'true'
+  }
+
+  // bidirectionalLength argument from a string to a KCL expression
+  let bidirectionalLength: KclCommandValue | undefined
+  if (
+    'bidirectionalLength' in operation.labeledArgs &&
+    operation.labeledArgs.bidirectionalLength
+  ) {
+    const result = await stringToKclExpression(
+      codeManager.code.slice(
+        operation.labeledArgs.bidirectionalLength.sourceRange[0],
+        operation.labeledArgs.bidirectionalLength.sourceRange[1]
+      )
+    )
+    if (err(result) || 'errors' in result) {
+      return { reason: "Couldn't retrieve bidirectionalLength argument" }
+    }
+
+    bidirectionalLength = result
+  }
+
+  // twistAngle argument from a string to a KCL expression
+  let twistAngle: KclCommandValue | undefined
+  if (
+    'twistAngle' in operation.labeledArgs &&
+    operation.labeledArgs.twistAngle
+  ) {
+    const result = await stringToKclExpression(
+      codeManager.code.slice(
+        operation.labeledArgs.twistAngle.sourceRange[0],
+        operation.labeledArgs.twistAngle.sourceRange[1]
+      )
+    )
+    if (err(result) || 'errors' in result) {
+      return { reason: "Couldn't retrieve twistAngle argument" }
+    }
+
+    twistAngle = result
+  }
+
   // 3. Assemble the default argument values for the command,
   // with `nodeToEdit` set, which will let the actor know
   // to edit the node that corresponds to the StdLibCall.
   const argDefaultValues: ModelingCommandSchema['Extrude'] = {
     sketches,
     length,
+    symmetric,
+    bidirectionalLength,
+    twistAngle,
     nodeToEdit: pathToNodeFromRustNodePath(operation.nodePath),
   }
   return {

src/machines/commandBarMachine.ts

@@ -143,6 +143,7 @@ export const commandBarMachine = setup({
               : !a[1].hidden
         )
         let argIndex = 0
+        let lastRequiredArg: CommandArgumentWithName<unknown> | undefined
 
         while (argIndex < nonHiddenArgs.length) {
           const [argName, argConfig] = nonHiddenArgs[argIndex]
@@ -150,13 +151,14 @@ export const commandBarMachine = setup({
             typeof argConfig.required === 'function'
               ? argConfig.required(context)
               : argConfig.required
-          /**
-           * TODO: we need to think harder about the relationship between
-           * `required`, `skip`, and `hidden`.
-           * This bit of logic essentially makes "skip false" arguments required.
-           * We may need a bit of state to mark an argument as "visited" for "skip false" args
-           * to truly not require any value to continue.
-           */
+
+          if (argIsRequired) {
+            lastRequiredArg = {
+              ...argConfig,
+              name: argName,
+            }
+          }
+
           const mustNotSkipArg =
             (argIsRequired || argConfig.skip === false) &&
             (!context.argumentsToSubmit.hasOwnProperty(argName) ||
@@ -166,12 +168,21 @@ export const commandBarMachine = setup({
                 'name' in rejectedArg &&
                 rejectedArg.name === argName))
 
-          if (
-            mustNotSkipArg === true ||
+          if (mustNotSkipArg) {
+            return {
+              ...selectedCommand.args[argName],
+              name: argName,
+            }
+          }
+
+          const reachedEndOfArgs =
             argIndex + 1 === Object.keys(nonHiddenArgs).length
-          ) {
-            // If we have reached the end of the arguments and none are skippable,
-            // return the last argument.
+          if (reachedEndOfArgs) {
+            if (lastRequiredArg) {
+              return lastRequiredArg
+            }
+
+            // Default to the last argument that is not hidden
             return {
               ...selectedCommand.args[argName],
               name: argName,

src/machines/modelingMachine.ts

@@ -2435,12 +2435,22 @@ export const modelingMachine = setup({
           return Promise.reject(new Error(NO_INPUT_PROVIDED_MESSAGE))
         }
 
-        const { nodeToEdit, sketches, length } = input
+        const {
+          nodeToEdit,
+          sketches,
+          length,
+          symmetric,
+          bidirectionalLength,
+          twistAngle,
+        } = input
         const { ast } = kclManager
         const astResult = addExtrude({
           ast,
           sketches,
           length,
+          symmetric,
+          bidirectionalLength,
+          twistAngle,
           nodeToEdit,
         })
         if (err(astResult)) {