Skip to content

fix: insert sketches and datums at the history cursor - #49

Merged
jeffglousher merged 4 commits into
jackControls:mainfrom
jeffglousher:fix/sketch-insert-at-rollback
Aug 21, 2026
Merged

fix: insert sketches and datums at the history cursor#49
jeffglousher merged 4 commits into
jackControls:mainfrom
jeffglousher:fix/sketch-insert-at-rollback

Conversation

@jeffglousher

@jeffglousher jeffglousher commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test plan

  • new_sketch_is_inserted_at_the_build_cursor covers rollback → sketch while a later sketch exists
  • cargo test -p nbcad-sketch --test solid_history

@jackControls

Copy link
Copy Markdown
Owner

Test does not compile — please fix before merge.

Local run of cargo test -p nbcad-sketch --test solid_history fails:

error[E0507]: cannot move out of `feature.name` which is behind a shared reference
   --> crates/sketch/tests/solid_history.rs:348
348 |         .map(|feature| feature.name)

FeatureTree::iter() yields &Feature and Feature.name: String (crates/core/src/feature.rs), so the test file never compiles. Existing tests in the same file use feature.name.as_str() (line 300) or feature.name == name (line 176). Fix: feature.name.clone() (or .as_str()).

CI didn't catch this because mcp-server.yml only tests mcp-server / nbcad-export / xtask — it never compiles crates/sketch's integration tests. Please:

  1. Fix the line (.clone())
  2. Run cargo test -p nbcad-sketch --test solid_history and confirm green
  3. Tick the test-plan checkbox and push

The change itself (insert_at_rollback for sketches/planes) reviewed fine.

@jeffglousher

Copy link
Copy Markdown
Collaborator Author

Fixed — feature.name.clone() so the iterator no longer moves out of &Feature.

cargo test -p nbcad-sketch --test solid_history — 4 passed.

@jeffglousher
jeffglousher merged commit bb6f0e1 into jackControls:main Aug 21, 2026
6 checks passed
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.

2 participants