D21661: add snoretoast backend for KNotifications on Windows

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> notifybysnore.cpp:115
> +{
> +    if (m_notifications.find(notification->id()) != m_notifications.end()) {
> +            qCDebug(LOG_KNOTIFICATIONS) << "Duplicate notification with ID: " << notification->id() << " ignored.";

since this method is not const, prefer using constFind + constEnd for the hash lookup

> notifybysnore.cpp:119-121
> +        if(notification->id() == -1){
> +            qCDebug(LOG_KNOTIFICATIONS) << "Notification with ID : \"-1\" ignored.";
> +        }

none of the other plugins seem to check for id == -1 here, so I'd say this check can be dropped

> notifybysnore.cpp:132
> +    arguments << QStringLiteral("-m") << notification->text();
> +    arguments << QStringLiteral("-p") <<  file.fileName();
> +    arguments << QStringLiteral("-appID") << app->applicationName();

if the notification has no pixmap (see `!notification->pixmap().isNull()` check above), then I guess you do not need to pass this argument, as the image will not exist

> notifybysnore.cpp:161-163
> +    if (it.value()) {
> +        finish(it.value());
> +    }

`it` is used after erasing it from `m_notifications`, and that means that iterator points to nothing; you cannot use iterators after you remove them from their container

you already have `notification`, so just use it instead

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190608/e94c5544/attachment.html>