From f30fc376ee72ca263f6c0bcc0549fd5ea4222703 Mon Sep 17 00:00:00 2001 From: Frank Noirot Date: Sat, 19 Apr 2025 05:27:23 -0400 Subject: [PATCH] Allow deletion of only item in AST (#6335) * Update test to check for deletion of only operation * Allow updateEditorWithAstAndWriteToFile to clear AST * Update src/lang/codeManager.ts Co-authored-by: Jonathan Tran * Weave `isDeleting` through to `deleteSelectionPromise` * fmt --------- Co-authored-by: Jonathan Tran Co-authored-by: Pierre Jacquier --- e2e/playwright/feature-tree-pane.spec.ts | 12 ++++++++++++ src/lang/codeManager.ts | 11 +++++++---- src/lang/modelingWorkflows.ts | 6 +++++- src/lang/modifyAst/deleteSelection.ts | 17 ++++++++++++----- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/e2e/playwright/feature-tree-pane.spec.ts b/e2e/playwright/feature-tree-pane.spec.ts index 530eadd2c..fb1ee6f41 100644 --- a/e2e/playwright/feature-tree-pane.spec.ts +++ b/e2e/playwright/feature-tree-pane.spec.ts @@ -430,5 +430,17 @@ profile003 = startProfileAt([0, -4.93], sketch001) await editor.expectEditor.not.toContain('sketch001 =') await editor.expectEditor.not.toContain('profile002 = ') }) + + await test.step(`Delete the remaining plane via feature tree`, async () => { + const operationButton = await toolbar.getFeatureTreeOperation( + 'Offset Plane', + 0 + ) + await operationButton.click({ button: 'left' }) + await page.keyboard.press('Delete') + + // Verify the plane code is gone, and https://github.com/KittyCAD/modeling-app/issues/5988 is fixed. + await editor.expectEditor.not.toContain('plane001 =') + }) }) }) diff --git a/src/lang/codeManager.ts b/src/lang/codeManager.ts index f8bf489df..955b2b9d7 100644 --- a/src/lang/codeManager.ts +++ b/src/lang/codeManager.ts @@ -165,13 +165,16 @@ export default class CodeManager { } } - async updateEditorWithAstAndWriteToFile(ast: Program) { + async updateEditorWithAstAndWriteToFile( + ast: Program, + options?: Partial<{ isDeleting: boolean }> + ) { // We clear the AST when it cannot be parsed. If we are trying to write an // empty AST, it's probably because of an earlier error. That's a bad state // to be in, and it's not going to be pretty, but at the least, let's not - // permanently delete the user's code. If you want to clear the scene, call - // updateCodeStateEditor directly. - if (ast.body.length === 0) return + // permanently delete the user's code accidentally. + // if you want to clear the scene, pass in the `isDeleting` option. + if (ast.body.length === 0 && !options?.isDeleting) return const newCode = recast(ast) if (err(newCode)) return // Test to see if we can parse the recast code, and never update the editor with bad code. diff --git a/src/lang/modelingWorkflows.ts b/src/lang/modelingWorkflows.ts index fb6022e42..3209ee177 100644 --- a/src/lang/modelingWorkflows.ts +++ b/src/lang/modelingWorkflows.ts @@ -54,6 +54,7 @@ export async function updateModelingState( }, options?: { focusPath?: Array + isDeleting?: boolean } ): Promise { let updatedAst: { @@ -69,7 +70,10 @@ export async function updateModelingState( // Step 2: Update the code editor and save file await dependencies.codeManager.updateEditorWithAstAndWriteToFile( - updatedAst.newAst + updatedAst.newAst, + { + isDeleting: options?.isDeleting, + } ) // Step 3: Set focus on the newly added code if needed diff --git a/src/lang/modifyAst/deleteSelection.ts b/src/lang/modifyAst/deleteSelection.ts index 4dc654ab1..8045d2073 100644 --- a/src/lang/modifyAst/deleteSelection.ts +++ b/src/lang/modifyAst/deleteSelection.ts @@ -38,9 +38,16 @@ export async function deleteSelectionPromise( if (testExecute.errors.length) { return new Error(deletionErrorMessage) } - await updateModelingState(modifiedAst, EXECUTION_TYPE_REAL, { - kclManager, - editorManager, - codeManager, - }) + await updateModelingState( + modifiedAst, + EXECUTION_TYPE_REAL, + { + kclManager, + editorManager, + codeManager, + }, + { + isDeleting: true, + } + ) }