Fix #313195: Add Synalepha shortcut #6928

Asmatzaile · 2020-11-24T15:02:14Z

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

Synalephas are common enough to have their own dedicated shortcut. ~~As suggested by @jeetee , I added an action to the shortcut.cpp listing.~~ I changed the textedit.cpp file.

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

RobFog · 2020-11-24T15:16:04Z

I guess it would make sense to use the pull request title as the commit message?

Asmatzaile · 2020-11-24T15:39:55Z

I guess it would make sense to use the pull request title as the commit message?

Did I do it right?

RobFog · 2020-11-24T15:51:42Z

Looks like you ended up with four commits? @Jojo-Schmitz can probably assist.

Jojo-Schmitz · 2020-11-24T15:51:44Z

I made sure the commit message title starts with "fix #424242:"

This exactly what you did not do. You have the issue number in the PR title, not in the commit title.
And you have 4 commits rather than just one
3rd and 4th are really not needed nor wanted

Try git rebase -i HEAD~4, replace the 2nd-4th pick with just f, then git push -f

With this PR so far we have a shortcut, viaible and configurable, but not the command behind it, as far as I can tell. You need a "synalepha" command too (in cmd.cpp, IIRC) that then actually does the 'dirty work' of adding the synalepha

Asmatzaile · 2020-11-24T18:18:47Z

I made sure the commit message title starts with "fix #424242:"

This exactly what you did not do. You have the issue number in the PR title, not in the commit title.
And you have 4 commits rather than just one
3rd and 4th are really not needed nor wanted

Try git rebase -i HEAD~4, replace the 2nd-4th pick with just f, then git push -f

With this PR so far we have a shortcut, but not the command behind it, as far as I can tell, you need a "synalepha" command in cmd.cpp that then actually does the dirty work of adding the synalepha

Oh, I misread commit for title.
I will try doing what you say, thanks :)

Jojo-Schmitz · 2020-11-24T18:39:16Z

OK, good, just one commit now. And we now have a shortcut that we can set, change, save.
But still nothing happens when that shortcut actually gets used, i.e. no synalepha gets added.

Asmatzaile · 2020-11-24T18:55:28Z

If I've done it right, now the shortcut should have the default of Ctrl+Alt+-, but I still have to edit cmd.cpp.
I don't know much of C++, so I hope to find a command that does something similar in that file.

Jojo-Schmitz · 2020-11-24T18:58:19Z

May not be cmd.cpp. Just check how it is done for e.g. "lyrics"

Asmatzaile · 2020-11-24T19:20:01Z

Now I'm completely lost.
For what I've read, I think I have to store the character in a variable like
char undertie = '‿';
and then somehow insert it

Jojo-Schmitz · 2020-11-24T19:32:48Z

Probably. But you need a function doing that and registering that with MuseScore

libmscore/textedit.cpp

Asmatzaile · 2020-11-25T22:04:04Z

Tests failing, I must have done something wrong when simplifying

Jojo-Schmitz · 2020-11-26T08:42:44Z

libmscore/textedit.cpp

@@ -455,6 +456,11 @@ bool TextBase::edit(EditData& ed)
                        }
                  }
            }
+            if (ctrlPressed && altPressed) {
+                  if (ed.key == Qt::Key_hyphen) {
+                        s = "\u203f"; // Unicode undertie


I guess we'd better use "<sym>lyricsElision</sym>" here. This should take the SMuFL symbol from the scores's Musical Text Font (including fallback to Bravura Text if need be) rather than relying on a 'normal' text font to have that glyph (and disaplay a box with a ? if it does not)

Or even better:

insertSym(ed, SymId::lyricsElision); return true;

Needs testing of course...

I will test when you want, I have many scores with Elision, and if success, at least 2 issues could be fixed.

I just tried i.e. 'manually', i.e. hand-editing an mscx file, and it works.
But whether my code above works remains to be tested ;-)

Really good☺

With the last commit I changed the symbol from unicode to smufl. As I originally used unicode because flat, sharp and natural symbols were drawn from unicode, I wonder whether those symbols should be changed too.

ecstrema · 2020-11-26T17:20:17Z

libmscore/textedit.cpp

@@ -455,6 +456,11 @@ bool TextBase::edit(EditData& ed)
                        }
                  }
            }
+            if (ctrlPressed && altPressed) {


If you want the shortcut to be configurable, you could add a configurable shortcut in shortucut.cpp like you did in the beginning, and compare that shortcut with what's pressed here.
Same for your other PR.

ie in shortcut.h there is a Shortcut class, with a keys method. you could get the Shortcut for this action and compare the key sequences.

Agreed, a configurable shortcut would be nice.

Any updates @JLWALTENER?

@vpereverzev It seems this PR is in Draft mode, the consensus seems to use the SMuFL symbols as preconised by @Jojo-Schmitz and I, for now the F2 palette present the SMuFL ones and the U+035C more availiable than the U+203F.
May be Asmatzaile will update his PR.

I updated the PR to use the SMuFL symbol instead of the unicode one.

I think I understand how I should change the shortcut.h and shortcut.cpp files to make the shortcut configurable, but I don't know what I should write in the textedit.cpp file to let it know about those shortcuts and thus execute the action.

Thanks, the SymID is mapped to the right unicode codepoint, could it be possible to have a test ?

I don't know how to test it. I tried compiling the code but I get an error (can't compile, install a generator) although I installed all the requirements specified on the developer's handbook

It doesn't build, there's a missing closing brace

libmscore/textedit.cpp

RobFog · 2020-12-09T23:59:12Z

The mtest failure is not your fault, by the way! See #7056.

Jojo-Schmitz · 2020-12-10T07:36:21Z

Looks good now. Now please squash all those commits into one using git rebase -i HEAD~10, replace all but the first pick by f or fixup, then git push -f

Asmatzaile · 2020-12-10T12:14:06Z

The mtest failure is not your fault, by the way! See #7056.

Nice to know! Now that #7056 is merged the mtest failure should disappear after I squash my commits, right?

RobFog · 2020-12-10T12:14:46Z

The mtest failure is not your fault, by the way! See #7056.

Nice to know! Now that #7056 is merged the mtest failure should disappear after I squash my commits, right?

You should rebase. Then it should disappear.

Asmatzaile · 2020-12-10T12:18:32Z

Looks good now. Now please squash all those commits into one using git rebase -i HEAD~10, replace all but the first pick by f or fixup, then git push -f

Done. Thanks for all your help! :)

Asmatzaile · 2020-12-14T12:06:08Z

PR is ready to be merged

Asmatzaile force-pushed the synalepha-shortcut branch 2 times, most recently from 6eae851 to 642c372 Compare November 24, 2020 18:27

Asmatzaile marked this pull request as draft November 24, 2020 20:40

Asmatzaile force-pushed the synalepha-shortcut branch from 7e497a9 to cde8e3e Compare November 25, 2020 18:01

Asmatzaile marked this pull request as ready for review November 25, 2020 18:02

Jojo-Schmitz reviewed Nov 25, 2020

View reviewed changes

libmscore/textedit.cpp Outdated Show resolved Hide resolved

Jojo-Schmitz reviewed Nov 25, 2020

View reviewed changes

libmscore/textedit.cpp Outdated Show resolved Hide resolved

Jojo-Schmitz mentioned this pull request Nov 25, 2020

MuseScore 3.6 Beta/RC/Release. PRs to be merged and issues to be fixed. #6887

Closed

Asmatzaile force-pushed the synalepha-shortcut branch from d7e8b8e to 2e2870e Compare November 25, 2020 21:54

Jojo-Schmitz reviewed Nov 26, 2020

View reviewed changes

ecstrema reviewed Nov 26, 2020

View reviewed changes

Jojo-Schmitz reviewed Dec 9, 2020

View reviewed changes

libmscore/textedit.cpp Show resolved Hide resolved

Jojo-Schmitz reviewed Dec 9, 2020

View reviewed changes

libmscore/textedit.cpp Show resolved Hide resolved

Asmatzaile force-pushed the synalepha-shortcut branch from 57fea60 to f7bf5f5 Compare December 9, 2020 21:40

Fix #313145: Add synalepha shortcut

45a6040

Asmatzaile force-pushed the synalepha-shortcut branch from f7bf5f5 to 45a6040 Compare December 10, 2020 12:17

Asmatzaile requested a review from vpereverzev December 10, 2020 16:47

vpereverzev merged commit 12225e6 into musescore:3.x Dec 14, 2020

Asmatzaile deleted the synalepha-shortcut branch December 14, 2020 16:40

Asmatzaile mentioned this pull request Dec 16, 2020

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

Merged

8 tasks

This was referenced Dec 16, 2020

[MU4] Enable Ctrl+Shift+# to add a sharp for some non-US keyboards, add Synalepha shortcut #7100

Merged

[MU4 Issue] PRs from 3.x to be merged into / ported to master #6079

Closed

Asmatzaile mentioned this pull request Aug 25, 2023

Make elision a customizable shortcut #19174

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #313195: Add Synalepha shortcut #6928

Fix #313195: Add Synalepha shortcut #6928

Asmatzaile commented Nov 24, 2020 •

edited

Loading

RobFog commented Nov 24, 2020

Asmatzaile commented Nov 24, 2020

RobFog commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020 •

edited

Loading

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020 •

edited

Loading

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020

Asmatzaile commented Nov 25, 2020

Jojo-Schmitz Nov 26, 2020 •

edited

Loading

JLWALTENER Nov 26, 2020

Jojo-Schmitz Nov 26, 2020 •

edited

Loading

JLWALTENER Nov 26, 2020

Asmatzaile Dec 8, 2020

ecstrema Nov 26, 2020 •

edited

Loading

ecstrema Nov 26, 2020

JLWALTENER Nov 26, 2020

vpereverzev Nov 30, 2020

JLWALTENER Nov 30, 2020

Asmatzaile Dec 8, 2020

JLWALTENER Dec 9, 2020

Asmatzaile Dec 9, 2020

Jojo-Schmitz Dec 9, 2020

RobFog commented Dec 9, 2020 •

edited

Loading

Jojo-Schmitz commented Dec 10, 2020 •

edited

Loading

Asmatzaile commented Dec 10, 2020 •

edited

Loading

RobFog commented Dec 10, 2020 •

edited

Loading

Asmatzaile commented Dec 10, 2020

Asmatzaile commented Dec 14, 2020

Fix #313195: Add Synalepha shortcut #6928

Fix #313195: Add Synalepha shortcut #6928

Conversation

Asmatzaile commented Nov 24, 2020 • edited Loading

RobFog commented Nov 24, 2020

Asmatzaile commented Nov 24, 2020

RobFog commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020 • edited Loading

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020 • edited Loading

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020

Asmatzaile commented Nov 24, 2020

Jojo-Schmitz commented Nov 24, 2020

Asmatzaile commented Nov 25, 2020

Jojo-Schmitz Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jojo-Schmitz Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecstrema Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobFog commented Dec 9, 2020 • edited Loading

Jojo-Schmitz commented Dec 10, 2020 • edited Loading

Asmatzaile commented Dec 10, 2020 • edited Loading

RobFog commented Dec 10, 2020 • edited Loading

Asmatzaile commented Dec 10, 2020

Asmatzaile commented Dec 14, 2020

Asmatzaile commented Nov 24, 2020 •

edited

Loading

Jojo-Schmitz commented Nov 24, 2020 •

edited

Loading

Jojo-Schmitz commented Nov 24, 2020 •

edited

Loading

Jojo-Schmitz Nov 26, 2020 •

edited

Loading

Jojo-Schmitz Nov 26, 2020 •

edited

Loading

ecstrema Nov 26, 2020 •

edited

Loading

RobFog commented Dec 9, 2020 •

edited

Loading

Jojo-Schmitz commented Dec 10, 2020 •

edited

Loading

Asmatzaile commented Dec 10, 2020 •

edited

Loading

RobFog commented Dec 10, 2020 •

edited

Loading