D6513: Add support for Attica tags support

[Christoph Feck]

cfeck added inline comments.

INLINE COMMENTS

> atticaprovider.cpp:282
> +                for (const Attica::DownloadDescription &dli : content.downloadUrlDescriptions()) {
> +                    if(downloadschecker.filterAccepts(dli.tags())) {
> +                        filterAcceptsDownloads = true;

Missing Space after `if`

> atticaprovider.cpp:296
> +        }
> +        else {
> +            qCDebug(KNEWSTUFFCORE) << "Filter has excluded" << content.name() << "on entry filter" << mCurrentRequest.tagFilter;

Coding style:

  if (...) {
      ...
  } else {
      ...
  }

> engine.h:226
> +     * @see setDownloadTagFilter(QStringList)
> +     * @since 5.50
> +     */

-> `5.51` (everywhere)

> staticxmlprovider.cpp:266
>              }
> +            else {
> +                qCDebug(KNEWSTUFFCORE) << "Filter has excluded" << entry.name() << "on download filter" << mCurrentRequest.downloadTagFilter;

again

> staticxmlprovider.cpp:270
> +        }
> +        else {
> +            qCDebug(KNEWSTUFFCORE) << "Filter has excluded" << entry.name() << "on entry filter" << mCurrentRequest.tagFilter;

again

> khotnewstuff_test.cpp:42
> +#include <unistd.h> // for exit()
> +#include <stdio.h> // for stdout
> +

<cstdio>

> khotnewstuff_test.cpp:49
> +    m_configFile = configFile;
> +    m_engine = NULL;
> +    m_testall = false;

nullptr

> khotnewstuff_test.cpp:66
> +{
> +    addMessage(QString::fromLocal8Bit("-- test kns2 entry class"), QStringLiteral("msg_info"));
> +

Any rationale for using `fromLocal8Bit()` for fixed strings? If, for whatever reason, you do not want to use QStringLiteral or QLatin1String, please use fromUtf8(). This is what we ship for source files.

> khotnewstuff_test.cpp:190
> +
> +void KNewStuff2Test::slotEngineError(const QString& error)
> +{

`& ` -> ` &`

> khotnewstuff_test.cpp:202
> +{
> +    QStandardItem* item = new QStandardItem(message);
> +    item->setData(iconName, Qt::WhatsThisRole);

`* ` -> ` *`

> khotnewstuff_test.cpp:211
> +{
> +    if(test) {
> +        test->addMessage(msg, QStringLiteral("msg_info"));

Missing space

> khotnewstuff_test.cpp:222
> +
> +    QCommandLineParser* parser = new QCommandLineParser;
> +    parser->addHelpOption();

`* ` -> ` *`

> khotnewstuff_test.cpp:231
> +    }
> +    else {
> +        test = new KNewStuff2Test(QString::fromLatin1("%1/khotnewstuff_test.knsrc").arg(QStringLiteral(KNSBUILDDIR)));

again

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, #kde_store, whiting, ahiemstra, mlaurent, dfaure, cfeck
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/20180904/09ff3a5c/attachment-0001.html>