diff --git a/e2e/playwright/sketch-tests.spec.ts b/e2e/playwright/sketch-tests.spec.ts index f2c68b712..0280df505 100644 --- a/e2e/playwright/sketch-tests.spec.ts +++ b/e2e/playwright/sketch-tests.spec.ts @@ -34,7 +34,7 @@ test.describe('Sketch tests', () => { screwRadius = 3 wireRadius = 2 wireOffset = 0.5 - + screwHole = startSketchOn('XY') ${startProfileAt1} |> arc({ @@ -42,7 +42,7 @@ test.describe('Sketch tests', () => { angleStart = 0, angleEnd = 360 }, %) - + part001 = startSketchOn('XY') ${startProfileAt2} |> xLine(width * .5, %) @@ -51,7 +51,7 @@ test.describe('Sketch tests', () => { |> close() |> hole(screwHole, %) |> extrude(length = thickness) - + part002 = startSketchOn('-XZ') ${startProfileAt3} |> xLine(width / 4, %) @@ -99,6 +99,7 @@ test.describe('Sketch tests', () => { test('Can delete most of a sketch and the line tool will still work', async ({ page, homePage, + scene, }) => { const u = await getUtils(page) await page.addInitScript(async () => { @@ -112,12 +113,13 @@ test.describe('Sketch tests', () => { }) await homePage.goToModelingScene() + await scene.waitForExecutionDone() await expect(async () => { await page.getByText('tangentialArcTo([24.95, -5.38], %)').click() await expect( page.getByRole('button', { name: 'Edit Sketch' }) - ).toBeEnabled({ timeout: 1000 }) + ).toBeEnabled({ timeout: 2000 }) await page.getByRole('button', { name: 'Edit Sketch' }).click() }).toPass({ timeout: 40_000, intervals: [1_000] }) @@ -1066,7 +1068,7 @@ test.describe('Sketch tests', () => { `lugHeadLength = 0.25 lugDiameter = 0.5 lugLength = 2 - + fn lug = (origin, length, diameter, plane) => { lugSketch = startSketchOn(plane) |> startProfileAt([origin[0] + lugDiameter / 2, origin[1]], %) @@ -1075,10 +1077,10 @@ test.describe('Sketch tests', () => { |> yLineTo(0, %) |> close() |> revolve({ axis = "Y" }, %) - + return lugSketch } - + lug([0, 0], 10, .5, "XY")` ) }) @@ -1130,14 +1132,14 @@ test.describe('Sketch tests', () => { `fn in2mm = (inches) => { return inches * 25.4 } - + const railTop = in2mm(.748) const railSide = in2mm(.024) const railBaseWidth = in2mm(.612) const railWideWidth = in2mm(.835) const railBaseLength = in2mm(.200) const railClampable = in2mm(.200) - + const rail = startSketchOn('XZ') |> startProfileAt([ -railTop / 2, diff --git a/src/lang/modifyAst.test.ts b/src/lang/modifyAst.test.ts index 565d2c840..6ea87dea2 100644 --- a/src/lang/modifyAst.test.ts +++ b/src/lang/modifyAst.test.ts @@ -5,6 +5,8 @@ import { Identifier, SourceRange, topLevelRange, + LiteralValue, + Literal, } from './wasm' import { createLiteral, @@ -37,10 +39,26 @@ beforeAll(async () => { }) describe('Testing createLiteral', () => { - it('should create a literal', () => { + it('should create a literal number without units', () => { const result = createLiteral(5) expect(result.type).toBe('Literal') expect((result as any).value.value).toBe(5) + expect((result as any).value.suffix).toBe('None') + expect((result as Literal).raw).toBe('5') + }) + it('should create a literal number with units', () => { + const lit: LiteralValue = { value: 5, suffix: 'Mm' } + const result = createLiteral(lit) + expect(result.type).toBe('Literal') + expect((result as any).value.value).toBe(5) + expect((result as any).value.suffix).toBe('Mm') + expect((result as Literal).raw).toBe('5mm') + }) + it('should create a literal boolean', () => { + const result = createLiteral(false) + expect(result.type).toBe('Literal') + expect((result as Literal).value).toBe(false) + expect((result as Literal).raw).toBe('false') }) }) describe('Testing createIdentifier', () => { diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index 439fb0a33..ffb07bbb0 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -22,6 +22,7 @@ import { SourceRange, sketchFromKclValue, isPathToNodeNumber, + formatNumber, } from './wasm' import { isNodeSafeToReplacePath, @@ -780,11 +781,26 @@ export function splitPathAtPipeExpression(pathToNode: PathToNode): { return splitPathAtPipeExpression(pathToNode.slice(0, -1)) } +/** + * Note: This depends on WASM, but it's not async. Callers are responsible for + * awaiting init of the WASM module. + */ export function createLiteral(value: LiteralValue | number): Node { - const raw = `${value}` if (typeof value === 'number') { value = { value, suffix: 'None' } } + let raw: string + if (typeof value === 'string') { + // TODO: Should we handle escape sequences? + raw = `${value}` + } else if (typeof value === 'boolean') { + raw = `${value}` + } else if (typeof value.value === 'number' && value.suffix === 'None') { + // Fast path for numbers when there are no units. + raw = `${value.value}` + } else { + raw = formatNumber(value.value, value.suffix) + } return { type: 'Literal', start: 0, diff --git a/src/lang/util.ts b/src/lang/util.ts index 571b6af7e..dc992e6e9 100644 --- a/src/lang/util.ts +++ b/src/lang/util.ts @@ -8,6 +8,8 @@ import { ArtifactGraph, CallExpressionKw, Expr, + LiteralValue, + NumericSuffix, } from './wasm' import { filterArtifacts } from 'lang/std/artifactGraph' import { isOverlap } from 'lib/utils' @@ -111,3 +113,15 @@ export function findKwArgAnyIndex( export function isAbsolute(call: CallExpressionKw): boolean { return findKwArgAny(['endAbsolute'], call) !== undefined } + +export function isLiteralValueNumber( + e: LiteralValue +): e is { value: number; suffix: NumericSuffix } { + return ( + typeof e === 'object' && + 'value' in e && + typeof e.value === 'number' && + 'suffix' in e && + typeof e.suffix === 'string' + ) +} diff --git a/src/lang/wasm.test.ts b/src/lang/wasm.test.ts index 5cb9c5f23..6dc9eeada 100644 --- a/src/lang/wasm.test.ts +++ b/src/lang/wasm.test.ts @@ -1,5 +1,5 @@ import { err } from 'lib/trap' -import { initPromise, parse, ParseResult } from './wasm' +import { formatNumber, initPromise, parse, ParseResult } from './wasm' import { enginelessExecutor } from 'lib/testHelpers' import { Node } from 'wasm-lib/kcl/bindings/Node' import { Program } from '../wasm-lib/kcl/bindings/Program' @@ -20,3 +20,12 @@ it('can execute parsed AST', async () => { expect(err(execState)).toEqual(false) expect(execState.memory.get('x')?.value).toEqual(1) }) + +it('formats numbers with units', () => { + expect(formatNumber(1, 'None')).toEqual('1') + expect(formatNumber(1, 'Count')).toEqual('1_') + expect(formatNumber(1, 'Mm')).toEqual('1mm') + expect(formatNumber(1, 'Inch')).toEqual('1in') + expect(formatNumber(0.5, 'Mm')).toEqual('0.5mm') + expect(formatNumber(-0.5, 'Mm')).toEqual('-0.5mm') +}) diff --git a/src/lang/wasm.ts b/src/lang/wasm.ts index 219fa485e..accb7249b 100644 --- a/src/lang/wasm.ts +++ b/src/lang/wasm.ts @@ -2,6 +2,7 @@ import { init, parse_wasm, recast_wasm, + format_number, execute, kcl_lint, modify_ast_for_sketch_wasm, @@ -54,6 +55,7 @@ import { ArtifactCommand } from 'wasm-lib/kcl/bindings/Artifact' import { ArtifactGraph as RustArtifactGraph } from 'wasm-lib/kcl/bindings/Artifact' import { Artifact } from './std/artifactGraph' import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils' +import { NumericSuffix } from 'wasm-lib/kcl/bindings/NumericSuffix' export type { Artifact } from 'wasm-lib/kcl/bindings/Artifact' export type { ArtifactCommand } from 'wasm-lib/kcl/bindings/Artifact' @@ -92,6 +94,7 @@ export type { Literal } from '../wasm-lib/kcl/bindings/Literal' export type { LiteralValue } from '../wasm-lib/kcl/bindings/LiteralValue' export type { ArrayExpression } from '../wasm-lib/kcl/bindings/ArrayExpression' export type { SourceRange } from 'wasm-lib/kcl/bindings/SourceRange' +export type { NumericSuffix } from 'wasm-lib/kcl/bindings/NumericSuffix' export type SyntaxType = | 'Program' @@ -642,6 +645,13 @@ export const recast = (ast: Program): string | Error => { return recast_wasm(JSON.stringify(ast)) } +/** + * Format a number with suffix as KCL. + */ +export function formatNumber(value: number, suffix: NumericSuffix): string { + return format_number(value, JSON.stringify(suffix)) +} + export const makeDefaultPlanes = async ( engineCommandManager: EngineCommandManager ): Promise => { diff --git a/src/lib/rectangleTool.test.ts b/src/lib/rectangleTool.test.ts new file mode 100644 index 000000000..e7cb3dcc9 --- /dev/null +++ b/src/lib/rectangleTool.test.ts @@ -0,0 +1,92 @@ +import { expect } from 'vitest' +import { + recast, + assertParse, + topLevelRange, + VariableDeclaration, + initPromise, +} from 'lang/wasm' +import { updateCenterRectangleSketch } from './rectangleTool' +import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils' +import { getNodeFromPath } from 'lang/queryAst' +import { findUniqueName } from 'lang/modifyAst' +import { err, trap } from './trap' + +beforeAll(async () => { + await initPromise +}) + +describe('library rectangleTool helper functions', () => { + describe('updateCenterRectangleSketch', () => { + // regression test for https://github.com/KittyCAD/modeling-app/issues/5157 + test('should update AST and source code', async () => { + // Base source code that will be edited in place + const sourceCode = `sketch001 = startSketchOn('XZ') +|> startProfileAt([120.37, 162.76], %) +|> angledLine([0, 0], %, $rectangleSegmentA001) +|> angledLine([segAng(rectangleSegmentA001) + 90, 0], %, $rectangleSegmentB001) +|> angledLine([ +segAng(rectangleSegmentA001), +-segLen(rectangleSegmentA001) +], %, $rectangleSegmentC001) +|> lineTo([profileStartX(%), profileStartY(%)], %) +|> close(%) +` + // Create ast + const _ast = assertParse(sourceCode) + let ast = structuredClone(_ast) + + // Find some nodes and paths to reference + const sketchSnippet = `startProfileAt([120.37, 162.76], %)` + const sketchRange = topLevelRange( + sourceCode.indexOf(sketchSnippet), + sourceCode.indexOf(sketchSnippet) + sketchSnippet.length + ) + const sketchPathToNode = getNodePathFromSourceRange(ast, sketchRange) + const _node = getNodeFromPath( + ast, + sketchPathToNode || [], + 'VariableDeclaration' + ) + if (trap(_node)) return + const sketchInit = _node.node?.declaration.init + + // Hard code inputs that a user would have taken with their mouse + const x = 40 + const y = 60 + const rectangleOrigin = [120, 180] + const tags: [string, string, string] = [ + 'rectangleSegmentA001', + 'rectangleSegmentB001', + 'rectangleSegmentC001', + ] + + // Update the ast + if (sketchInit.type === 'PipeExpression') { + updateCenterRectangleSketch( + sketchInit, + x, + y, + tags[0], + rectangleOrigin[0], + rectangleOrigin[1] + ) + } + + // ast is edited in place from the updateCenterRectangleSketch + const expectedSourceCode = `sketch001 = startSketchOn('XZ') + |> startProfileAt([80, 120], %) + |> angledLine([0, 80], %, $rectangleSegmentA001) + |> angledLine([segAng(rectangleSegmentA001) + 90, 120], %, $rectangleSegmentB001) + |> angledLine([ + segAng(rectangleSegmentA001), + -segLen(rectangleSegmentA001) + ], %, $rectangleSegmentC001) + |> lineTo([profileStartX(%), profileStartY(%)], %) + |> close(%) +` + const recasted = recast(ast) + expect(recasted).toEqual(expectedSourceCode) + }) + }) +}) diff --git a/src/lib/rectangleTool.ts b/src/lib/rectangleTool.ts index c61bec0ed..0809625e8 100644 --- a/src/lib/rectangleTool.ts +++ b/src/lib/rectangleTool.ts @@ -10,13 +10,19 @@ import { createTagDeclarator, createUnaryExpression, } from 'lang/modifyAst' -import { ArrayExpression, CallExpression, PipeExpression } from 'lang/wasm' +import { + ArrayExpression, + CallExpression, + PipeExpression, + recast, +} from 'lang/wasm' import { roundOff } from 'lib/utils' import { isCallExpression, isArrayExpression, isLiteral, isBinaryExpression, + isLiteralValueNumber, } from 'lang/util' /** @@ -144,10 +150,12 @@ export function updateCenterRectangleSketch( if (isArrayExpression(arrayExpression)) { const literal = arrayExpression.elements[0] if (isLiteral(literal)) { - callExpression.arguments[0] = createArrayExpression([ - createLiteral(literal.value), - createLiteral(Math.abs(twoX)), - ]) + if (isLiteralValueNumber(literal.value)) { + callExpression.arguments[0] = createArrayExpression([ + createLiteral(literal.value), + createLiteral(Math.abs(twoX)), + ]) + } } } } diff --git a/src/lib/wasm_lib_wrapper.ts b/src/lib/wasm_lib_wrapper.ts index 1fe1b8bf7..e55a78d58 100644 --- a/src/lib/wasm_lib_wrapper.ts +++ b/src/lib/wasm_lib_wrapper.ts @@ -10,6 +10,7 @@ import { parse_wasm as ParseWasm, recast_wasm as RecastWasm, + format_number as FormatNumber, execute as Execute, kcl_lint as KclLint, modify_ast_for_sketch_wasm as ModifyAstForSketch, @@ -51,6 +52,9 @@ export const parse_wasm: typeof ParseWasm = (...args) => { export const recast_wasm: typeof RecastWasm = (...args) => { return getModule().recast_wasm(...args) } +export const format_number: typeof FormatNumber = (...args) => { + return getModule().format_number(...args) +} export const execute: typeof Execute = (...args) => { return getModule().execute(...args) } diff --git a/src/wasm-lib/kcl/src/lib.rs b/src/wasm-lib/kcl/src/lib.rs index 00ce5e7f4..83bf4fd21 100644 --- a/src/wasm-lib/kcl/src/lib.rs +++ b/src/wasm-lib/kcl/src/lib.rs @@ -120,6 +120,11 @@ pub mod std_utils { pub use crate::std::utils::{get_tangential_arc_to_info, is_points_ccw_wasm, TangentialArcInfoInput}; } +pub mod pretty { + pub use crate::parsing::token::NumericSuffix; + pub use crate::unparser::format_number; +} + use serde::{Deserialize, Serialize}; #[allow(unused_imports)] diff --git a/src/wasm-lib/kcl/src/unparser.rs b/src/wasm-lib/kcl/src/unparser.rs index 895696680..6bb76f3e7 100644 --- a/src/wasm-lib/kcl/src/unparser.rs +++ b/src/wasm-lib/kcl/src/unparser.rs @@ -8,6 +8,7 @@ use crate::parsing::{ LiteralValue, MemberExpression, MemberObject, Node, NonCodeNode, NonCodeValue, ObjectExpression, Parameter, PipeExpression, Program, TagDeclarator, UnaryExpression, VariableDeclaration, VariableKind, }, + token::NumericSuffix, PIPE_OPERATOR, }; @@ -370,6 +371,11 @@ impl VariableDeclaration { } } +// Used by TS. +pub fn format_number(value: f64, suffix: NumericSuffix) -> String { + format!("{value}{suffix}") +} + impl Literal { fn recast(&self) -> String { match self.value { diff --git a/src/wasm-lib/src/wasm.rs b/src/wasm-lib/src/wasm.rs index bd4a0cf76..7f4ff7f4b 100644 --- a/src/wasm-lib/src/wasm.rs +++ b/src/wasm-lib/src/wasm.rs @@ -5,7 +5,8 @@ use std::sync::Arc; use futures::stream::TryStreamExt; use gloo_utils::format::JsValueSerdeExt; use kcl_lib::{ - exec::IdGenerator, CacheInformation, CoreDump, EngineManager, ExecState, ModuleId, OldAstState, Point2d, Program, + exec::IdGenerator, pretty::NumericSuffix, CacheInformation, CoreDump, EngineManager, ExecState, ModuleId, + OldAstState, Point2d, Program, }; use tokio::sync::RwLock; use tower_lsp::{LspService, Server}; @@ -251,6 +252,14 @@ pub fn recast_wasm(json_str: &str) -> Result { Ok(JsValue::from_serde(&program.recast())?) } +#[wasm_bindgen] +pub fn format_number(value: f64, suffix_json: &str) -> Result { + console_error_panic_hook::set_once(); + + let suffix: NumericSuffix = serde_json::from_str(suffix_json).map_err(JsError::from)?; + Ok(kcl_lib::pretty::format_number(value, suffix)) +} + #[wasm_bindgen] pub struct ServerConfig { into_server: js_sys::AsyncIterator,