Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace tanArc(to) with tanArcTo(relative: true) #3334

Closed
wants to merge 0 commits into from

Conversation

adamchalmers
Copy link
Collaborator

Fixes #3319

Copy link

qa-wolf bot commented Aug 8, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented Aug 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ❌ Failed (Inspect) Aug 28, 2024 3:24pm

@adamchalmers adamchalmers requested review from jtran and paultag August 8, 2024 19:11
@adamchalmers adamchalmers force-pushed the achalmers/remove-tanarc-to branch from 0b64dbd to 9f478e8 Compare August 8, 2024 19:17
"summary": "Starting at the current sketch's origin, draw a curved line segment along",
"description": "some part of an imaginary circle of the specified radius.\nThe arc is constructed such that the last line segment is placed tangent to the imaginary circle of the specified radius. The resulting arc is the segment of the imaginary circle from that tangent point for 'offset' degrees along the imaginary circle.",
"summary": "Draw a curved line segment along some part of an imaginary circle of the specified radius.",
"description": "If `relative` is true, the curve starts at the end of the previous path segment (i.e. the location of the \"pen\" which draws these lines). If `relative` is false, starts from the current sketch's origin.\nThe arc is constructed such that the last line segment is placed tangent to the imaginary circle of the specified radius. The resulting arc is the segment of the imaginary circle from that tangent point for 'offset' degrees along the imaginary circle.",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paultag wdyt of my docs here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent docs; this is great, super good change.

@jtran
Copy link
Collaborator

jtran commented Aug 8, 2024

I haven't tracked down the exact spot, but I feel like there are a lot of assumptions in TS code about the arguments and their order, like where to put the %.

updateTangentialArcToSegment({

@jtran
Copy link
Collaborator

jtran commented Aug 8, 2024

Here's a place that adds a new call.

const newLine = createCallExpression('tangentialArcTo', [
createArrayExpression([toX, toY]),
createPipeSubstitution(),
])

Copy link
Collaborator

@paultag paultag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's still some last bits to tackle here (some of the 20/20 changes look real flag on behavior change, the draft line for the arc) but the gist here is good - going to mark approved but pending ci/finalization on the rest -- this is great, and a totally needed clenaup. The Arc KCL functions are a bit confusing to use at the moment, thanks for all the work here, this is awesome.

"summary": "Starting at the current sketch's origin, draw a curved line segment along",
"description": "some part of an imaginary circle of the specified radius.\nThe arc is constructed such that the last line segment is placed tangent to the imaginary circle of the specified radius. The resulting arc is the segment of the imaginary circle from that tangent point for 'offset' degrees along the imaginary circle.",
"summary": "Draw a curved line segment along some part of an imaginary circle of the specified radius.",
"description": "If `relative` is true, the curve starts at the end of the previous path segment (i.e. the location of the \"pen\" which draws these lines). If `relative` is false, starts from the current sketch's origin.\nThe arc is constructed such that the last line segment is placed tangent to the imaginary circle of the specified radius. The resulting arc is the segment of the imaginary circle from that tangent point for 'offset' degrees along the imaginary circle.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent docs; this is great, super good change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks real, I'm guessing something changed in the program memory?

When sketch mode doesn't know how to draw something it defaults to a straight line.

@jessfraz
Copy link
Contributor

can we merge this?

@jessfraz
Copy link
Contributor

will leave to @adamchalmers to rebase

@jtran
Copy link
Collaborator

jtran commented Aug 28, 2024

Replaced by #3705.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tangentialArc should not have a "to" mode
5 participants