D26801: Really fix the Windows backend for KNotifications

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> notifybysnore.cpp:89
> +        KNotification *notification = m_notifications.value(id);
> +        if (notification == nullptr) {
>              qCDebug(LOG_KNOTIFICATIONS) << "Notification not found!";

`if (!notification) {`

> notifybysnore.cpp:147
>  {
> -    Q_UNUSED(config);
> -    // HACK work around that notification->id() is only populated after returning from here
> -    // note that config will be invalid at that point, so we can't pass that along
> -    QMetaObject::invokeMethod(this, [this, notification](){ notifyDeferred(notification); }, Qt::QueuedConnection);
> -}
> +    if(notification->id() == -1 && notification->eventId() == QStringLiteral("notification")) {
> +            return;

Why this random `eventId` check?
Also, compare with `QLatin1String`

> notifybysnore.cpp:200
>  
>  void NotifyBySnore::close(KNotification *notification)
>  {

Does the notification still end up with an `id` eventually?

> notifybysnore.cpp:202
>  {
> -    if (m_notifications.constFind(notification->id()) == m_notifications.constEnd()) {
> +    if (m_notifications.key(notification) == -1) {
> +        qCWarning(LOG_KNOTIFICATIONS) << "Notification was already closed.";

Where do you get this `-1`?

> If the hash contains no item with the value, the function returns a default-constructed key.

You'll get `0` back unless you specify a `defaultValue`.

Also cache this as you do another `key` look up again below.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, vonreth, broulik, #kde_connect
Cc: anthonyfieroni, kde-frameworks-devel, nalvarez, KunalRaghav, ankitbaluni, ankit, aliencode, Orage, ritwizsinha, LeGast00n, ewentzel, dshelley, pawelkwiecinski, ctakano, vporvaznik, mschroeder, varunp, shivanshukantprasad, skymoore, fbampaloukas, GB_2, brute4s99, wistak, anoopv, dvalencia, rmenezes, julioc, Leptopoda, timothyc, Danial0_0, johnq, Pitel, domson, adeen-s, michaelh, SemperPeritus, daniel.z.tg, jeanv, seebauer, ngraham, bruns, bugzy, MayeulC, lemuel, menasshock, mikesomov, tctara, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20200123/538e7dd4/attachment-0001.html>