Bug 442079

qWarning() << "scResult debug:" << activeShortcut << intListFromShortcut(activeShortcut) << shortcutFromIntList(intListFromShortcut(activeShortcut));

causes this:
scResult debug: (QKeySequence("Alt+B, Alt+F, Alt+G")) (134217794) (QKeySequence("Alt+B"))

The issue is in KGlobalAccelPrivate::intListFromShortcut:

According to QKeySequence documentation it could contain up to 4 keys.[1]

> for (const QKeySequence &sequence : cut) {
>     ret.append(sequence[0]);
> }
Hence, in this code all keys except the first one are lost.

[1] https://doc.qt.io/qt-5/qkeysequence.html#QKeySequence-2

Good investigation.

Lets just set a limit on our input prompt to stop when we hit 4 keys then, so it's at least clear to the user.

(In reply to David Edmundson from comment #3)
> Good investigation.
> 
> Lets just set a limit on our input prompt to stop when we hit 4 keys then,
> so it's at least clear to the user.

limit is already set, I can't set more than 4 keys (with modifiers, etc.)
The problem is that only first one is stored.

e.g. if I set something like "Alt+B, Alt+F, Alt+G" it will store only "Alt+B", silently omitting other keys.

Currently we're using int <-> QKeySequence which will definitely cause sequence truncation.
I'd suggest using one of the following:
int        <---> QKeySequence (dumb impl)
quint128 ? <---> QKeySequence (each key [1] with modifiers [2] uses 32 bits, use a 128bit type if exist)
int        <-X-> QKeySequence (avoid conversion and transfer )

[1] https://doc.qt.io/qt-5/qt.html#Key-enum
[2] https://doc.qt.io/qt-5/qt.html#KeyboardModifier-enum

(In reply to Oleg Solovyov from comment #4)
> (In reply to David Edmundson from comment #3)
> > Good investigation.
> > 
> > Lets just set a limit on our input prompt to stop when we hit 4 keys then,
> > so it's at least clear to the user.
> 
> limit is already set, I can't set more than 4 keys (with modifiers, etc.)
> The problem is that only first one is stored.
> 
> e.g. if I set something like "Alt+B, Alt+F, Alt+G" it will store only
> "Alt+B", silently omitting other keys.
> 
> Currently we're using int <-> QKeySequence which will definitely cause
> sequence truncation.
> I'd suggest using one of the following:
> int        <---> QKeySequence (dumb impl)
> quint128 ? <---> QKeySequence (each key [1] with modifiers [2] uses 32 bits,
> use a 128bit type if exist)
> int        <-X-> QKeySequence (avoid conversion and transfer )
> 
> [1] https://doc.qt.io/qt-5/qt.html#Key-enum
> [2] https://doc.qt.io/qt-5/qt.html#KeyboardModifier-enum

Another idea, conversion based on QKeySequence::{from,to}String()

made a patch [1] but it's WIP yet: actions still aren't invoked when count > 1

[1] https://invent.kde.org/osolovyov/kglobalaccel/-/commit/5d79966be20b8df79c25f3fd9559dd5387f1c1f9

https://invent.kde.org/osolovyov/kglobalaccel/-/commits/work/avoid-convert-keyseq

Almost done.

Not Implemented Yet:
Inexact matches:
1) New sequence will be shadowed by existing one
2) New sequence will shadow existing one

bugfixes:
Don't ungrab key still to be grabbed

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kdeclarative/-/merge_requests/81

Another merge request not caught by bug janitor:
(edited description, added bug number)

https://invent.kde.org/frameworks/kglobalaccel/-/merge_requests/27

Fixed by Oleg Solovyov with:
- https://invent.kde.org/frameworks/kglobalaccel/-/commit/6e4494fe7c8c4649026111c63431ab4ab33814e5
- https://invent.kde.org/frameworks/kxmlgui/-/commit/33d6d300c029922c16a9523dda22fd55cdbaff9d
- https://invent.kde.org/frameworks/kdeclarative/-/commit/0bcc4e8e5503008632aca51586716f57290262b3
- https://invent.kde.org/plasma/plasma-desktop/-/commit/ea407197d6da1fa2bda62beac3d5a8fa14c16dd5