D6233: KKeyServer: fix handling of KeypadModifier.

[Martin Flöser]

graesslin requested changes to this revision.
graesslin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kkeyserver.cpp:160-180
> +    { Qt::Key_Space,      XK_KP_Space },
> +    { Qt::Key_Tab,        XK_KP_Tab },
> +    { Qt::Key_Enter,      XK_KP_Enter },
> +    { Qt::Key_Home,       XK_KP_Home },
> +    { Qt::Key_Left,       XK_KP_Left },
> +    { Qt::Key_Up,         XK_KP_Up },
> +    { Qt::Key_Right,      XK_KP_Right },

This looks very unrelated to the described change. Maybe an own commit?

> kkeyserver.cpp:765
>          if (g_rgQtToSymX[i].keySymQt == symQt) {
> +            if ((keyQt & Qt::KeypadModifier) && !xcb_is_keypad_key(g_rgQtToSymX[i].keySymX))
> +                continue;

xcb_is_keypad_key is not part of any xcb component KWindowSystem looks for.

> kkeyserver.cpp:783
> +     int keyModQt = 0;
> +     if (symXToKeyQt(keySym, keyQt) && modXToQt(modX, &keyModQt)) {
> +         *keyQt |= keyModQt;

why are you calling a deprecated method from a new method?

> kkeyserver_x11.h:150
>   */
> -KWINDOWSYSTEM_EXPORT bool symXToKeyQt(uint sym, int *keyQt);
> +KWINDOWSYSTEM_DEPRECATED_EXPORT bool symXToKeyQt(uint sym, int *keyQt);
> +

if it's getting deprecated it must be wrapped in ifdef, shouldn't it?

> kkeyserver_x11.h:159
> + */
> +KWINDOWSYSTEM_EXPORT bool symXModXToKeyQt(uint keySym, uint modX, int *keyQt);
>  

for new code I would find it better to use the proper types of either uint32_t or quint32.

REPOSITORY
  R278 KWindowSystem

REVISION DETAIL
  https://phabricator.kde.org/D6233

To: dfaure, graesslin
Cc: graesslin, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170616/badd95bb/attachment.html>