Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9001006
WIP: Add point-and-click Import for geometry
pierremtb Apr 2, 2025
3afcb33
Better pathToNOde, log on non-working cm dispatch call
pierremtb Apr 3, 2025
e00f6c2
Add workaround to updateModelingState not working
pierremtb Apr 3, 2025
2bae5ff
Back to updateModelingState with a skip flag
pierremtb Apr 3, 2025
79fd435
Better todo
pierremtb Apr 3, 2025
2d08d47
Update from main
pierremtb Apr 3, 2025
1bb37fc
Change working from Import to Insert, cleanups
pierremtb Apr 3, 2025
9371860
Sister command in kclCommands to populate file options
pierremtb Apr 3, 2025
69a0198
Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…
pierremtb Apr 3, 2025
58c87ac
Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…
pierremtb Apr 4, 2025
9e32ce3
Improve path selector
pierremtb Apr 4, 2025
5d8f7a5
Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…
pierremtb Apr 4, 2025
7b7d8b4
Unsure: move importAstMod to kclCommands onSubmit :no_mouth:
pierremtb Apr 4, 2025
eaf1f09
Add e2e test
pierremtb Apr 4, 2025
8052af8
Clean up for review
pierremtb Apr 4, 2025
efcf164
Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…
pierremtb Apr 4, 2025
8fa8b16
Add native file menu entry and test
pierremtb Apr 4, 2025
5f11d61
No await yo lint said so
pierremtb Apr 4, 2025
4698339
@lrev-Dev's suggestion to remove a comment
pierremtb Apr 7, 2025
a932339
Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…
pierremtb Apr 7, 2025
c5e18ba
Update to scene.settled(cmdBar)
pierremtb Apr 7, 2025
a699e94
Lint
pierremtb Apr 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions e2e/playwright/fixtures/toolbarFixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class ToolbarFixture {
featureTreePane!: Locator
gizmo!: Locator
gizmoDisabled!: Locator
insertButton!: Locator

constructor(page: Page) {
this.page = page
Expand Down Expand Up @@ -78,6 +79,8 @@ export class ToolbarFixture {
// element or two different elements can represent these states.
this.gizmo = page.getByTestId('gizmo')
this.gizmoDisabled = page.getByTestId('gizmo-disabled')

this.insertButton = page.getByTestId('insert-pane-button')
}

get logoLink() {
Expand Down
37 changes: 37 additions & 0 deletions e2e/playwright/native-file-menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,43 @@ test.describe('Native file menu', { tag: ['@electron'] }, () => {
const expected = 'Open sample'
expect(actual).toBe(expected)
})
test('Modeling.File.Insert from project file', async ({
tronApp,
cmdBar,
page,
homePage,
scene,
}) => {
if (!tronApp) {
throwTronAppMissing()
return
}
Comment on lines +580 to +583
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be abstracted into one of the fixtures, and just be a oneLiner, or maybe part of "goToModelingScene"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might leave this with @nadr0 as a separate issue if that's cool. This pattern only exists in e2e/playwright/native-file-menu.spec.ts, not sure how much it should be generalized.

But I agree with the sentiment yes!

await homePage.goToModelingScene()
await scene.settled(cmdBar)

// Run electron snippet to find the Menu!
await page.waitForTimeout(100) // wait for createModelingPageMenu() to run
await tronApp.electron.evaluate(async ({ app }) => {
if (!app || !app.applicationMenu) {
throw new Error('app or app.applicationMenu is missing')
}
const openProject = app.applicationMenu.getMenuItemById(
'File.Insert from project file'
)
if (!openProject) {
throw new Error('File.Insert from project file')
}
openProject.click()
})
// Check that the command bar is opened
await expect(cmdBar.cmdBarElement).toBeVisible()
// Check the placeholder project name exists
const actual = await cmdBar.cmdBarElement
.getByTestId('command-name')
.textContent()
const expected = 'Insert'
expect(actual).toBe(expected)
})
test('Modeling.File.Export current part', async ({
tronApp,
cmdBar,
Expand Down
115 changes: 115 additions & 0 deletions e2e/playwright/point-click-assemblies.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import * as fsp from 'fs/promises'
import path from 'path'

import { executorInputPath } from '@e2e/playwright/test-utils'
import { test } from '@e2e/playwright/zoo-test'

// test file is for testing point an click code gen functionality that's assemblies related
test.describe('Point-and-click assemblies tests', () => {
test(
`Insert kcl part into assembly as whole module import`,
{ tag: ['@electron'] },
async ({
context,
page,
homePage,
scene,
editor,
toolbar,
cmdBar,
tronApp,
}) => {
if (!tronApp) {
fail()
}

// One dumb hardcoded screen pixel value
const testPoint = { x: 575, y: 200 }
const initialColor: [number, number, number] = [50, 50, 50]
const partColor: [number, number, number] = [150, 150, 150]
const tolerance = 50

await test.step('Setup parts and expect empty assembly scene', async () => {
const projectName = 'assembly'
await context.folderSetupFn(async (dir) => {
const bracketDir = path.join(dir, projectName)
await fsp.mkdir(bracketDir, { recursive: true })
await Promise.all([
fsp.copyFile(
executorInputPath('cylinder-inches.kcl'),
path.join(bracketDir, 'cylinder.kcl')
),
fsp.copyFile(
executorInputPath('e2e-can-sketch-on-chamfer.kcl'),
path.join(bracketDir, 'bracket.kcl')
),
fsp.writeFile(path.join(bracketDir, 'main.kcl'), ''),
])
})
await page.setBodyDimensions({ width: 1000, height: 500 })
await homePage.openProject(projectName)
await scene.settled(cmdBar)
await scene.expectPixelColor(initialColor, testPoint, tolerance)
})

await test.step('Insert first part into the assembly', async () => {
await toolbar.insertButton.click()
await cmdBar.selectOption({ name: 'cylinder.kcl' }).click()
await cmdBar.expectState({
stage: 'arguments',
currentArgKey: 'localName',
currentArgValue: '',
headerArguments: { Path: 'cylinder.kcl', LocalName: '' },
highlightedHeaderArg: 'localName',
commandName: 'Insert',
})
await page.keyboard.insertText('cylinder')
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'review',
headerArguments: { Path: 'cylinder.kcl', LocalName: 'cylinder' },
commandName: 'Insert',
})
await cmdBar.progressCmdBar()
await editor.expectEditor.toContain(
`
import "cylinder.kcl" as cylinder
cylinder
`,
{ shouldNormalise: true }
)
await scene.expectPixelColor(partColor, testPoint, tolerance)
})

await test.step('Insert second part into the assembly', async () => {
await toolbar.insertButton.click()
await cmdBar.selectOption({ name: 'bracket.kcl' }).click()
await cmdBar.expectState({
stage: 'arguments',
currentArgKey: 'localName',
currentArgValue: '',
headerArguments: { Path: 'bracket.kcl', LocalName: '' },
highlightedHeaderArg: 'localName',
commandName: 'Insert',
})
await page.keyboard.insertText('bracket')
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'review',
headerArguments: { Path: 'bracket.kcl', LocalName: 'bracket' },
commandName: 'Insert',
})
await cmdBar.progressCmdBar()
await editor.expectEditor.toContain(
`
import "cylinder.kcl" as cylinder
import "bracket.kcl" as bracket
cylinder
bracket
`,
{ shouldNormalise: true }
)
})
}
)
})
22 changes: 21 additions & 1 deletion src/components/FileMachineProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,30 @@ export const FileMachineProvider = ({
name: sample.title,
})),
},
specialPropsForInsertCommand: {
providedOptions: (isDesktop() && project?.children
? project.children
: []
).flatMap((v) => {
// TODO: add support for full tree traversal when KCL support subdir imports
const relativeFilePath = v.path.replace(
project?.path + window.electron.sep,
''
)
const isDirectory = v.children
const isCurrentFile = v.path === file?.path
return isDirectory || isCurrentFile
? []
: {
name: relativeFilePath,
value: relativeFilePath,
}
}),
},
}).filter(
(command) => kclSamples.length || command.name !== 'open-kcl-example'
),
[codeManager, kclManager, send, kclSamples]
[codeManager, kclManager, send, kclSamples, project, file]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This right here is the reason why this command was added in kclCommands and not in modelingMachine as it was at first, since I wanted the path input to be prefilled with project files which is a lot easier and faster to navigate than a full blown filesystem dialog. But I didn't really know how to nicely inject thse dependencies into the modelingMachine, so I went this way instead. Feedback on this super welcome!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nadr0 is in the process of excavating the projectMachine out of React. When that is done, then the inputs will be available anywhere the appMachine is, and it should be easier to get into a modeling command definition. But I think this is understandable as an approach!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok phew that's awesome!

)

useEffect(() => {
Expand Down
15 changes: 15 additions & 0 deletions src/components/ModelingSidebar/ModelingSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import type {
} from '@src/components/ModelingSidebar/ModelingPanes'
import { sidebarPanes } from '@src/components/ModelingSidebar/ModelingPanes'
import Tooltip from '@src/components/Tooltip'
import { DEV } from '@src/env'
import { useModelingContext } from '@src/hooks/useModelingContext'
import { useKclContext } from '@src/lang/KclProvider'
import { SIDEBAR_BUTTON_SUFFIX } from '@src/lib/constants'
import { isDesktop } from '@src/lib/isDesktop'
import { useSettings } from '@src/machines/appMachine'
import { commandBarActor } from '@src/machines/commandBarMachine'
import { onboardingPaths } from '@src/routes/Onboarding/paths'
import { IS_NIGHTLY_OR_DEBUG } from '@src/routes/utils'

interface ModelingSidebarProps {
paneOpacity: '' | 'opacity-20' | 'opacity-40'
Expand Down Expand Up @@ -60,6 +62,19 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
)

const sidebarActions: SidebarAction[] = [
{
id: 'insert',
title: 'Insert from project file',
sidebarName: 'Insert from project file',
icon: 'import',
keybinding: 'Ctrl + Shift + I',
hide: (a) => a.platform === 'web' || !(DEV || IS_NIGHTLY_OR_DEBUG),
action: () =>
commandBarActor.send({
type: 'Find and select command',
data: { name: 'Insert', groupId: 'code' },
}),
},
{
id: 'export',
title: 'Export part',
Expand Down
24 changes: 24 additions & 0 deletions src/components/ProjectSidebarMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ function ProjectMenuPopover({
const commands = useSelector(commandBarActor, commandsSelector)

const { onProjectClose } = useLspContext()
const insertCommandInfo = { name: 'Insert', groupId: 'code' }
const exportCommandInfo = { name: 'Export', groupId: 'modeling' }
const makeCommandInfo = { name: 'Make', groupId: 'modeling' }
const shareCommandInfo = { name: 'share-file-link', groupId: 'code' }
Expand Down Expand Up @@ -145,6 +146,29 @@ function ProjectMenuPopover({
},
},
'break',
{
id: 'insert',
Element: 'button',
children: (
<>
<span>Insert from project file</span>
{!findCommand(insertCommandInfo) && (
<Tooltip
position="right"
wrapperClassName="!max-w-none min-w-fit"
>
Awaiting engine connection
</Tooltip>
)}
</>
),
disabled: !findCommand(insertCommandInfo),
onClick: () =>
commandBarActor.send({
type: 'Find and select command',
data: insertCommandInfo,
}),
},
{
id: 'export',
Element: 'button',
Expand Down
43 changes: 43 additions & 0 deletions src/lang/create.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import type { ImportStatement } from '@rust/kcl-lib/bindings/ImportStatement'
import type { Name } from '@rust/kcl-lib/bindings/Name'
import type { Node } from '@rust/kcl-lib/bindings/Node'
import type { TagDeclarator } from '@rust/kcl-lib/bindings/TagDeclarator'

import type { ImportPath } from '@rust/kcl-lib/bindings/ImportPath'
import type { ImportSelector } from '@rust/kcl-lib/bindings/ImportSelector'
import type { ItemVisibility } from '@rust/kcl-lib/bindings/ItemVisibility'
import { ARG_TAG } from '@src/lang/constants'
import { getNodeFromPath } from '@src/lang/queryAst'
import { getNodePathFromSourceRange } from '@src/lang/queryAstNodePathUtils'
Expand All @@ -12,6 +16,7 @@ import type {
CallExpression,
CallExpressionKw,
Expr,
ExpressionStatement,
Identifier,
LabeledArg,
Literal,
Expand Down Expand Up @@ -333,6 +338,44 @@ export function createBinaryExpressionWithUnary([left, right]: [
return createBinaryExpression([left, '+', right])
}

export function createImportAsSelector(name: string): ImportSelector {
return { type: 'None', alias: createIdentifier(name) }
}

export function createImportStatement(
selector: ImportSelector,
path: ImportPath,
visibility: ItemVisibility = 'default'
): Node<ImportStatement> {
return {
type: 'ImportStatement',
start: 0,
end: 0,
moduleId: 0,
outerAttrs: [],
preComments: [],
commentStart: 0,
selector,
path,
visibility,
}
}

export function createExpressionStatement(
expression: Expr
): Node<ExpressionStatement> {
return {
type: 'ExpressionStatement',
start: 0,
end: 0,
moduleId: 0,
outerAttrs: [],
preComments: [],
commentStart: 0,
expression,
}
}

export function findUniqueName(
ast: Program | string,
name: string,
Expand Down
23 changes: 16 additions & 7 deletions src/lang/modelingWorkflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
EXECUTION_TYPE_NONE,
EXECUTION_TYPE_REAL,
} from '@src/lib/constants'
import type { Selections } from '@src/lib/selections'

/**
* Updates the complete modeling state:
Expand Down Expand Up @@ -52,15 +53,23 @@ export async function updateModelingState(
},
options?: {
focusPath?: Array<PathToNode>
skipUpdateAst?: boolean
}
): Promise<void> {
// Step 1: Update AST without executing (prepare selections)
const updatedAst = await dependencies.kclManager.updateAst(
ast,
// false == mock execution. Is this what we want?
false, // Execution handled separately for error resilience
options
)
let updatedAst: {
newAst: Node<Program>
selections?: Selections
} = { newAst: ast }
// TODO: understand why this skip flag is needed for insertAstMod.
// It's unclear why we double casts the AST
Comment on lines +63 to +64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This took a lot of time from a few of us to debug. Thanks a lot to @franknoirot @nadr0 @lee-at-zoo-corp

The solution below is not perfect but helps unblocking us without changing the code path too much.

if (!options?.skipUpdateAst) {
// Step 1: Update AST without executing (prepare selections)
updatedAst = await dependencies.kclManager.updateAst(
ast,
false, // Execution handled separately for error resilience
options
)
}

// Step 2: Update the code editor and save file
await dependencies.codeManager.updateEditorWithAstAndWriteToFile(
Expand Down
Loading
Loading