D6183: [Digital Clock] Allow copying current date and time to clipboard

[David Edmundson]

davidedmundson added a comment.


  It's a bit of a shame that we're mixing two data sources in the same applet, but I guess we're limited by whatever works.

INLINE COMMENTS

> clipboardmenu.cpp:83
> +        s = time.toString(Qt::SystemLocaleLongDate);
> +        s.replace(rx, "");
> +        a = menu->addAction(s);

you're removing everything that isn't ':' or a number ? Why?

Does every locale use : as a separator?

> clipboardmenu.cpp:116
> +
> +        QMenu *menu_cal = new QMenu;
> +        menu_cal->clear();

lets not put POC in code that we want to merge.

Also, by your own comment later, this leaks.

> clipboardmenu.cpp:143
> +    // QMenu cannot have QAction as parent and setMenu doesn't take ownership
> +    connect(action, &QObject::destroyed, this, [menu] {
> +        menu->deleteLater();

We want "menu" not  "this" as the second context.

Otherwise if you delete the clipboardmenu class first, this leaks
If you delete the menu first, this crashes.

Neither should happen, but better to be safer.

REPOSITORY
  R120 Plasma Workspace

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

To: bschiffner, #plasma, broulik
Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170612/b03f79ff/attachment.html>