Skip to content

Commit

Permalink
Fix test
Browse files Browse the repository at this point in the history
Combination of two issues:
 - Test was wrong
 - Library code for removing constraints from sketch lines was wrong,
   was not properly recognizing when kw calls were absolute or relative
   because they only considered the function name, not its parameters
   (and parameters like 'length' or 'endAbsolute' determine if a call
   is actually relative/absolute, not just the function name)
  • Loading branch information
adamchalmers committed Feb 27, 2025
1 parent e54ba22 commit 3e08a4f
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 21 deletions.
14 changes: 8 additions & 6 deletions src/lang/modifyAst.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ describe('Testing removeSingleConstraintInfo', () => {
'objectProperty',
'angle',
],
['line(endAbsolute = [6.14 + 0, 3.14 + 0])', 'arrayIndex', 0],
['line(endAbsolute = [6.14, 3.14 + 0])', 'arrayIndex', 0],
['xLine(endAbsolute = 8)', '', ''],
['yLine(endAbsolute = 5)', '', ''],
['yLine(length = 3.14, tag = $a)', '', ''],
Expand Down Expand Up @@ -718,11 +718,13 @@ describe('Testing removeSingleConstraintInfo', () => {
const ast = assertParse(code)

const execState = await enginelessExecutor(ast)
const lineOfInterest = expectedFinish.split('(')[0] + '('
const range = topLevelRange(
code.indexOf(lineOfInterest) + 1,
code.indexOf(lineOfInterest) + lineOfInterest.length
)
const lineOfInterest =
expectedFinish.indexOf('=') >= 0 && expectedFinish.indexOf('{') === -1
? expectedFinish.split('=')[0]
: expectedFinish.split('(')[0] + '('
const start = code.indexOf(lineOfInterest)
expect(start).toBeGreaterThanOrEqual(0)
const range = topLevelRange(start + 1, start + lineOfInterest.length)
const pathToNode = getNodePathFromSourceRange(ast, range)
let argPosition: SimplifiedArgDetails
if (key === 'arrayIndex' && typeof value === 'number') {
Expand Down
4 changes: 1 addition & 3 deletions src/lang/std/sketch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2387,9 +2387,7 @@ export function sketchFnNameToTooltip(
}
}

export function sketchFnIsAbsolute(
callExpression: Node<CallExpressionKw>
): boolean {
export function sketchFnIsAbsolute(callExpression: CallExpressionKw): boolean {
const fnName = callExpression.callee.name
return (
fnName === 'circleThreePoint' ||
Expand Down
55 changes: 43 additions & 12 deletions src/lang/std/sketchcombos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ import {
getConstraintInfoKw,
isAbsoluteLine,
ARG_LENGTH,
sketchFnNameToTooltip,
sketchFnIsAbsolute,
} from './sketch'
import {
getSketchSegmentFromPathToNode,
Expand Down Expand Up @@ -173,9 +175,8 @@ function createCallWrapper(
return [true, 'yLine']
}
})()
const labeledArgs = [
createLabeledArg(isAbsolute ? ARG_END_ABSOLUTE : ARG_LENGTH, val),
]
const argLabel = isAbsolute ? ARG_END_ABSOLUTE : ARG_LENGTH
const labeledArgs = [createLabeledArg(argLabel, val)]
if (tag) {
labeledArgs.push(createLabeledArg(ARG_TAG, tag))
}
Expand Down Expand Up @@ -1412,6 +1413,14 @@ export function removeSingleConstraint({
console.error(callExp)
return false
}
const correctFnName = (() => {
switch (callExp.node.type) {
case 'CallExpressionKw': {
const isAbsolute = sketchFnIsAbsolute(callExp.node)
return sketchFnNameToTooltip(callExp.node.callee.name, isAbsolute)
}
}
})()
if (
callExp.node.type !== 'CallExpression' &&
callExp.node.type !== 'CallExpressionKw'
Expand Down Expand Up @@ -1454,21 +1463,29 @@ export function removeSingleConstraint({
)
} else {
// It's a kw call.
const isAbsolute = callExp.node.callee.name == 'lineTo'
const res: { isAbsolute: boolean; fnName: ToolTip } | undefined =
(() => {
switch (correctFnName) {
case 'lineTo':
return { isAbsolute: true, fnName: 'line' }
case 'line':
return { isAbsolute: false, fnName: 'line' }
}
})()
if (res === undefined) {
return new Error('Unrecognized kw call function ' + correctFnName)
}
const { isAbsolute, fnName } = res
if (isAbsolute) {
const args = [
createLabeledArg(ARG_END_ABSOLUTE, createArrayExpression(values)),
]
return createStdlibCallExpressionKw('line', args, tag)
return createStdlibCallExpressionKw(fnName, args, tag)
} else {
const args = [
createLabeledArg(ARG_END, createArrayExpression(values)),
]
return createStdlibCallExpressionKw(
callExp.node.callee.name as ToolTip,
args,
tag
)
return createStdlibCallExpressionKw(fnName, args, tag)
}
}
}
Expand Down Expand Up @@ -1586,7 +1603,7 @@ export function removeSingleConstraint({
}

return createCallWrapper(
callExp.node.callee.name as any,
correctFnName || (callExp.node.callee.name as any),
rawArgs[0].expr,
tag
)
Expand Down Expand Up @@ -2084,12 +2101,26 @@ export function transformAstSketchLines({
}
const { to, from } = seg
// Note to ADAM: Here is where the replaceExisting call gets sent.
const correctFnName = (() => {
switch (call.node.type) {
case 'CallExpressionKw': {
const fnName = call.node.callee.name as ToolTip
const correctFnName = sketchFnNameToTooltip(
fnName,
sketchFnIsAbsolute(call.node)
)
return correctFnName
}
}
})()
const fnName =
correctFnName || transformTo || (call.node.callee.name as ToolTip)
const replacedSketchLine = replaceSketchLine({
node: node,
variables: memVars,
pathToNode: _pathToNode,
referencedSegment,
fnName: transformTo || (call.node.callee.name as ToolTip),
fnName,
segmentInput:
seg.type === 'Circle'
? {
Expand Down

0 comments on commit 3e08a4f

Please sign in to comment.