ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add edit flow for named constants / parameters (#5911)

Browse Source 
* Add support for forcing kcl input create variable

* Command palette padding tweak

* Make traverse function work for ExpressionStatements

* Add utilities for getting earliest safe index in AST

* Fix the insertIndex logic to not be based on the selection anymore

* Add workflow to create a named constant

* Fix bug with nameEndInDigits matcher

* Tweak command config

* Add a three-dot menu to feature tree pane to create parameters

* Add E2E test for create parameter flow

* Remove edit flow oops

* Fix tsc error

* Fix E2E test

* Update named constant position in edit flow test

* Add tags into consideration for safe insert index

Per @Irev-dev's helpful feedback, with unit tests!

* Fix tsc by removing a generic type

* Remove unused imports

* Fix lints

* A snapshot a day keeps the bugs away! 📷🐛

* Add utilities for working with variable declarations

* Add "edit parameter" user flow

* Add edit flow config

* WIP working on de-bloating useCalculateKclExpreesion

* Add the ability to specify a `displayName` for an arg

* Add utility to type check on SourceRanges

* Review step design tweak fixes

* Refactor useCalculateKclExpression to take a sourceRange

* Make option arg validation work for objects and arrays

Using an admittedly dumb stringification approach

* Make edit flow never move the constant to be edited

* Add E2E test section

* Fix lints

* Remove lying comment, tiny CSS tweak

* A snapshot a day keeps the bugs away! 📷🐛

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Frank Noirot
2025-03-20 16:41:09 -04:00
committed by GitHub
parent 2c6404f671
commit 9da8574103
18 changed files with 301 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
e2e/playwright/command-bar-tests.spec.ts
View File

@ -489,7 +489,7 @@ test.describe('Command bar tests', { tag: ['@skipWin'] }, () => {
})
})
test(`Can add a named parameter or constant`, async ({
test(`Can add and edit a named parameter or constant`, async ({
page,
homePage,
context,
@ -511,7 +511,7 @@ c = 3 + a`
// but you do because all modeling commands have that requirement
await scene.settled(cmdBar)
await test.step(`Go through the command palette flow`, async () => {
await test.step(`Create a parameter via command bar`, async () => {
await cmdBar.cmdBarOpenBtn.click()
await cmdBar.chooseCommand('create parameter')
await cmdBar.expectState({
@ -536,5 +536,57 @@ c = 3 + a`
await editor.expectEditor.toContain(
`a = 5b = a * amyParameter001 = b - 5c = 3 + a`
)
const newValue = `2 * b + a`
await test.step(`Edit the parameter via command bar`, async () => {
await cmdBar.cmdBarOpenBtn.click()
await cmdBar.chooseCommand('edit parameter')
await cmdBar.expectState({
stage: 'arguments',
commandName: 'Edit parameter',
currentArgKey: 'Name',
currentArgValue: '',
headerArguments: {
Name: '',
Value: '',
},
highlightedHeaderArg: 'Name',
})
await cmdBar
.selectOption({
name: 'myParameter001',
})
.click()
await cmdBar.expectState({
stage: 'arguments',
commandName: 'Edit parameter',
currentArgKey: 'value',
currentArgValue: 'b - 5',
headerArguments: {
Name: 'myParameter001',
Value: '',
},
highlightedHeaderArg: 'value',
})
await cmdBar.argumentInput.locator('[contenteditable]').fill(newValue)
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'review',
commandName: 'Edit parameter',
headerArguments: {
Name: 'myParameter001',
// KCL inputs show the *computed* value, not the input value, in the command palette header
Value: '55',
},
})
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'commandBarClosed',
})
})
await editor.expectEditor.toContain(
`a = 5b = a * amyParameter001 = ${newValue}c = 3 + a`
)
})
})

BIN
e2e/playwright/snapshot-tests.spec.ts-snapshots/code-color-goober-code-color-goober-1-Google-Chrome-linux.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 143 KiB

After

Width:  |  Height:  |  Size: 143 KiB

4
src/components/CommandBar/CommandBarArgument.tsx
View File

@ -54,7 +54,7 @@ function ArgumentInput({
return (
<CommandArgOptionInput
arg={arg}
argName={arg.name}
argName={arg.displayName || arg.name}
stepBack={stepBack}
onSubmit={onSubmit}
placeholder="Select an option"
@ -71,7 +71,7 @@ function ArgumentInput({
{ name: 'Off', value: false },
],
}}
argName={arg.name}
argName={arg.displayName || arg.name}
stepBack={stepBack}
onSubmit={onSubmit}
placeholder="Select an option"

2
src/components/CommandBar/CommandBarBasicInput.tsx
View File

@ -38,7 +38,7 @@ function CommandBarBasicInput({
className="flex items-center mx-4 my-4"
>
<span className="capitalize px-2 py-1 rounded-l bg-chalkboard-100 dark:bg-chalkboard-80 text-chalkboard-10 border-b border-b-chalkboard-100 dark:border-b-chalkboard-80">
{arg.name}
{arg.displayName || arg.name}
</span>
<input
data-testid="cmd-bar-arg-value"

2
src/components/CommandBar/CommandBarHeader.tsx
View File

@ -130,7 +130,7 @@ function CommandBarHeader({ children }: React.PropsWithChildren<{}>) {
data-test-name="arg-name"
className="capitalize"
>
{argName}
{arg.displayName || argName}
</span>
<span className="sr-only">:&nbsp;</span>
<span data-testid="header-arg-value">

38
src/components/CommandBar/CommandBarKclInput.tsx
View File

@ -21,10 +21,16 @@ import { useSelector } from '@xstate/react'
import { commandBarActor, useCommandBarState } from 'machines/commandBarMachine'
import { useSettings } from 'machines/appMachine'
import toast from 'react-hot-toast'
import { AnyStateMachine, SnapshotFrom } from 'xstate'
import { kclManager } from 'lib/singletons'
import { getNodeFromPath } from 'lang/queryAst'
import { isPathToNode, SourceRange, VariableDeclarator } from 'lang/wasm'
import { Node } from '@rust/kcl-lib/bindings/Node'
import { err } from 'lib/trap'
const machineContextSelector = (snapshot?: {
context: Record<string, unknown>
}) => snapshot?.context
// TODO: remove the need for this selector once we decouple all actors from React
const machineContextSelector = (snapshot?: SnapshotFrom<AnyStateMachine>) =>
snapshot?.context
function CommandBarKclInput({
arg,
@ -47,6 +53,16 @@ function CommandBarKclInput({
arg.machineActor,
machineContextSelector
)
const sourceRangeForPrevVariables = useMemo<SourceRange | undefined>(() => {
const nodeToEdit = commandBarState.context.argumentsToSubmit.nodeToEdit
const pathToNode = isPathToNode(nodeToEdit) ? nodeToEdit : undefined
const node = pathToNode
? getNodeFromPath<Node<VariableDeclarator>>(kclManager.ast, pathToNode)
: undefined
return !err(node) && node && node.node.type === 'VariableDeclarator'
? [node.node.start, node.node.end, node.node.moduleId]
: undefined
}, [kclManager.ast, commandBarState.context.argumentsToSubmit.nodeToEdit])
const defaultValue = useMemo(
() =>
arg.defaultValue
@ -90,21 +106,22 @@ function CommandBarKclInput({
const editorRef = useRef<HTMLDivElement>(null)
const {
prevVariables,
calcResult,
newVariableInsertIndex,
valueNode,
newVariableName,
setNewVariableName,
isNewVariableNameUnique,
prevVariables,
} = useCalculateKclExpression({
value,
initialVariableName,
sourceRange: sourceRangeForPrevVariables,
})
const varMentionData: Completion[] = prevVariables.map((v) => ({
label: v.key,
detail: String(roundOff(v.value as number)),
detail: String(roundOff(Number(v.value))),
}))
const varMentionsExtension = varMentions(varMentionData)
@ -219,13 +236,18 @@ function CommandBarKclInput({
}
return (
<form id="arg-form" onSubmit={handleSubmit} data-can-submit={canSubmit}>
<form
id="arg-form"
className="mb-2"
onSubmit={handleSubmit}
data-can-submit={canSubmit}
>
<label className="flex gap-4 items-center mx-4 my-4 border-solid border-b border-chalkboard-50">
<span
data-testid="cmd-bar-arg-name"
className="capitalize text-chalkboard-80 dark:text-chalkboard-20"
>
{arg.name}
{arg.displayName || arg.name}
</span>
<div
data-testid="cmd-bar-arg-value"
@ -249,7 +271,7 @@ function CommandBarKclInput({
</span>
</label>
{createNewVariable ? (
<div className="flex mb-2 items-baseline gap-4 mx-4 border-solid border-0 border-b border-chalkboard-50">
<div className="flex items-baseline gap-4 mx-4 border-solid border-0 border-b border-chalkboard-50">
<label
htmlFor="variable-name"
className="text-base text-chalkboard-80 dark:text-chalkboard-20"

4
src/components/CommandBar/CommandBarReview.tsx
View File

@ -58,7 +58,7 @@ function CommandBarReview({ stepBack }: { stepBack: () => void }) {
return (
<CommandBarHeader>
<p className="px-4">
<p className="px-4 pb-2">
{selectedCommand?.reviewMessage ? (
selectedCommand.reviewMessage instanceof Function ? (
selectedCommand.reviewMessage(commandBarState.context)
@ -66,7 +66,7 @@ function CommandBarReview({ stepBack }: { stepBack: () => void }) {
selectedCommand.reviewMessage
)
) : (
<>Confirm {selectedCommand?.name}</>
<>Confirm {selectedCommand?.displayName || selectedCommand?.name}</>
)}
</p>
<form

2
src/components/CommandBar/CommandBarTextareaInput.tsx
View File

@ -40,7 +40,7 @@ function CommandBarTextareaInput({
data-testid="cmd-bar-arg-name"
className="capitalize px-2 py-1 rounded-br bg-chalkboard-100 dark:bg-chalkboard-80 text-chalkboard-10 border-b border-b-chalkboard-100 dark:border-b-chalkboard-80"
>
{arg.name}
{arg.displayName || arg.name}
</span>
<textarea
data-testid="cmd-bar-arg-value"

5
src/lang/queryAst/getSafeInsertIndex.ts
View File

@ -21,11 +21,6 @@ export function getSafeInsertIndex(
}, 0)
const tagDeclarators = getTagDeclaratorsInProgram(program)
console.log('FRANK tagDeclarators', {
identifiers,
tagDeclarators,
targetExpr,
})
const safeTagIndex = tagDeclarators.reduce((acc, curr) => {
return identifiers.findIndex((a) => a.name === curr.tag.value) === -1
? acc

22
src/lang/queryAst/getVariableDeclaration.ts Normal file
View File

@ -0,0 +1,22 @@
import { Node } from '@rust/kcl-lib/bindings/Node'
import { Program } from 'lang/wasm'
/**
* Given a program and a variable name, return the variable declaration
* if it exists.
*/
export function getVariableDeclaration(
program: Node<Program>,
variableName: string
) {
const foundItem = program.body.find(
(item) =>
item.type === 'VariableDeclaration' &&
item.declaration.id.name === variableName
)
if (foundItem?.type === 'VariableDeclaration') {
return foundItem
} else {
return undefined
}
}

17
src/lang/queryAst/getVariableDeclarationIndex.ts Normal file
View File

@ -0,0 +1,17 @@
import { Node } from '@rust/kcl-lib/bindings/Node'
import { Program } from 'lang/wasm'
/**
* Given a program and a variable name, return the index of the variable declaration
* in the body, returning `-1` if it doesn't exist.
*/
export function getVariableDeclarationIndex(
program: Node<Program>,
variableName: string
) {
return program.body.findIndex(
(item) =>
item.type === 'VariableDeclaration' &&
item.declaration.id.name === variableName
)
}

8
src/lang/wasm.ts
View File

@ -54,6 +54,7 @@ import { UnitLen } from '@rust/kcl-lib/bindings/UnitLen'
import { UnitAngle as UnitAng } from '@rust/kcl-lib/bindings/UnitAngle'
import { ModulePath } from '@rust/kcl-lib/bindings/ModulePath'
import { DefaultPlanes } from '@rust/kcl-lib/bindings/DefaultPlanes'
import { isArray } from 'lib/utils'
export type { Artifact } from '@rust/kcl-lib/bindings/Artifact'
export type { ArtifactCommand } from '@rust/kcl-lib/bindings/Artifact'
@ -285,6 +286,13 @@ export const isPathToNodeNumber = (
return typeof pathToNode === 'number'
}
export const isPathToNode = (input: unknown): input is PathToNode =>
isArray(input) &&
isArray(input[0]) &&
input[0].length == 2 &&
(typeof input[0][0] === 'number' || typeof input[0][0] === 'string') &&
typeof input[0][1] === 'string'
export interface ExecState {
variables: { [key in string]?: KclValue }
operations: Operation[]

87
src/lib/commandBarConfigs/modelingCommandConfig.ts
View File

@ -1,12 +1,17 @@
import { Models } from '@kittycad/lib'
import { angleLengthInfo } from 'components/Toolbar/setAngleLength'
import { transformAstSketchLines } from 'lang/std/sketchcombos'
import { PathToNode } from 'lang/wasm'
import {
isPathToNode,
PathToNode,
SourceRange,
VariableDeclarator,
} from 'lang/wasm'
import { StateMachineCommandSetConfig, KclCommandValue } from 'lib/commandTypes'
import { KCL_DEFAULT_LENGTH, KCL_DEFAULT_DEGREE } from 'lib/constants'
import { components } from 'lib/machine-api'
import { Selections } from 'lib/selections'
import { kclManager } from 'lib/singletons'
import { codeManager, kclManager } from 'lib/singletons'
import { err } from 'lib/trap'
import { modelingMachine, SketchTool } from 'machines/modelingMachine'
import {
@ -15,6 +20,9 @@ import {
shellValidator,
sweepValidator,
} from './validators'
import { getVariableDeclaration } from 'lang/queryAst/getVariableDeclaration'
import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils'
import { getNodeFromPath } from 'lang/queryAst'
type OutputFormat = Models['OutputFormat_type']
type OutputTypeKey = OutputFormat['type']
@ -95,6 +103,10 @@ export type ModelingCommandSchema = {
'event.parameter.create': {
value: KclCommandValue
}
'event.parameter.edit': {
nodeToEdit: PathToNode
value: KclCommandValue
}
'change tool': {
tool: SketchTool
}
@ -601,6 +613,77 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
},
},
},
'event.parameter.edit': {
displayName: 'Edit parameter',
description: 'Edit the value of a named constant',
icon: 'make-variable',
status: 'development',
needsReview: false,
args: {
nodeToEdit: {
displayName: 'Name',
inputType: 'options',
valueSummary: (nodeToEdit: PathToNode) => {
const node = getNodeFromPath<VariableDeclarator>(
kclManager.ast,
nodeToEdit
)
if (err(node) || node.node.type !== 'VariableDeclarator')
return 'Error'
return node.node.id.name || ''
},
required: true,
options() {
return (
Object.entries(kclManager.execState.variables)
// TODO: @franknoirot && @jtran would love to make this go away soon 🥺
.filter(([_, variable]) => variable?.type === 'Number')
.map(([name, variable]) => {
const node = getVariableDeclaration(kclManager.ast, name)
if (node === undefined) return
const range: SourceRange = [node.start, node.end, node.moduleId]
const pathToNode = getNodePathFromSourceRange(
kclManager.ast,
range
)
return {
name,
value: pathToNode,
}
})
.filter((a) => !!a) || []
)
},
},
value: {
inputType: 'kcl',
required: true,
defaultValue(commandBarContext) {
const nodeToEdit = commandBarContext.argumentsToSubmit.nodeToEdit
if (!nodeToEdit || !isPathToNode(nodeToEdit)) return '5'
const node = getNodeFromPath<VariableDeclarator>(
kclManager.ast,
nodeToEdit
)
if (err(node) || node.node.type !== 'VariableDeclarator')
return 'Error'
const variableName = node.node.id.name || ''
if (typeof variableName !== 'string') return '5'
const variableNode = getVariableDeclaration(
kclManager.ast,
variableName
)
if (!variableNode) return '5'
const code = codeManager.code.slice(
variableNode.declaration.init.start,
variableNode.declaration.init.end
)
return code
},
createVariable: 'disallow',
},
},
},
'Constrain length': {
description: 'Constrain the length of one or more segments.',
icon: 'dimension',

2
src/lib/commandTypes.ts
View File

@ -114,6 +114,7 @@ export type CommandArgumentConfig<
OutputType,
C = ContextFrom<AnyStateMachine>
> = {
displayName?: string
description?: string
required:
| boolean
@ -236,6 +237,7 @@ export type CommandArgument<
OutputType,
T extends AnyStateMachine = AnyStateMachine
> = {
displayName?: string
description?: string
required:
| boolean

1
src/lib/createMachineCommand.ts
View File

@ -160,6 +160,7 @@ export function buildCommandArgument<
// GOTCHA: modelingCommandConfig is not a 1:1 mapping to this baseCommandArgument
// You need to manually add key/value pairs here.
const baseCommandArgument = {
displayName: arg.displayName,
description: arg.description,
required: arg.required,
hidden: arg.hidden,

12
src/lib/useCalculateKclExpression.ts
View File

@ -3,7 +3,7 @@ import { kclManager } from 'lib/singletons'
import { useKclContext } from 'lang/KclProvider'
import { findUniqueName } from 'lang/modifyAst'
import { PrevVariable, findAllPreviousVariables } from 'lang/queryAst'
import { Expr } from 'lang/wasm'
import { Expr, SourceRange } from 'lang/wasm'
import { useEffect, useRef, useState } from 'react'
import { getCalculatedKclExpressionValue } from './kclHelpers'
import { parse, resultIsOk } from 'lang/wasm'
@ -21,9 +21,11 @@ const isValidVariableName = (name: string) =>
export function useCalculateKclExpression({
value,
initialVariableName: valueName = '',
sourceRange,
}: {
value: string
initialVariableName?: string
sourceRange?: SourceRange
}): {
inputRef: React.RefObject<HTMLInputElement>
valueNode: Expr | null
@ -41,6 +43,9 @@ export function useCalculateKclExpression({
const selectionRange:
| (typeof context)['selectionRanges']['graphSelections'][number]['codeRef']['range']
| undefined = context.selectionRanges.graphSelections[0]?.codeRef?.range
// If there is no selection, use the end of the code
const endingSourceRange = sourceRange ||
selectionRange || [code.length, code.length]
const inputRef = useRef<HTMLInputElement>(null)
const [availableVarInfo, setAvailableVarInfo] = useState<
ReturnType<typeof findAllPreviousVariables>
@ -94,11 +99,10 @@ export function useCalculateKclExpression({
const varInfo = findAllPreviousVariables(
kclManager.ast,
kclManager.variables,
// If there is no selection, use the end of the code
selectionRange || [code.length, code.length]
endingSourceRange
)
setAvailableVarInfo(varInfo)
}, [kclManager.ast, kclManager.variables, selectionRange])
}, [kclManager.ast, kclManager.variables, endingSourceRange])
useEffect(() => {
const execAstAndSetResult = async () => {

14
src/machines/commandBarMachine.ts
View File

@ -250,6 +250,7 @@ export const commandBarMachine = setup({
},
guards: {
'Command needs review': ({ context }) =>
// Edit flows are (for now) always considered to need review
context.selectedCommand?.needsReview ||
('nodeToEdit' in context.argumentsToSubmit &&
context.argumentsToSubmit.nodeToEdit !== undefined) ||
@ -376,7 +377,18 @@ export const commandBarMachine = setup({
argConfig.machineActor?.getSnapshot().context
)
: argConfig.options
).some((o) => o.value === argValue)
).some((o) => {
// Objects are only equal by reference in JavaScript, so we compare stringified values.
// GOTCHA: this means that JS class instances will behave badly as option arg values I believe.
if (
typeof o.value === 'object' &&
typeof argValue === 'object'
) {
return JSON.stringify(o.value) === JSON.stringify(argValue)
} else {
return o.value === argValue
}
})
if (
hasMismatchedDefaultValueType ||

54
src/machines/modelingMachine.ts
View File

@ -97,6 +97,8 @@ import { createProfileStartHandle } from 'clientSideScene/segments'
import { DRAFT_POINT } from 'clientSideScene/sceneInfra'
import { setAppearance } from 'lang/modifyAst/setAppearance'
import { DRAFT_DASHED_LINE } from 'clientSideScene/sceneEntities'
import { Node } from '@rust/kcl-lib/bindings/Node'
import { updateModelingState } from 'lang/modelingWorkflows'
export const MODELING_PERSIST_KEY = 'MODELING_PERSIST_KEY'
@ -311,6 +313,10 @@ export type ModelingMachineEvent =
type: 'event.parameter.create'
data: ModelingCommandSchema['event.parameter.create']
}
| {
type: 'event.parameter.edit'
data: ModelingCommandSchema['event.parameter.edit']
}
| { type: 'Export'; data: ModelingCommandSchema['Export'] }
| { type: 'Make'; data: ModelingCommandSchema['Make'] }
| { type: 'Extrude'; data?: ModelingCommandSchema['Extrude'] }
@ -2323,6 +2329,39 @@ export const modelingMachine = setup({
}
}
),
'actor.parameter.edit': fromPromise(
async ({
input,
}: {
input: ModelingCommandSchema['event.parameter.edit'] | undefined
}) => {
if (!input) return new Error('No input provided')
// Get the variable AST node to edit
const { nodeToEdit, value } = input
const newAst = structuredClone(kclManager.ast)
const variableNode = getNodeFromPath<Node<VariableDeclarator>>(
newAst,
nodeToEdit
)
if (
err(variableNode) ||
variableNode.node.type !== 'VariableDeclarator' ||
!variableNode.node
) {
return new Error('No variable found, this is a bug')
}
// Mutate the variable's value
variableNode.node.init = value.valueAst
await updateModelingState(newAst, {
codeManager,
editorManager,
kclManager,
})
}
),
'set-up-draft-circle': fromPromise(
async (_: {
input: Pick<ModelingMachineContext, 'sketchDetails'> & {
@ -2574,6 +2613,9 @@ export const modelingMachine = setup({
'event.parameter.create': {
target: '#Modeling.parameter.creating',
},
'event.parameter.edit': {
target: '#Modeling.parameter.editing',
},
Export: {
target: 'Exporting',
@ -3893,6 +3935,18 @@ export const modelingMachine = setup({
onError: ['#Modeling.idle'],
},
},
editing: {
invoke: {
src: 'actor.parameter.edit',
id: 'actor.parameter.edit',
input: ({ event }) => {
if (event.type !== 'event.parameter.edit') return undefined
return event.data
},
onDone: ['#Modeling.idle'],
onError: ['#Modeling.idle'],
},
},
},
},