Skip to content

Refactor EditStaff::apply() to use m_orgStaff for undo stack access#32828

Draft
CubikingChill wants to merge 1 commit intomusescore:masterfrom
CubikingChill:fix-editstaff-apply-undo-stack
Draft

Refactor EditStaff::apply() to use m_orgStaff for undo stack access#32828
CubikingChill wants to merge 1 commit intomusescore:masterfrom
CubikingChill:fix-editstaff-apply-undo-stack

Conversation

@CubikingChill
Copy link
Copy Markdown
Contributor

Use m_orgStaff->score()->undoStack() instead of m_staff->score()->undoStack() for better code clarity and semantic correctness.

While both references point to the same undo stack (since m_staff and m_orgStaff share the same Part), using m_orgStaff makes the intent clearer: we're accessing the undo stack of the score containing the staff being modified, not the temporary UI copy used for staging changes.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

Use m_orgStaff->score()->undoStack() instead of m_staff->score()->undoStack()
for better code clarity and semantic correctness.

While both references point to the same undo stack (since m_staff and m_orgStaff
share the same Part), using m_orgStaff makes the intent clearer: we're accessing
the undo stack of the score containing the staff being modified, not the temporary
UI copy used for staging changes.
@RomanPudashkin
Copy link
Copy Markdown
Contributor

@zacjansheski please test the "Staff/Part properties" dialog (there are no crashes or other problems when pressing Apply)

@CubikingChill CubikingChill marked this pull request as draft March 30, 2026 13:49
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.

3 participants