D26565: KCM/Component Revamp email config

Kevin Ottens noreply at phabricator.kde.org
Mon Jan 20 12:50:27 GMT 2020


ervin added inline comments.

INLINE COMMENTS

> componentchooseremail.cpp:67
>  }
> +static const char s_AddedAssociations[] = "Added Associations";
> +static const auto s_mimetype = QStringLiteral("x-scheme-handler/mailto");

Not a huge fan of static consts appearing like this in the middle of the file. Could you please move them up before the ctor? Bonus point for putting them in the anonymous namespace.

> componentchooseremail.cpp:106
> +        }
> +        if (service &&
> +                // avoid duplicates entry when email clients are present in mimeapps.list's Added Associations too

To simplify reading this I'd go for:

  if (!service) {
      continue;
  }

and then I'd put the result of none_of in a properly named intermediate variable before checking it in its own if. Will make the intents clearer.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  email-config

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

To: meven, ngraham, ervin, #plasma, bport, crossi, dvratil
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200120/f68af4b6/attachment.html>


More information about the Plasma-devel mailing list