kcm-touchpad in plasma 5.2?

[David Edmundson]

On Sat, Jan 3, 2015 at 7:07 PM, Rajeesh K Nambiar <rajeeshknambiar at gmail.com
> wrote:

> On Thu, Dec 11, 2014 at 4:30 PM, David Edmundson
> <david at davidedmundson.co.uk> wrote:
> > In general this looks OK, there's some useful features and I can see
> myself
> > using this. I'd very much like it to become part of Plasma.
> >
> > I gave it a review and made some notes below.
>
> Thanks for the review. I cannot answer many of the comments, but some
> queries below:
>
> >
> > kded.cpp
> > The touchpad backend leaks?
>
> TouchpadBackend::implementation has static variable, would this still
> cause leak? (especially if there are multiple backends...)
>
> >
> > There are blocking calls in the constructor using isServiceRegistered
> > combined with the dataengine querying this kded module in a blocking way
> in
> > init we have a strong potential to deadlock the two applications
> > For KDED modules we have to be a lot more strict to never block as others
> > will be querying us.
>
> I couldn't find a way to work around this as there's no async
> alternative to isServiceRegistered. Could Alexander help?
>

Ah, it's a bit obscure.

    QDBusPendingCall async =
QDBusConnection::sessionBus().interface()->asyncCall(QLatin1String("ListNames"));
    QDBusPendingCallWatcher *callWatcher = new
QDBusPendingCallWatcher(async, this);
    connect(callWatcher, SIGNAL(finished(QDBusPendingCallWatcher*)),
            this, SLOT(serviceNameFetchFinished(QDBusPendingCallWatcher*)));


void serviceNameFetchFinished(QDBusPendingCallWatcher *callWatcher)
{
    QDBusPendingReply<QStringList> reply = *callWatcher;
    callWatcher->deleteLater();

    if (reply.isError()) {
        qDebug() << "omg wtf bbq";
        return;
    }

    QStringList allServices = reply.value();
    if (allService.contains("MYSERVICE")) {
      //do stuff
   }
}



> >
> > I don't understand why we're watching for plasmashell/kglobalaccel
> anyway.
> > Could you explain the rationale here.
> >
> > applet:
> > The applet is completely not ported.
> >
> > Applet translations need to go into a differnet .po file with a specific
> > name
>
> Top level Messages.sh does put .{h,cpp} file translations to
> kcm_touchpad.pot and {qml,js} file translations to
> plasma_applet_touchpad.pot - could you be more specific if this needs
> to change?
>
> > Copy a Messages.sh from one of the other plasmoids
> >
> > KCM:
> > reverse scrolling doesn't seem to work
> > "disabled taps and scrolling only" -- The HIG says to avoid negated
> checkbox
> > descriptions.
>
> But this makes sense to leave it as it is - as users would want to
> 'disable' tap+scrolling alone.
>
> >
> > The translation_domain doesn't seem to be set, so kded/kcm won't load
> > anything
> >
> > touchpad backend.h:
> > This class shouldn't be instantiated directly, don't make the constructor
> > public.
> >
> > The X backend:
> > This looked scary so I didn't review it at all.
>
>
>
> --
> Rajeesh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150103/bebb0bd4/attachment.html>