Skip to content

Commit

Permalink
KCL: Make shell stdlib fn use keyword arguments (#5293)
Browse files Browse the repository at this point in the history
Before:

```
shell({ faces = ['end'], thickness = caseThickness }, case)
```

After:
```
shell(case, faces = ['end'], thickness = caseThickness)
```
  • Loading branch information
adamchalmers authored Feb 7, 2025
1 parent 357bbff commit 61807e7
Show file tree
Hide file tree
Showing 23 changed files with 2,846 additions and 868 deletions.
4 changes: 2 additions & 2 deletions docs/kcl/appearance.md

Large diffs are not rendered by default.

24 changes: 11 additions & 13 deletions docs/kcl/shell.md

Large diffs are not rendered by default.

3,239 changes: 2,624 additions & 615 deletions docs/kcl/std.json

Large diffs are not rendered by default.

12 changes: 5 additions & 7 deletions e2e/playwright/point-click.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,7 @@ chamfer04 = chamfer({ length = 5, tags = [getOppositeEdge(seg02)]}, extrude001
const testPoint = { x: 575, y: 200 }
const [clickOnCap] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
const shellDeclaration =
"shell001 = shell({ faces = ['end'], thickness = 5 }, extrude001)"
"shell001 = shell(extrude001, faces = ['end'], thickness = 5)"

await test.step(`Look for the grey of the shape`, async () => {
await scene.expectPixelColor([127, 127, 127], testPoint, 15)
Expand Down Expand Up @@ -1990,8 +1990,7 @@ extrude001 = extrude(sketch001, length = 40)
const [clickOnWall] = scene.makeMouseHelpers(testPoint.x, testPoint.y + 70)
const mutatedCode = 'xLine(-40, %, $seg01)'
const shellDeclaration =
"shell001 = shell({ faces = ['end', seg01], thickness = 5}, extrude001)"
const formattedOutLastLine = '}, extrude001)'
"shell001 = shell(extrude001, faces = ['end', seg01], thickness = 5)"

await test.step(`Look for the grey of the shape`, async () => {
await scene.expectPixelColor([99, 99, 99], testPoint, 15)
Expand Down Expand Up @@ -2034,7 +2033,7 @@ extrude001 = extrude(sketch001, length = 40)
await editor.expectEditor.toContain(shellDeclaration)
await editor.expectState({
diagnostics: [],
activeLines: [formattedOutLastLine],
activeLines: [shellDeclaration],
highlightedCode: '',
})
await scene.expectPixelColor([49, 49, 49], testPoint, 15)
Expand Down Expand Up @@ -2088,9 +2087,8 @@ extrude002 = extrude(sketch002, length = 50)
// One dumb hardcoded screen pixel value
const testPoint = { x: 550, y: 295 }
const [clickOnCap] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
const shellDeclaration = `shell001 = shell({ faces = ['end'], thickness = 5 }, ${
hasExtrudesInPipe ? 'sketch002' : 'extrude002'
})`
const shellTarget = hasExtrudesInPipe ? 'sketch002' : 'extrude002'
const shellDeclaration = `shell001 = shell(${shellTarget}, faces = ['end'], thickness = 5)`

await test.step(`Look for the grey of the shape`, async () => {
await toolbar.closePane('code')
Expand Down
2 changes: 1 addition & 1 deletion e2e/playwright/regression-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ extrude001 = extrude(sketch001, length = 50)
|>
example = extrude(exampleSketch, length = 5)
shell({ faces: ['end'], thickness: 0.25 }, exampleSketch)`
shell(exampleSketch, faces = ['end'], thickness = 0.25)`
)
})

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"fmt": "prettier --write ./src *.ts *.json *.js ./e2e ./packages",
"fmt-check": "prettier --check ./src *.ts *.json *.js ./e2e ./packages",
"fetch:wasm": "./get-latest-wasm-bundle.sh",
"fetch:samples": "echo \"Fetching latest KCL samples...\" && curl -o public/kcl-samples-manifest-fallback.json https://raw.githubusercontent.com/KittyCAD/kcl-samples/main/manifest.json",
"fetch:samples": "echo \"Fetching latest KCL samples...\" && curl -o public/kcl-samples-manifest-fallback.json https://raw.githubusercontent.com/KittyCAD/kcl-samples/achalmers/kw-shell/manifest.json",
"isomorphic-copy-wasm": "(copy src/wasm-lib/pkg/wasm_lib_bg.wasm public || cp src/wasm-lib/pkg/wasm_lib_bg.wasm public)",
"build:wasm-dev": "yarn wasm-prep && (cd src/wasm-lib && wasm-pack build --dev --target web --out-dir pkg && cargo test -p kcl-lib export_bindings) && yarn isomorphic-copy-wasm && yarn fmt",
"build:wasm": "yarn wasm-prep && cd src/wasm-lib && wasm-pack build --release --target web --out-dir pkg && cargo test -p kcl-lib export_bindings && cd ../.. && yarn isomorphic-copy-wasm && yarn fmt",
Expand Down
9 changes: 8 additions & 1 deletion src/lang/kclSamples.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ try {
console.log(e)
}

child_process.spawnSync('git', ['clone', URL_GIT_KCL_SAMPLES, DIR_KCL_SAMPLES])
child_process.spawnSync('git', [
'clone',
'--single-branch',
'--branch',
'achalmers/kw-shell',
URL_GIT_KCL_SAMPLES,
DIR_KCL_SAMPLES,
])

// @ts-expect-error
let files = await fs.readdir(DIR_KCL_SAMPLES)
Expand Down
18 changes: 10 additions & 8 deletions src/lang/modifyAst/addShell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
createObjectExpression,
createArrayExpression,
createVariableDeclaration,
createCallExpressionStdLibKw,
createLabeledArg,
} from 'lang/modifyAst'
import { KCL_DEFAULT_CONSTANT_PREFIXES } from 'lib/constants'
import { KclManager } from 'lang/KclSingleton'
Expand Down Expand Up @@ -121,13 +123,14 @@ export function addShell({
}

const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SHELL)
const shell = createCallExpressionStdLib('shell', [
createObjectExpression({
faces: createArrayExpression(expressions),
thickness,
}),
const shell = createCallExpressionStdLibKw(
'shell',
createIdentifier(extrudeNode.node.id.name),
])
[
createLabeledArg('faces', createArrayExpression(expressions)),
createLabeledArg('thickness', thickness),
]
)
const declaration = createVariableDeclaration(name, shell)

// TODO: check if we should append at the end like here or right after the extrude
Expand All @@ -137,8 +140,7 @@ export function addShell({
[modifiedAst.body.length - 1, 'index'],
['declaration', 'VariableDeclaration'],
['init', 'VariableDeclarator'],
['arguments', 'CallExpression'],
[0, 'index'],
['unlabeled', 'CallExpressionKw'],
]
return {
modifiedAst,
Expand Down
20 changes: 10 additions & 10 deletions src/wasm-lib/kcl/src/execution/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, ctx, _) = parse_execute(new).await.unwrap();

Expand Down Expand Up @@ -273,7 +273,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#;
shell(firstSketch, faces = ['end'], thickness = 0.25) "#;

let new = r#"// Remove the end face for the extrusion.
firstSketch = startSketchOn('XY')
Expand All @@ -285,7 +285,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program_old, ctx, _) = parse_execute(old).await.unwrap();

Expand Down Expand Up @@ -318,7 +318,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#;
shell(firstSketch, faces = ['end'], thickness = 0.25) "#;

let new = r#"// Remove the end face for the extrusion.
firstSketch = startSketchOn('XY')
Expand All @@ -330,7 +330,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, ctx, _) = parse_execute(old).await.unwrap();

Expand Down Expand Up @@ -363,7 +363,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch) "#;
shell(firstSketch, faces = ['end'], thickness = 0.25) "#;

let new = r#"// Remove the end face for the extrusion.
firstSketch = startSketchOn('XY')
Expand All @@ -375,7 +375,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, ctx, _) = parse_execute(old).await.unwrap();

Expand Down Expand Up @@ -409,7 +409,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, mut ctx, _) = parse_execute(new).await.unwrap();

Expand Down Expand Up @@ -451,7 +451,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, mut ctx, _) = parse_execute(new).await.unwrap();

Expand Down Expand Up @@ -486,7 +486,7 @@ firstSketch = startSketchOn('XY')
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell({ faces = ['end'], thickness = 0.25 }, firstSketch)"#;
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;

let (program, mut ctx, _) = parse_execute(new).await.unwrap();

Expand Down
16 changes: 8 additions & 8 deletions src/wasm-lib/kcl/src/execution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1590,10 +1590,10 @@ let w = f() + f()
|> line(endAbsolute = [profileStartX(%), profileStartY(%)])
|> close()
|> extrude(length = 40.14)
|> shell({
faces: [seg01],
thickness: 3.14,
}, %)
|> shell(
thickness = 3.14,
faces = [seg01]
)
"#;

let ctx = crate::test_server::new_context(UnitLength::Mm, true, None)
Expand All @@ -1620,10 +1620,10 @@ let w = f() + f()
|> line(endAbsolute = [profileStartX(%), profileStartY(%)])
|> close()
|> extrude(length = 40.14)
|> shell({
faces: [seg01],
thickness: 3.14,
}, %)
|> shell(
faces = [seg01],
thickness = 3.14,
)
"#;

// Execute a slightly different program again.
Expand Down
10 changes: 6 additions & 4 deletions src/wasm-lib/kcl/src/std/appearance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,11 @@ pub async fn appearance(_exec_state: &mut ExecState, args: Args) -> Result<KclVa
/// |> close()
/// |> extrude(length = 6)
///
/// shell({
/// shell(
/// firstSketch,
/// faces = ['end'],
/// thickness = 0.25,
/// }, firstSketch)
/// )
/// |> appearance({
/// color = '#ff0000',
/// metalness = 90,
Expand All @@ -147,10 +148,11 @@ pub async fn appearance(_exec_state: &mut ExecState, args: Args) -> Result<KclVa
/// roughness = 90
/// }, %)
///
/// shell({
/// shell(
/// firstSketch,
/// faces = ['end'],
/// thickness = 0.25,
/// }, firstSketch)
/// )
/// ```
///
/// ```no_run
Expand Down
9 changes: 0 additions & 9 deletions src/wasm-lib/kcl/src/std/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1019,15 +1019,6 @@ impl<'a> FromKclValue<'a> for super::sketch::BezierData {
}
}

impl<'a> FromKclValue<'a> for super::shell::ShellData {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
let obj = arg.as_object()?;
let_field_of!(obj, thickness);
let_field_of!(obj, faces);
Some(Self { thickness, faces })
}
}

impl<'a> FromKclValue<'a> for super::chamfer::ChamferData {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
let obj = arg.as_object()?;
Expand Down
Loading

0 comments on commit 61807e7

Please sign in to comment.