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.

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

fix 23187: Change UI in prefrences for exporting musicxml #4368

Closed
wants to merge 1 commit into from
Closed

fix 23187: Change UI in prefrences for exporting musicxml #4368

wants to merge 1 commit into from

Conversation

KuribohG
Copy link

No description provided.

@Jojo-Schmitz
Copy link
Contributor

Did you already sign the CLA and if is so under what name?

@KuribohG
Copy link
Author

Did you already sign the CLA and if is so under what name?

Yes. Here's my info:
Full name: Yuheng Zou
E-Mail: zouyuheng1998@gmail.com

@KuribohG
Copy link
Author

KuribohG commented Dec 11, 2018

Are these tests exactly correct?
I think the options for the test is not correct. In the old UI, we don't allow users export breaks without exporting layouts. However, in the test, PREF_EXPORT_MUSICXML_EXPORTLAYOUT is false and PREF_EXPORT_MUSICXML_EXPORTBREAKS is MANUAL. So in the new UI, this settings should print informations of manually created breaks. That's why some new lines are printed in the test.
In the old version, we must export layouts when we export breaks, so these lines were hidden when PREF_EXPORT_MUSICXML_EXPORTLAYOUT is false.
I tested this on a manually created score. The new code in exportxml.cpp worked fine, but the old code didn't work.

@Jojo-Schmitz
Copy link
Contributor

then it seems the *-ref.* files need to get adjusted

@lvinken
Copy link
Contributor

lvinken commented Dec 11, 2018

Looks good ! Almost (but not completely) OK: the system-layout should only be printed if "export all layouts" is true.

Obviously, the autotester should also pass. I would prefer to run the autotester with "do not export breaks" (as this isolates it from precise layout details) en remove "print new-system" and print new-page" from the files in mtest/musicxml/io.

@KuribohG
Copy link
Author

I have fixed this. Now it passed the tests, and I also tested it on some files manually.

I'm very sorry that I wrongly deleted the original fork repo. I must start a new pull request to merge this fix so I close this one.

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.

None yet

3 participants