D6513: Add support for Attica tags support

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  @since 5.50 is missing on any new *public* API (methods or classes).
  
  (This won't get into 5.49 which I just tagged)

INLINE COMMENTS

> engine.cpp:132
> +    d->tagFilter = group.readEntry("TagFilter", QStringList());
> +    if(d->tagFilter.length() == 0) {
> +        d->tagFilter[0] = QStringLiteral("ghns_exclude!=1");

use isEmpty()

> engine.cpp:133
> +    if(d->tagFilter.length() == 0) {
> +        d->tagFilter[0] = QStringLiteral("ghns_exclude!=1");
> +    }

That's out of bounds, if tagFilter is empty!! It will assert.

You meant append or push_back, I think.

> engine.cpp:137
> +    d->downloadTagFilter = group.readEntry("DownloadTagFilter", QStringList());
> +    if(d->downloadTagFilter.length() == 0) {
> +        d->downloadTagFilter.clear();

isEmpty()

> engine.cpp:138
> +    if(d->downloadTagFilter.length() == 0) {
> +        d->downloadTagFilter.clear();
> +    }

!? What's the point in clearing a list that is already empty?

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, #kde_store, whiting, ahiemstra, mlaurent, dfaure
Cc: dfaure, cfeck, mlaurent, ngraham, ahiemstra, kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180804/30c5eff1/attachment-0001.html>