Skip to content

Fix deletion of unassigned standalone edge treatments #6815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
May 10, 2025
Merged

Fix deletion of unassigned standalone edge treatments #6815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1686725
oops, make it nicer for no reason
max-mrgrsk May 9, 2025
0c0165d
tests
max-mrgrsk May 9, 2025
8ccc565
deleteTopLevelStatement
max-mrgrsk May 9, 2025
17d0f8c
little swap
max-mrgrsk May 9, 2025
51e1e94
astMod edits
max-mrgrsk May 9, 2025
d2c5e7d
typos
max-mrgrsk May 9, 2025
6875ad5
Merge branch 'main' into max/undeletable-unassigned-fillet
max-mrgrsk May 9, 2025
f6a0235
add playwright test for chamfers
max-mrgrsk May 10, 2025
a2343fc
scene.settled instead of page.waitForTimeout
max-mrgrsk May 10, 2025
1da7d09
Merge branch 'main' into max/undeletable-unassigned-fillet
max-mrgrsk May 10, 2025
4434e8a
unfuck circular dep - move locateExtrudeDeclarator
max-mrgrsk May 10, 2025
dc49105
Merge branch 'max/undeletable-unassigned-fillet' of https://github.co…
max-mrgrsk May 10, 2025
66fba3b
locateExtrudeDeclarator > locateVariableWithCallOrPipe
max-mrgrsk May 10, 2025
19d4ad6
fmt
max-mrgrsk May 10, 2025
f5fdad0
Merge branch 'main' into max/undeletable-unassigned-fillet
max-mrgrsk May 10, 2025
b0371cc
edit the comment
max-mrgrsk May 10, 2025
792a101
Merge branch 'max/undeletable-unassigned-fillet' of https://github.co…
max-mrgrsk May 10, 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
150 changes: 110 additions & 40 deletions e2e/playwright/point-click.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2388,6 +2388,7 @@ fillet001 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg01)])
scene,
editor,
toolbar,
cmdBar,
}) => {
// Code samples
const initialCode = `sketch001 = startSketchOn(XY)
Expand All @@ -2401,14 +2402,14 @@ extrude001 = extrude(sketch001, length = -12)
|> fillet(radius = 5, tags = [seg01]) // fillet01
|> fillet(radius = 5, tags = [seg02]) // fillet02
fillet03 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg01)])
fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
`
const pipedFilletDeclaration = 'fillet(radius = 5, tags = [seg01])'
const firstPipedFilletDeclaration = 'fillet(radius = 5, tags = [seg01])'
const secondPipedFilletDeclaration = 'fillet(radius = 5, tags = [seg02])'
const standaloneFilletDeclaration =
const standaloneAssignedFilletDeclaration =
'fillet03 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg01)])'
const secondStandaloneFilletDeclaration =
'fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])'
const standaloneUnassignedFilletDeclaration =
'fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])'

// Locators
const pipedFilletEdgeLocation = { x: 600, y: 193 }
Expand All @@ -2430,6 +2431,7 @@ fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
}, initialCode)
await page.setBodyDimensions({ width: 1000, height: 500 })
await homePage.goToModelingScene()
await scene.settled(cmdBar)

// verify modeling scene is loaded
await scene.expectPixelColor(
Expand All @@ -2446,15 +2448,19 @@ fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
await test.step('Delete fillet via feature tree selection', async () => {
await test.step('Open Feature Tree Pane', async () => {
await toolbar.openPane('feature-tree')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})

await test.step('Delete piped fillet via feature tree selection', async () => {
await test.step('Verify all fillets are present in the editor', async () => {
await editor.expectEditor.toContain(pipedFilletDeclaration)
await editor.expectEditor.toContain(firstPipedFilletDeclaration)
await editor.expectEditor.toContain(secondPipedFilletDeclaration)
await editor.expectEditor.toContain(standaloneFilletDeclaration)
await editor.expectEditor.toContain(secondStandaloneFilletDeclaration)
await editor.expectEditor.toContain(
standaloneAssignedFilletDeclaration
)
await editor.expectEditor.toContain(
standaloneUnassignedFilletDeclaration
)
})
await test.step('Verify test fillets are present in the scene', async () => {
await scene.expectPixelColor(
Expand All @@ -2475,13 +2481,17 @@ fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})
await test.step('Verify piped fillet is deleted but other fillets are not (in the editor)', async () => {
await editor.expectEditor.not.toContain(pipedFilletDeclaration)
await editor.expectEditor.not.toContain(firstPipedFilletDeclaration)
await editor.expectEditor.toContain(secondPipedFilletDeclaration)
await editor.expectEditor.toContain(standaloneFilletDeclaration)
await editor.expectEditor.toContain(secondStandaloneFilletDeclaration)
await editor.expectEditor.toContain(
standaloneAssignedFilletDeclaration
)
await editor.expectEditor.toContain(
standaloneUnassignedFilletDeclaration
)
})
await test.step('Verify piped fillet is deleted but non-piped is not (in the scene)', async () => {
await scene.expectPixelColor(
Expand All @@ -2497,22 +2507,51 @@ fillet04 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg02)])
})
})

await test.step('Delete non-piped fillet via feature tree selection', async () => {
await test.step('Delete non-piped fillet', async () => {
await test.step('Delete standalone assigned fillet via feature tree selection', async () => {
await test.step('Delete standalone assigned fillet', async () => {
const operationButton = await toolbar.getFeatureTreeOperation(
'Fillet',
1
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})
await test.step('Verify standalone assigned fillet is deleted but other two fillets are not (in the editor)', async () => {
await editor.expectEditor.toContain(secondPipedFilletDeclaration)
await editor.expectEditor.not.toContain(
standaloneAssignedFilletDeclaration
)
await editor.expectEditor.toContain(
standaloneUnassignedFilletDeclaration
)
})
await test.step('Verify standalone assigned fillet is deleted but piped is not (in the scene)', async () => {
await scene.expectPixelColor(
edgeColorWhite,
standaloneFilletEdgeLocation,
lowTolerance
)
})
})

await test.step('Delete standalone unassigned fillet via feature tree selection', async () => {
await test.step('Delete standalone unassigned fillet', async () => {
const operationButton = await toolbar.getFeatureTreeOperation(
'Fillet',
1
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await scene.settled(cmdBar)
})
await test.step('Verify non-piped fillet is deleted but other two fillets are not (in the editor)', async () => {
await test.step('Verify standalone unassigned fillet is deleted but other fillet is not (in the editor)', async () => {
await editor.expectEditor.toContain(secondPipedFilletDeclaration)
await editor.expectEditor.not.toContain(standaloneFilletDeclaration)
await editor.expectEditor.toContain(secondStandaloneFilletDeclaration)
await editor.expectEditor.not.toContain(
standaloneUnassignedFilletDeclaration
)
})
await test.step('Verify non-piped fillet is deleted but piped is not (in the scene)', async () => {
await test.step('Verify standalone unassigned fillet is deleted but piped is not (in the scene)', async () => {
await scene.expectPixelColor(
edgeColorWhite,
standaloneFilletEdgeLocation,
Expand Down Expand Up @@ -2964,14 +3003,14 @@ extrude001 = extrude(sketch001, length = -12)
|> chamfer(length = 5, tags = [seg01]) // chamfer01
|> chamfer(length = 5, tags = [seg02]) // chamfer02
chamfer03 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg01)])
chamfer04 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])
chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])
`
const pipedChamferDeclaration = 'chamfer(length = 5, tags = [seg01])'
const firstPipedChamferDeclaration = 'chamfer(length = 5, tags = [seg01])'
const secondPipedChamferDeclaration = 'chamfer(length = 5, tags = [seg02])'
const standaloneChamferDeclaration =
const standaloneAssignedChamferDeclaration =
'chamfer03 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg01)])'
const secondStandaloneChamferDeclaration =
'chamfer04 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])'
const standaloneUnassignedChamferDeclaration =
'chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])'

// Locators
const pipedChamferEdgeLocation = { x: 600, y: 193 }
Expand Down Expand Up @@ -3010,16 +3049,18 @@ chamfer04 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])
await test.step('Delete chamfer via feature tree selection', async () => {
await test.step('Open Feature Tree Pane', async () => {
await toolbar.openPane('feature-tree')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})

await test.step('Delete piped chamfer via feature tree selection', async () => {
await test.step('Verify all chamfers are present in the editor', async () => {
await editor.expectEditor.toContain(pipedChamferDeclaration)
await editor.expectEditor.toContain(firstPipedChamferDeclaration)
await editor.expectEditor.toContain(secondPipedChamferDeclaration)
await editor.expectEditor.toContain(standaloneChamferDeclaration)
await editor.expectEditor.toContain(
secondStandaloneChamferDeclaration
standaloneAssignedChamferDeclaration
)
await editor.expectEditor.toContain(
standaloneUnassignedChamferDeclaration
)
})
await test.step('Verify test chamfers are present in the scene', async () => {
Expand All @@ -3041,14 +3082,16 @@ chamfer04 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})
await test.step('Verify piped chamfer is deleted but other chamfers are not (in the editor)', async () => {
await editor.expectEditor.not.toContain(pipedChamferDeclaration)
await editor.expectEditor.not.toContain(firstPipedChamferDeclaration)
await editor.expectEditor.toContain(secondPipedChamferDeclaration)
await editor.expectEditor.toContain(standaloneChamferDeclaration)
await editor.expectEditor.toContain(
secondStandaloneChamferDeclaration
standaloneAssignedChamferDeclaration
)
await editor.expectEditor.toContain(
standaloneUnassignedChamferDeclaration
)
})
await test.step('Verify piped chamfer is deleted but non-piped is not (in the scene)', async () => {
Expand All @@ -3065,24 +3108,51 @@ chamfer04 = chamfer(extrude001, length = 5, tags = [getOppositeEdge(seg02)])
})
})

await test.step('Delete non-piped chamfer via feature tree selection', async () => {
await test.step('Delete non-piped chamfer', async () => {
await test.step('Delete standalone assigned chamfer via feature tree selection', async () => {
await test.step('Delete standalone assigned chamfer', async () => {
const operationButton = await toolbar.getFeatureTreeOperation(
'Chamfer',
1
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await page.waitForTimeout(500)
await scene.settled(cmdBar)
})
await test.step('Verify non-piped chamfer is deleted but other two chamfers are not (in the editor)', async () => {
await test.step('Verify standalone assigned chamfer is deleted but other two chamfers are not (in the editor)', async () => {
await editor.expectEditor.toContain(secondPipedChamferDeclaration)
await editor.expectEditor.not.toContain(standaloneChamferDeclaration)
await editor.expectEditor.not.toContain(
standaloneAssignedChamferDeclaration
)
await editor.expectEditor.toContain(
secondStandaloneChamferDeclaration
standaloneUnassignedChamferDeclaration
)
})
await test.step('Verify standalone assigned chamfer is deleted but piped is not (in the scene)', async () => {
await scene.expectPixelColor(
edgeColorWhite,
standaloneChamferEdgeLocation,
lowTolerance
)
})
})

await test.step('Delete standalone unassigned chamfer via feature tree selection', async () => {
await test.step('Delete standalone unassigned chamfer', async () => {
const operationButton = await toolbar.getFeatureTreeOperation(
'Chamfer',
1
)
await operationButton.click({ button: 'left' })
await page.keyboard.press('Delete')
await scene.settled(cmdBar)
})
await test.step('Verify standalone unassigned chamfer is deleted but piped chamfer is not (in the editor)', async () => {
await editor.expectEditor.toContain(secondPipedChamferDeclaration)
await editor.expectEditor.not.toContain(
standaloneUnassignedChamferDeclaration
)
})
await test.step('Verify non-piped chamfer is deleted but piped is not (in the scene)', async () => {
await test.step('Verify standalone unassigned chamfer is deleted but piped is not (in the scene)', async () => {
await scene.expectPixelColor(
edgeColorWhite,
standaloneChamferEdgeLocation,
Expand Down
23 changes: 22 additions & 1 deletion src/lang/modifyAst.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import { KCL_DEFAULT_CONSTANT_PREFIXES } from '@src/lib/constants'
import type { DefaultPlaneStr } from '@src/lib/planes'

import { err, trap } from '@src/lib/trap'
import { isOverlap, roundOff } from '@src/lib/utils'
import { isArray, isOverlap, roundOff } from '@src/lib/utils'
import type { ExtrudeFacePlane } from '@src/machines/modelingMachine'
import { ARG_AT } from '@src/lang/constants'

Expand Down Expand Up @@ -949,6 +949,27 @@ export function deleteSegmentFromPipeExpression(
return _modifiedAst
}

/**
* Deletes a standalone top level statement from the AST
* Used for removing both unassigned statements and variable declarations
*
* @param ast The AST to modify
* @param pathToNode The path to the node to delete
*/
export function deleteTopLevelStatement(
ast: Node<Program>,
pathToNode: PathToNode
): Error | void {
const pathStep = pathToNode[1]
if (!isArray(pathStep) || typeof pathStep[0] !== 'number') {
return new Error(
'Invalid pathToNode structure: expected a number at path[1][0]'
)
}
const statementIndex: number = pathStep[0]
ast.body.splice(statementIndex, 1)
}

export function removeSingleConstraintInfo(
pathToCallExp: PathToNode,
argDetails: SimplifiedArgDetails,
Expand Down
32 changes: 29 additions & 3 deletions src/lang/modifyAst/addEdgeTreatment.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ extrude001 = extrude(sketch001, length = -15)`
expectedCode
)
}, 10_000)
it(`should delete a non-piped ${edgeTreatmentType} from a single segment`, async () => {
it(`should delete a standalone assigned ${edgeTreatmentType} from a single segment`, async () => {
const code = `sketch001 = startSketchOn(XY)
|> startProfile(at = [-10, 10])
|> line(end = [20, 0])
Expand All @@ -810,8 +810,34 @@ extrude001 = extrude(sketch001, length = -15)`
|> line(endAbsolute = [profileStartX(%), profileStartY(%)])
|> close()
extrude001 = extrude(sketch001, length = -15)
fillet001 = ${edgeTreatmentType}(extrude001, ${parameterName} = 3, tags = [seg01])`
const edgeTreatmentSnippet = `fillet001 = ${edgeTreatmentType}(extrude001, ${parameterName} = 3, tags = [seg01])`
${edgeTreatmentType}001 = ${edgeTreatmentType}(extrude001, ${parameterName} = 3, tags = [seg01])`
const edgeTreatmentSnippet = `${edgeTreatmentType}001 = ${edgeTreatmentType}(extrude001, ${parameterName} = 3, tags = [seg01])`
const expectedCode = `sketch001 = startSketchOn(XY)
|> startProfile(at = [-10, 10])
|> line(end = [20, 0])
|> line(end = [0, -20])
|> line(end = [-20, 0], tag = $seg01)
|> line(endAbsolute = [profileStartX(%), profileStartY(%)])
|> close()
extrude001 = extrude(sketch001, length = -15)`

await runDeleteEdgeTreatmentTest(
code,
edgeTreatmentSnippet,
expectedCode
)
}, 10_000)
it(`should delete a standalone ${edgeTreatmentType} without assignment from a single segment`, async () => {
const code = `sketch001 = startSketchOn(XY)
|> startProfile(at = [-10, 10])
|> line(end = [20, 0])
|> line(end = [0, -20])
|> line(end = [-20, 0], tag = $seg01)
|> line(endAbsolute = [profileStartX(%), profileStartY(%)])
|> close()
extrude001 = extrude(sketch001, length = -15)
${edgeTreatmentType}(extrude001, ${parameterName} = 5, tags = [seg01])`
const edgeTreatmentSnippet = `${edgeTreatmentType}(extrude001, ${parameterName} = 5, tags = [seg01])`
const expectedCode = `sketch001 = startSketchOn(XY)
|> startProfile(at = [-10, 10])
|> line(end = [20, 0])
Expand Down
Loading
Loading