KCL: Change core sketch functions to use keyword args #4826
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
2 times, most recently
from
1f6427d to
a61d547
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
7 times, most recently
from
68b00f2 to
5f6057f
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
from
92360e7 to
f240bbb
Compare
|
I skimmed this and it looks great so far. Does it maintain backward compatibility? If not, the UI needs to generate the new code. |
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
from
d66bbce to
356b257
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
from
101bd66 to
b686ca2
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
2 times, most recently
from
41e05c9 to
e99f814
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
from
4e8305d to
f678077
Compare
adamchalmers
force-pushed
the
achalmers/kw-fn-sketches
branch
from
cbf0abb to
95b6476
Compare
|
|
|
|
|
|
|
|
|
just wanna make sure whatever regex you've been using to easily migrate files should be included in the changelog (cc @pierremtb ) and we should have instructions for discord users as well |
|
@jessfraz shouldn't the regex be executed by a command in the app (as in some sort of auto migration)? I feel like having the user open up a terminal to batch replaces things is a little too much no? |
|
his thing doesnt work perfectly its more like a helper im just trying to help folks without increasing the burden on the pr to do it |
| @@ -110,6 +122,39 @@ function createCallWrapper( | |||
| tag?: Expr, | |||
| valueUsedInTransform?: number | |||
| ): CreatedSketchExprResult { | |||
| if (Array.isArray(val)) { | |||
There was a problem hiding this comment.
We should use isArray() in src/lib/utils.ts.
There was a problem hiding this comment.
Array.isArray is used 8 other places in this file, I would like to be consistent with the rest of the file. Follow-up PR could stop using that function.
|
|
|
|
Changes to KCL stdlib:
line(point, sketch, tag)andlineTo(point, sketch, tag)are combined intoline(@sketch, end?, endAbsolute?, tag?)close(sketch, tag?)is nowclose(@sketch, tag?)extrude(length, sketch)is nowextrude(@sketch, length)Part of #4600
Also changes frontend tests to use KittyCAD/kcl-samples#139 instead of its main
The regex find-and-replace I use for migrating code (note these don't work with multi-line expressions) are:
line(([^=]*), %)
line(end = $1)
line((.), %, (.))
line(end = $1, tag = $2)
lineTo((.*), %)
line(endAbsolute = $1)
lineTo((.), %, (.))
line(endAbsolute = $1, tag = $2)
extrude((.*), %)
extrude(length = $1)
extrude(([^=]*), ([a-zA-Z0-9]+))
extrude($2, length = $1)
close(%, (.*))
close(tag = $1)