D9167: Improve string handling

[Dominik Haumann]

dhaumann added a comment.


  Looks already pretty just. I just stumbled over two issues which maybe should be clarified - see comments.

INLINE COMMENTS

> kiconeffect.cpp:163
> +
> +        if (state == Colorize || effectGroupState == ToMonochrome) {
> +            cached += QLatin1Char(':') + d->color[group][state].name();

This should be:
 if (effectGroupState == Colorize || effectGroupState == ToMonochrome) {

or do I miss something?

> kiconeffect.cpp:165
> +            cached += QLatin1Char(':') + d->color[group][state].name();
> +        } else if (effectGroupState == ToMonochrome) {
> +            cached += QLatin1Char(':') + d->color2[group][state].name();

Previously, this was not an else if(), but just an if().

Are you sure this works as intended?

REPOSITORY
  R302 KIconThemes

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

To: apol, #frameworks
Cc: dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171204/0f6bb592/attachment.html>