subdiff added inline comments.
INLINE COMMENTS
> x11mousebackend.cpp:26
> +#include <evdev-properties.h>
> +#include <libinput-properties.h>
> +#include <KLocalizedString>
This include is not needed.
> x11mousebackend.cpp:42
> +static const char PROFILE_ADAPTIVE[] = I18N_NOOP("Adaptive");
> +static const char PROFILE_FLAT[] = I18N_NOOP("Flat");
> +
These 3 are never used.
> x11mousebackend.cpp:50
> +template<typename Callback>
> +static void XI2ForallPointerDevices(Display *dpy, const Callback &callback) {
> + int ndevices_return;
Never used.
> x11mousebackend.cpp:139
> + // get settings from X server
> + unsigned char map[20];
> + m_numButtons = XGetPointerMapping(m_dpy, map, 20);
Below you do it with 256 for reasons explained there.
> x11mousebackend.cpp:168
> + unsigned char map[256];
> + m_numButtons = XGetPointerMapping(m_dpy, map, 256);
> +
You set m_numButtons here and in load. This creates some ambiguity. Shouldn't
be it enough to set it in the load() function?
> x11mousebackend.cpp:170
> +
> + int remap=(m_numButtons>=1);
> + if (settings.handedEnabled && (settings.m_handedNeedsApply || force)) {
nitpick: Spaces
> x11mousebackend.cpp:206
> + int retval;
> + if (remap) {
> + while ((retval=XSetPointerMapping(m_dpy, map,
remap only used here -> Don't use local variable. Just check the condition here.
> x11mousebackend.cpp:207
> + if (remap) {
> + while ((retval=XSetPointerMapping(m_dpy, map,
> + m_numButtons)) == MappingBusy)
Spaces
> x11mousebackend.cpp:215
> + // are belong to kcm touchpad.
> + XIForallPointerDevices(m_dpy, [&] (XDeviceInfo *info) {
> + int deviceid = info->id;
Capture "this" pointer and settings reference directly.
> main.cpp:71
> QDBusConnection::sessionBus());
> if(!theme.isEmpty())
> klauncher.setLaunchEnv(QStringLiteral("XCURSOR_THEME"), theme);
I know it's copy paste. But so we can directly clean it up: Coding style ->
always use braces.
> mousebackend.h:26
> +
> +enum class MouseHanded {
> + Right = 0,
Put this enum class in a separate file, such that you don't have to include the
full mousebackend.h in other header files like mouse.h
> mousesettings.cpp:96
> + kcminputGroup.writeEntry("Threshold",thresholdMove);
> + if (handed == MouseHanded::Right)
> +
> kcminputGroup.writeEntry("MouseButtonMapping",QString("RightHanded"));
coding style
> mousesettings.h:27
> +
> +class MouseSettings
> +{
class -> struct
And remove the public keywords.
> mousesettings.h:36
> + bool handedEnabled;
> + bool m_handedNeedsApply;
> + MouseHanded handed;
-> handedNeedsApply
like the others.
> mousesettings.h:46
> + bool reverseScrollPolarity;
> + QString m_currentAccelProfile;
> +};
-> currentAccelProfile
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D8460
To: xuetianweng, subdiff, davidedmundson, ngraham
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart
