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 #15407: Hide zero-length lyrics dashes #15459

Merged
merged 3 commits into from Apr 11, 2023

Conversation

asattely
Copy link
Contributor

@asattely asattely commented Dec 20, 2022

Resolves: #15407
Resolves: #15093

See comment, the solution for this one has changed.

@asattely
Copy link
Contributor Author

This one ended up being a little more in-depth than I expected it to be.

  • If the user places exactly one underscore (or dash) and then ends editing, the melisma is deleted and the alignment of the lyric is reset.
  • If the user places multiple dashes, the lyric will be left-justified (as it spans multiple notes), but if the editing ends with that string of dashes, the original lyric will again have its alignment reset and the "melisma" will be deleted.

@asattely asattely force-pushed the lyrics-dashes-to-nowhere branch 2 times, most recently from 5fb0a5f to 28317e3 Compare January 8, 2023 23:54
// - currently used to determine the first lyric of a melisma
//---------------------------------------------------------

Lyrics* Lyrics::prevLyrics() const
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest moving it into navigate.h/cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid I don't understand this one. You want the lyrics class to be in navigate.h?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, only this function :)

@asattely asattely force-pushed the lyrics-dashes-to-nowhere branch 2 times, most recently from 2ba7519 to f1f1f8d Compare March 24, 2023 20:31
@asattely asattely force-pushed the lyrics-dashes-to-nowhere branch 2 times, most recently from 1715bc8 to d881c91 Compare April 4, 2023 15:27
@asattely
Copy link
Contributor Author

asattely commented Apr 4, 2023

Issue #15093 was extremely similar, so I just added it to this PR since it deals with the same behavior.

If the ticks for a lyric become zero for any reason now (not just when editing a lyric finishes), the lyric re-lays-out to make sure invalid segments don't remain.

@asattely asattely force-pushed the lyrics-dashes-to-nowhere branch 3 times, most recently from 27f99a6 to 6021b7d Compare April 5, 2023 22:15
// - currently used to determine the first lyric of a melisma
//---------------------------------------------------------

Lyrics* Lyrics::prevLyrics() const
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not exactly what I meant :) You should declare a function in navigate.h (similar to prevChordRest):

extern Lyrics* prevLyrics(const Lyrics* lyrics);

Then use this new function in TextBase::endEdit(EditData& ed):

 if (isLyrics()) {
     Lyrics* prevLyrics = prevLyrics(toLyrics(this));
     if (prevLyrics) {
         ...
     }
 }


Copy link
Contributor Author

@asattely asattely Apr 11, 2023

Choose a reason for hiding this comment

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

For the record, why do we want to do this? Is there a reason prevLyrics shouldn't be a method?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it can iterate implicitly over the whole score. And in general, has low cohesion in this class (it does not depend on anything within this class, and can easily be moved out of it and take it as an argument). We want engraving elements to be as simple as possible

@RomanPudashkin RomanPudashkin merged commit 99f20ff into musescore:master Apr 11, 2023
11 checks passed
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.

[MU4 Issue] Lyrics layout breaks on incomplete input Melisma remains after changing note value
3 participants