[MU3] Fix #313195: enable the synalepha shortcut in lyrics edit mode

[Asmatzaile]

Resolves: https://musescore.org/en/node/313195

Having PR #6928 been merged for 3.6 Beta, I tried using the introduced shortcut and it failed.
I think that this PR solves that problem, because the needed key wasn't Qt::Key_hyphen but Qt::Key_Minus.
Although I yet don't know how to use cmake properly and get errors when trying to compile, I'll test the artifacts when they are generated.
This will conflict with #7099, so maybe a rebase will be needed (would that be done with git checkout <my-branch-name> and git rebase 3.x?).
editlyrics.cpp was also changed because, as it was, pressing ctrl ignored the commands from textedit.cpp

• 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

[Jojo-Schmitz]

It should not conflict with #7099, it changes code at a sufficientlly different spot (the diffs don't overlap).

Once you got it working, please make sure to also submit a PR for master (or just let me know and I'll add it to my #7100)

[Asmatzaile]

It should not conflict with #7099, it changes code at a sufficientlly different spot (the diffs don't overlap).

Once you got it working, please make sure to also submit a PR for master

Oh, that's nice. Will do!

[Jojo-Schmitz]

Also make sure to have your commit title starting with "Fix #313195: " (as I've just reopened that issue as it is not fixed)

[Jojo-Schmitz]

This code change does work for me, prior it did not.

[Jojo-Schmitz]

Actually I've just added it to #7100 now (so for master)

[Asmatzaile]

Unfortunately, although now the shortcut does work in text edit mode, it doesn't in lyrics edit mode, where it would be most helpful. Anyway, it's a step forward towards the solution...

[Jojo-Schmitz]

Oh, darn, I guess in lyrics mode the Ctrl - is used differently and the added Alt doesn't change that

[Jojo-Schmitz]

It might help if you'd join the Developers' chat on Telegram https://t.me/musescoreeditorchat

[Asmatzaile]

Oh, darn, I guess in lyrics mode the Ctrl - is used differently and the added Alt doesn't change that

Ahhh... So I suppose I should find where the code for adding this symbol (-) is and add a conditional statement or something like that

[Asmatzaile]

Maybe, in editlyrics.cpp, line 87...

case Qt::Key_Minus: if (Qt::ControlModifier & Qt::AltModifier) { insertSym(ed, SymId::lyricsElision); return true; } else if (editData.control(textEditing)) { // change into normal minus editData.modifiers &= ~CONTROL_MODIFIER; return false; } else lyricsMinus(); break;

but insertSym and ed should be defined/declarated/whatever

[Jojo-Schmitz]

rather line 67
Instead if ed use editData, for insertSym() you need #include "libmscore/textbase.h", but is a protected member of that class.

[Asmatzaile]

rather line 67
Instead if ed use editData, for insertSym() you need #include "libmscore/textbase.h", but is a protected member of that class.

What does that mean? Should I move insertSym() to public?

[Jojo-Schmitz]

rather if (editDate.modifier & Qt::ControlModifier & Qt::AltModifier) I think
Or if ( editData.control(textEditing) && (editDate.modifier & Qt::AltModifier))

[Asmatzaile]

In function Ms::ScoreView::editKeyLyrics(): /home/runner/work/MuseScore/MuseScore/mscore/editlyrics.cpp:72: undefined reference to Ms::insertSym(Ms::EditData&, Ms::MSQE_SymId::SymId)

[Jojo-Schmitz]

Adding the prototype here doesn't make a protected member visible to a foreign class

[Jojo-Schmitz]

How about just this:

diff --git a/libmscore/textedit.cpp b/libmscore/textedit.cpp
index 03e01d43e..dc7522c06 100644
--- a/libmscore/textedit.cpp
+++ b/libmscore/textedit.cpp
@@ -458,7 +458,7 @@ bool TextBase::edit(EditData& ed)
                         }
                   }
             if (ctrlPressed && altPressed) {
-                  if (ed.key == Qt::Key_hyphen) {
+                  if (ed.key == Qt::Key_Minus) {
                         insertSym(ed, SymId::lyricsElision);
                         return true;
                         }
diff --git a/mscore/editlyrics.cpp b/mscore/editlyrics.cpp
index c7b951ddf..0f4e247a1 100644
--- a/mscore/editlyrics.cpp
+++ b/mscore/editlyrics.cpp
@@ -66,6 +66,8 @@ bool ScoreView::editKeyLyrics()

             case Qt::Key_Minus:
                   if (editData.control(textEditing)) {
+                        if ((editData.modifiers & Qt::AltModifier) && editData.element->edit(editData))
+                              break;
                         // change into normal minus
                         editData.modifiers &= ~CONTROL_MODIFIER;
                         return false;

Seems to work just fine and doesn't need a protected method to get made public and avoids code duplication. win-win-win ;-)

[Asmatzaile]

How about just this:
[...]
Seems to work just fine and doesn't need a protected method to get made public and avoids code duplication. win-win-win ;-)

Yes! It works! Many thanks Jojo :))

[Jojo-Schmitz]

Actually better add a _score->update(); before that break; (and add the braces)

[Asmatzaile]

Actually better add a _score->update(); before that break; (and add the braces)

Should the ending brace be before or after the break:?
Also at line 54 the if statement that includes a _score->update(): doesn't have any braces

[Jojo-Schmitz]

                        if ((editData.modifiers & Qt::AltModifier) && editData.element->edit(editData)) {
                              _score->update();
                              break;
                              }

In line 54 it is the only statement in the block, so the braces are not strictly needed (they are for master though, required by the new coding style)

[Jojo-Schmitz]

The mtest failures seem to not be your fault...