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

[MU3] Paren time sig #6917

Closed
wants to merge 2 commits into from
Closed

[MU3] Paren time sig #6917

wants to merge 2 commits into from

Conversation

ecstrema
Copy link
Contributor

Resolves: https://musescore.org/en/node/313219#comment-1042033

NOTE: This PR needs elerouxx's PR to be merged first (#6550 )

MuseJazz includes parentheses by default in its time signatures. With elerouxx's PR #6550 it is now possible to append them manually. As such, new scores created with 3.6 will not have this trailing parenthesis by default.

The change to MuseJazz hasn't been done yet, but because I have no development machine right now, I am testing with the build here and the artifacts generated while I remove the MuseJazz Parentheses.

  • 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

@ecstrema ecstrema force-pushed the paren_time_sig branch 2 times, most recently from 4f39313 to 18f1700 Compare November 24, 2020 02:25
@ecstrema
Copy link
Contributor Author

@elerouxx do you have any clue why these changes do not set the parser string?

@elerouxx
Copy link
Contributor

@elerouxx do you have any clue why these changes do not set the parser string?

You mean timeSig.cpp @ 261? I don't think the (old) variable is getting set true for these types. This variable seems to be for a very old time signature implementation that had a sequence of nominators. (additive?).

@ecstrema ecstrema marked this pull request as ready for review November 25, 2020 05:30
@ecstrema
Copy link
Contributor Author

okay, this is now ready. As soon as elerouxx's PR is merged this one can be too. (or both can be merged at the same time with this PR)

So here's how it works: When older scores are loaded, if these scores are using MuseJazz, then the symbols are corrected.

@ecstrema ecstrema changed the title Paren time sig [mu3] Paren time sig Nov 25, 2020
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Nov 25, 2020

The vtest fails are actually fixes (esp. for Emmentaler), before:
musejazz-4-ref
emmentaler-4-ref
after:
emmentaler-4-1
musejazz-4-1

Similar in #6550 (comment)

Note that the MuseJazz text reads Emmentaler...

@igorkorsukov igorkorsukov changed the title [mu3] Paren time sig [MU3] Paren time sig Feb 5, 2021
@vpereverzev vpereverzev added the archived PRs that have gone stale but could potentially be revived in the future label May 5, 2021
@vpereverzev
Copy link
Member

3.x merges are closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived PRs that have gone stale but could potentially be revived in the future
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants