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 broken keyboard shortcut capturing for combos that include Shift #45

Closed
wants to merge 11 commits into from

Conversation

fyzix
Copy link
Contributor

@fyzix fyzix commented Aug 15, 2012

This should fix that many Shift key combos were displayed wrong
in the capture dialog and didn't work afterwards.

See commit message for details.

Tested on MacBook Pro and 10.8 and Qt 4.8, Would be good to test this
also on Windows/Linux, also with extended keyboards (numpad keys).

@travisbot
Copy link

This pull request passes (merged dc7997e0 into 2d506f3).

@lasconic
Copy link
Contributor

Hi,
I'm not exactly sure how to test this, maybe because I don't see the problem the same way you do in the first place.
I'm on Windows with a french keyboard. The key with , has ? if you press shift.
Before the fix, if I press Shift + , the shortcut is recorded as Shift + ? and indeed it doesn't work
After the fix, if I press Shift + , the shortcut is displayed as ~ and pressing shift + , doesn't work either. :-/

@fyzix
Copy link
Contributor Author

fyzix commented Aug 16, 2012

Hi,

thanks for checking this out. Maybe my pull request came a little early.
I'm still fixing a lot of things there, since arrow keys and other stuff don't
work correctly either. I'll keep investigating there. I'll also try french keyboard
layout.

On 16.08.2012, at 15:49, Nicolas Froment notifications@github.com wrote:

Hi,
I'm not exactly sure how to test this, maybe because I don't see the problem the same way you do in the first place.
I'm on Windows with a french keyboard. The key with , has ? if you press shift.
Before the fix, if I press Shift + , the shortcut is recorded as Shift + ? and indeed it doesn't work
After the fix, if I press Shift + , the shortcut is displayed as ~ and pressing shift + , doesn't work either. :-/


Reply to this email directly or view it on GitHub.

@travisbot
Copy link

This pull request fails (merged cd1a280f into 2d506f3).

@lasconic
Copy link
Contributor

lasconic commented Sep 8, 2012

How is it doing? Ready for more testing?

@fyzix
Copy link
Contributor Author

fyzix commented Sep 8, 2012

Hi,

after you told me it doesn't work on Windows with a French keyboard, I tested my local most recent version on Xp/Virtual box, different keyboard settings etc. For me it worked IIRC. But it's a hacky answer to the shortcomings of Qt, so I'm not sure if this should even
be merged. What do you think?

Jon

Am 08.09.2012 um 11:13 schrieb Nicolas Froment notifications@github.com:

How is it doing? Readu for more testing?


Reply to this email directly or view it on GitHub.

Example on a US keyboard layout, how it was before:

'Shift+,' would be captured as 'Shift+<',
'Shift+7' would be captured as 'Shift+&',

and such combinations did not work after configuring them,
because they are simply unpressable.

With the fix I'm proposing, 'Shift+,' is recognized as '<',
'Shift+8' as '*', and most importantly, they actually work as
shortcuts.

The fix feels a little bit hacky, because it relies on a Qt
implementation detail (the Qt::KeypadModifier flag), but there
seems to be no way in Qt to get the 'naked' keys (',' and '7'
in the examples above) when the user presses them with shift.
So I think this is still better than a broken shortcut
mechanism.

This is only tested on a MacBook Pro with OSX 10.8, so it
would be good if someone could test it on Windows/Linux,
preferably also with numpad keys.
Makes the code more readable and self-documenting.
Technically, it shouldn't be possible any more to have a
broken shortcut list, since you can't create conflicts
any more. But if you already have a broken shortcut list,
it might be helpful to see all conflicts.

Anyway, this is also a preparation to try a different
use for the "replace" button. Rather than allowing multiple
shortcuts for the same thing, it would be more useful to
be able to resolve the conflict right from the dialog box,
and actually that's how I would have understood the "Replace"
button: it deletes the key sequence from the conflicting
shortcut and sets it for the one I'm currently editing.
QLineEdit fields swallow certain keys on Mac,
even if they are in read-only mode. Extended
the fix for backspace to also handle delete.
Added a list of exceptions where the shift modifier must
not be removed (keys like Enter, Backspace, Esc etc.).

Now it's possible again to set Shift+Backspace,
for example.
squash! Allow Shift combos for
Since Qt often adds the KeypadModifier flag to keys,
e.g. the arrow keys, they get converted into something
unreadable by QKeySequence::NativeText.
@Jojo-Schmitz
Copy link
Contributor

I too got bitten by this issue, > doesn't work as a Shortcut, as on a German Keyboard this is Shift-< (and recoded as Shift-> in MuseScore

@Jojo-Schmitz
Copy link
Contributor

This PR may not be needed anymore, since the move to Qt5, does it?

@Jojo-Schmitz
Copy link
Contributor

can't this PR get closed?

@lasconic lasconic closed this Apr 4, 2014
Xmader added a commit to Xmader/MuseScore that referenced this pull request Nov 6, 2020
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.

4 participants