Add bodyType to Revolve P&C #9572
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| it('should add basic revolve call with surface bodyType', async () => { | ||
| const { ast, sketches } = await getAstAndSketchSelections( | ||
| circleCode, | ||
| instanceInThisFile, | ||
| kclManagerInThisFile | ||
| ) | ||
| expect(sketches.graphSelections).toHaveLength(1) | ||
| const angle = await getKclCommandValue( | ||
| '10', | ||
| instanceInThisFile, | ||
| rustContextInThisFile | ||
| ) | ||
| const axis = 'X' | ||
| const result = addRevolve({ | ||
| ast, | ||
| sketches, | ||
| angle, | ||
| axis, | ||
| bodyType: 'SURFACE', | ||
| wasmInstance: instanceInThisFile, | ||
| }) | ||
| if (err(result)) throw result | ||
| await runNewAstAndCheckForSweep(result.modifiedAst, rustContextInThisFile) | ||
| const newCode = recast(result.modifiedAst, instanceInThisFile) | ||
| expect(newCode).toContain(circleCode) | ||
| expect(newCode).toContain( | ||
| `revolve001 = revolve( | ||
| profile001, | ||
| angle = 10, | ||
| axis = X, | ||
| bodyType = SURFACE, | ||
| )` | ||
| ) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Extra codemod integration test case
| }) | ||
| }) | ||
|
|
||
| describe('Testing retrieveBodyTypeFromOpArg', () => { |
There was a problem hiding this comment.
New util function used for Extrude and Revolve here in the edit flow, see operations.ts
| * Gather up the argument values for the Revolve command | ||
| * to be used in the command bar edit flow. | ||
| */ | ||
| const prepareToEditRevolve: PrepareToEditCallback = async ({ |
There was a problem hiding this comment.
We don't do it yet but I'd be keen on trying to spin up an integration test for each prepareToEdit* function here, we rely on e2e for it plus each individual util function called in there but we could benefit from a few arg permutations at this level. Note for later!
| /** Version of `toUtf16` bound to our code, for mapping source range values. */ | ||
| const boundToUtf16 = (n: number) => toUtf16(n, code) | ||
| const result = code.slice(...opArg.sourceRange.map(boundToUtf16)) | ||
| if (result === KCL_PRELUDE_BODY_TYPE_SOLID) { |
There was a problem hiding this comment.
we don't need to normalize the lover/upper case here, right?
There was a problem hiding this comment.
Good question, are both defined?
| } | ||
| const res = retrieveBodyTypeFromOpArg(operation.labeledArgs.bodyType, code) | ||
| if (err(res)) return { reason: res.message } | ||
| bodyType = res |
There was a problem hiding this comment.
so much cleaner, love it
|
@max-mrgrsk Made the change! Good catch ty |
max-mrgrsk
left a comment
max-mrgrsk
left a comment
There was a problem hiding this comment.
good stuff, I've revolved these
with 0 problems into these
great PR, ty!
|
When Sketch 2 lands, paths will go away and we’ll switch to separate segments. That change will ripple through the system - especially surface modeling - so we should plan for its impact. |
|
Agreed! Ty |
pierremtb
deleted the
pierremtb/issue9567-ZDS-support-for-Revolve-Surface
branch
Closes #9581
Adds the optional arg to the command palette config just like for Extrude at #9354. Cleans up a bit so we're ready to add this to more places.
A few notes at #9572 (review)