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.

Sign up for GitHub

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

Jump to bottom

Fix #307530: MusicXML export for hidden staves #11953

Merged
merged 1 commit into from
Jun 28, 2022
Merged

Fix #307530: MusicXML export for hidden staves #11953

merged 1 commit into from
Jun 28, 2022

Conversation

HemantAntony
Copy link
Contributor

@HemantAntony HemantAntony commented Jun 9, 2022 •
edited by shoogle
Loading

Resolves: #307530

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@shoogle FYI

@RomanPudashkin
Copy link
Contributor

Thanks for the fix! Could you add a unit test to check this fix? You can use this PR as an example:
#11413

@HemantAntony HemantAntony marked this pull request as draft June 9, 2022 10:07
@HemantAntony HemantAntony force-pushed the 307530-musicxml_export_missing_hidden_staves branch 5 times, most recently from 9b55b6f to 8464954 Compare June 10, 2022 04:27
@HemantAntony HemantAntony force-pushed the 307530-musicxml_export_missing_hidden_staves branch from 8464954 to e41c7ed Compare June 25, 2022 16:44
@HemantAntony
Copy link
Contributor Author

I'll fix the unit tests tomorrow

@Jojo-Schmitz
Copy link
Contributor

It'll be an easy fix

@HemantAntony HemantAntony force-pushed the 307530-musicxml_export_missing_hidden_staves branch from e41c7ed to d2c217a Compare June 27, 2022 14:29
@HemantAntony HemantAntony force-pushed the 307530-musicxml_export_missing_hidden_staves branch from d2c217a to c6dc78c Compare June 27, 2022 16:32
shoogle
shoogle approved these changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@shoogle shoogle left a comment

Choose a reason for hiding this comment

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

Looks good! While testing I noticed a very strange problem with hiding staves after a MusicXML file has been imported into MuseScore, but the problem already exists in the nightly builds so I created a separate issue for it (see #12186). I think this one is ready to merge as-is.

@shoogle shoogle marked this pull request as ready for review June 28, 2022 00:23
@RomanPudashkin RomanPudashkin merged commit 1445841 into musescore:master Jun 28, 2022
@HemantAntony HemantAntony deleted the 307530-musicxml_export_missing_hidden_staves branch June 28, 2022 06:17
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
c5918be 
Backport of musescore#11953
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Nov 7, 2023
Merged
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
d1ab553 
Backport of musescore#11953
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
ceae337 
Backport of musescore#11953
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
1fd9c15 
Backport of musescore#11953

Mu3 doesn't allow for staves to get hidden, only parts, so needed to adjust the mtest file
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
44d58f0 
Backport of musescore#11953

Mu3 doesn't allow for staves to get hidden, only parts, at least that is what I guess to be the reason for the mtest failure, so need to adjust the newly added mtest file

Allso need to fix another mtest file, see d842fce, b8a0361 and f36f842
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 7, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
56172d5 
Backport of musescore#11953

Mu3 doesn't allow for staves to get hidden, only parts, at least that is what I guess to be the reason for the mtest failure, so need to adjust the newly added mtest file

Also need to fix another mtest file which doesn't exist in Mu4, as it stems from (the backport of) musescore#8763, part 2, which never got ported to master.
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Nov 14, 2023
@HemantAntony @Jojo-Schmitz
Fix #307530: MusicXML export for hidden staves
da12f44 
Backport of musescore#11953

Mu3 doesn't allow for staves to get hidden, only parts, at least that is what I guess to be the reason for the mtest failure, so need to adjust the newly added mtest file

Also need to fix another mtest file which doesn't exist in Mu4, as it stems from (the backport of) musescore#8763, part 2, which never got ported to master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tantacrul Tantacrul Tantacrul left review comments

@shoogle shoogle shoogle approved these changes

@RomanPudashkin RomanPudashkin RomanPudashkin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@HemantAntony @RomanPudashkin @Jojo-Schmitz @shoogle @Tantacrul