D12807: Akregator feed detector plugin: Port the context menu plugin

[David Faure]

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


  Nice

INLINE COMMENTS

> akregatorplugin.cpp:84
>  
> -AkregatorMenu::~AkregatorMenu()
> -{
> -    KLocale::global()->removeCatalog("akregator_konqplugin");
> -    //delete m_conf;
> +    return (acts);
>  }

coding style: "return acts", no parenthesis needed

> akregatorplugin.cpp:99
>      }
> -    if (url.contains("xml", false) != 0) {
> +    if (url.contains("xml", Qt::CaseInsensitive)) {
>          return true;

This reads like it would trigger false positives easily.
E.g. https://www.w3schools.com/xml/xml_whatis.asp would be treated as a feed URL because it contains xml?

I wonder if this code meant to say endsWith...

> akregatorplugin.h:40
> +
> +    QList<QAction *> actions(const KFileItemListProperties &fileItemInfos, QWidget *parent);
>  

override ?

> akregatorplugin.h:45
>  
>  protected:
>      bool isFeedUrl(const QString &s);

white at it: private

> akregatorplugin.h:46
>  protected:
>      bool isFeedUrl(const QString &s);
>      bool isFeedUrl(const KFileItem &item);

while at it: const, or static, or better, make it file-static

> akregatorplugin.h:47
>      bool isFeedUrl(const QString &s);
>      bool isFeedUrl(const KFileItem &item);
>  

const

REPOSITORY
  R226 Konqueror

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

To: marten, #konqueror, #plasma, dfaure
Cc: dfaure
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180510/4619924f/attachment.htm>