ivan requested changes to this revision.
ivan added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> recentlyused.cpp:140-144
> +        if (agentValue.contains(QLatin1Char(','))) {
> +            query = query | Agent(agentValue.split(QLatin1Char(',')));
> +        } else {
> +            query = query | Agent(agentValue);
> +        }

Could this just be replaced with `query = query | 
Agent(agentValue.split(QLatin1Char(',')));`?

Obviously, the downside is always creating a list (usually with a single item 
in it), the upside is that it does not need to traverse the string twice (once 
to chech for commas, and the second time to split the string.

The trade-off depends on internals of Qt - if  you don't have the time to 
benchmark what is faster, leave it as is.

> recentlyused.cpp:250
> +    if (isRootUrl(url)) {
> +        mimeType(QString::fromLatin1("inode/directory"));
> +        finished();

`QStringLiteral`?

REPOSITORY
  R320 KIO Extras

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

To: meven, ivan, #frameworks, ngraham
Cc: elvisangelaccio, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

Reply via email to