Fix part syncing when a Box item is pasted#33675
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a new Score::insertBox(MeasureBase*) overload and moves box initialization, linkage (tick/prev/next), FBOX setup, undo registration (InsertMeasures), and option handling into it. The ElementType overload now validates/creates a MeasureBase and delegates to the new overload. Measure::drop was simplified to call the new insertBox overload directly. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/engraving/dom/measure.cppIn file included from src/engraving/dom/measure.cpp:28: ... [truncated 1258 characters] ... stall/lib/clang/18/include" src/engraving/editing/edit.cppIn file included from src/engraving/editing/edit.cpp:26: ... [truncated 2200 characters] ... in file "src/clang/cTrans.ml", line 5389, characters 6-70 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/engraving/editing/edit.cpp (1)
4759-4811:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRebind the incoming box to the destination score before insertion.
This overload now accepts an existing
MeasureBase*, but it never updatesbox->score(). The new caller is the paste/drop path, so the box can arrive as a clone from another score context; in that caseInsertMeasuresandmanageExclusionFromParts(false)will run with stale ownership.Suggested fix
MeasureBase* Score::insertBox(MeasureBase* box, MeasureBase* beforeMeasure, const InsertMeasureOptions& options) { IF_ASSERT_FAILED(box) { return nullptr; } ElementType type = box->type(); const bool isFrame = type == ElementType::FBOX || type == ElementType::HBOX || type == ElementType::TBOX || type == ElementType::VBOX; if (!isFrame) { return nullptr; } + + if (box->score() != this) { + box->setScore(this); + } Fraction tick;If
setScore()does not cascade to children, those descendants need rebinding too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/engraving/editing/edit.cpp` around lines 4759 - 4811, The incoming MeasureBase* box passed into Score::insertBox must be rebound to this Score before insertion so InsertMeasures and manageExclusionFromParts(false) operate on the correct ownership; call box->setScore(this) (and if setScore does not update descendants, traverse box's children and call setScore(this) on them as well) after computing tick and before undo(new InsertMeasures(...)) and before manageExclusionFromParts; ensure you update any nested frames/contents (use existing accessors on MeasureBase/FBox/Measure) so all descendants reference this Score.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/engraving/editing/edit.cpp`:
- Around line 4759-4811: The incoming MeasureBase* box passed into
Score::insertBox must be rebound to this Score before insertion so
InsertMeasures and manageExclusionFromParts(false) operate on the correct
ownership; call box->setScore(this) (and if setScore does not update
descendants, traverse box's children and call setScore(this) on them as well)
after computing tick and before undo(new InsertMeasures(...)) and before
manageExclusionFromParts; ensure you update any nested frames/contents (use
existing accessors on MeasureBase/FBox/Measure) so all descendants reference
this Score.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f17f4ad-abcc-4a84-b80d-8d10668cb5d0
📒 Files selected for processing (3)
src/engraving/dom/measure.cppsrc/engraving/dom/score.hsrc/engraving/editing/edit.cpp
|
@ajuncosa Tested and approved in Ubuntu 24.04.4 LTS. |
3 participants
Resolves: #33657
Boxelements were not being synced across parts when pasted. TheMeasure::drop()function was missing a call tomanageExclusionFromParts()to properly trigger this syncing. Since the relevant part of the function seemed to have the same behaviour asScore::insertBox(), I slightly refactored that part of the code as well as theinsertBoxfunction to avoid duplication.