D6233: KKeyServer: fix handling of KeypadModifier.

David Faure noreply at phabricator.kde.org
Sat Jun 17 14:42:01 UTC 2017


dfaure added inline comments.

INLINE COMMENTS

> graesslin wrote in kkeyserver.cpp:160-180
> This looks very unrelated to the described change. Maybe an own commit?

Well those are the XK_KP_* codes, i.e. Num Keypad keys, so it's related. But yeah, it would probably work without this change, it just seems best to update the full list from Qt.

Do you insist on a separate commit?

> graesslin wrote in kkeyserver.cpp:765
> xcb_is_keypad_key is not part of any xcb component KWindowSystem looks for.

Oh. OK, then I'll go back to >= and <=, it's just as simple anyway.

> graesslin wrote in kkeyserver.cpp:783
> why are you calling a deprecated method from a new method?

it was simpler, but ok, I'll refactor ;)

> graesslin wrote in kkeyserver_x11.h:150
> if it's getting deprecated it must be wrapped in ifdef, shouldn't it?

now that it's not called by the new method, it's possible indeed ;)
Done.

> graesslin wrote in kkeyserver_x11.h:159
> for new code I would find it better to use the proper types of either uint32_t or quint32.

Not very symmetrical with keyQtToSymX, but whatever makes you happy.

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/20170617/0a87ebb7/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list