You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Path: path.setHandle to the handle's own point leaves a smooth anchor unchanged - #7464
#7305 made path.setHandle record no undo step when the handle is already at the requested point. It checked this by running the handle edit on a copy of the anchor and comparing the whole anchor. On a smooth anchor that edit recomputes the other handle from its length and the new direction, so a request for the handle's own point could still change the anchor: when the other handle was not exactly in line, it was turned into line, and when it was in line, the recomputed value could round to a different number. Either way the command edited the path and recorded an undo step.
path.setHandle now compares the requested point with the handle itself. When they are equal it leaves the document as it is and records no undo step, unless independent would turn a smooth anchor into a corner, which is still an edit.
docs/mcp.md already describes this behavior, so no doc changes.
How it was tested
New test in crates/engine/src/tests/anchor_edit_work.rs, a_handle_already_in_place_on_a_smooth_anchor_records_no_undo_step: on a smooth anchor whose handles are not in line, path.setHandle to the point of its outgoing handle and then of its incoming handle records no undo step and leaves the anchor unchanged; the same request with independent makes the anchor a corner and records one undo step; repeating it on the corner records none. It fails without the change ("path.setHandle on the out handle recorded an undo step").
Focused checks on Linux x86-64, on main at 1e4e3534f:
cargo test -p vectorcraft-engine --lib filtered to anchor_edit_work (4 passed) and handle (48 passed), 0 failed
cargo fmt --all -- --check reports no diff in the changed files; on this main it reports one in crates/svg/src/lib.rs, which this pull request does not touch.
The branch is one commit on main at 1e4e3534f. cargo xtask ci runs in this pull request's CI.
No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
messages. Errors are returned as Result and shown to the user
(why and how).
Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
Every new asset has a row in ASSETS.md.
Summary by CodeRabbit
Bug Fixes
Setting a handle to its existing position on a smooth anchor no longer creates an unnecessary edit or undo step, including when the opposite handle is not aligned.
When independent handles are requested, an anchor still changes to a corner if needed. Repeating the request after that change does not add another undo step.
… unchanged
path.setHandle checked for a no-op by running the handle edit on a copy
and comparing the whole anchor. On a smooth anchor the edit recomputes
the other handle, so when that handle was not exactly in line, or the
recomputed value rounded differently, a request for the handle's own
point realigned the other handle and recorded an undo step.
The check now compares the requested point with the handle itself, and
still lets independent turn a smooth anchor into a corner.
Reviewing files that changed from the base of the PR and between 541059f and d7d22a8.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
path.setHandle now skips edits when the requested handle matches the selected handle, except when independent: true must convert a non-corner anchor to a corner. Tests cover the anchor state and undo-step counts.
Changes: Handle update behavior
Layer / File(s)
Summary
Handle comparison and test coverage crates/engine/src/cmd/path.rs, crates/engine/src/tests/anchor_edit_work.rs
The command returns without editing when the requested handle matches the selected handle, unless independent: true must convert a non-corner anchor to a corner. Tests cover both unchanged handles on a smooth anchor, conversion to a corner, and undo-step counts.
Setting a handle to the position it already has no longer records a spurious undo step on smooth anchors, and converting a smooth anchor to a corner with the independent option still works. The change is small and covered by a targeted test, so merge risk is minimal. The CI gate result should be checked before merging.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: path.setHandle no longer leaves a smooth anchor modified when the requested point equals the handle's own point. It is specific, and the changeset (path.rs skip logic …
Description check
The description follows the template. It explains the what and why with the root cause, and it covers testing with the new test name, the commands run, and results. Checklist items are present, with t…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: path.setHandle no longer leaves a smooth anchor modified when the requested point equals the handle's own point. It is specific, and the changeset (path.rs skip logic and a new test) matches it.
Full details: Description check
Explanation
The description follows the template. It explains the what and why with the root cause, and it covers testing with the new test name, the commands run, and results. Checklist items are present, with the first left unchecked because CI is still pending and the other three marked as done. The description is mostly complete.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 1e4e353 and 541059f.
Files selected for processing: 2
crates/engine/src/cmd/path.rs
crates/engine/src/tests/anchor_edit_work.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Merged, thanks @dotCooCoo! It passed its CI checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
#7305 made
path.setHandlerecord no undo step when the handle is already at the requested point. It checked this by running the handle edit on a copy of the anchor and comparing the whole anchor. On a smooth anchor that edit recomputes the other handle from its length and the new direction, so a request for the handle's own point could still change the anchor: when the other handle was not exactly in line, it was turned into line, and when it was in line, the recomputed value could round to a different number. Either way the command edited the path and recorded an undo step.path.setHandlenow compares the requested point with the handle itself. When they are equal it leaves the document as it is and records no undo step, unlessindependentwould turn a smooth anchor into a corner, which is still an edit.docs/mcp.mdalready describes this behavior, so no doc changes.How it was tested
New test in
crates/engine/src/tests/anchor_edit_work.rs,a_handle_already_in_place_on_a_smooth_anchor_records_no_undo_step: on a smooth anchor whose handles are not in line,path.setHandleto the point of its outgoing handle and then of its incoming handle records no undo step and leaves the anchor unchanged; the same request withindependentmakes the anchor a corner and records one undo step; repeating it on the corner records none. It fails without the change ("path.setHandle on the out handle recorded an undo step").Focused checks on Linux x86-64, on
mainat1e4e3534f:cargo check --workspace --testscargo clippy -p vectorcraft-engine --all-targets -- -D warningscargo test -p vectorcraft-engine --libfiltered toanchor_edit_work(4 passed) andhandle(48 passed), 0 failedcargo fmt --all -- --checkreports no diff in the changed files; on thismainit reports one incrates/svg/src/lib.rs, which this pull request does not touch.The branch is one commit on
mainat1e4e3534f.cargo xtask ciruns in this pull request's CI.Checklist
cargo xtask cipasses (fmt, clippy, tests, assets, brands, layers, wasm).unwrap(),expect(),panic!,unreachable!,todo!orunimplemented!outside tests, and no unchecked indexing on data from files, parameters ormessages. Errors are returned as
Resultand shown to the user(why and how).
ASSETS.md.Summary by CodeRabbit