D9104: Properly do strings in the kpackage framework

Martin Flöser noreply at phabricator.kde.org
Sat Dec 2 12:04:34 UTC 2017


graesslin added a comment.


  There are a few unrelated changes, but otherwise looks good.

INLINE COMMENTS

> package.cpp:360
>      //Nested loop, but in the medium case resolves to just one iteration
> -    //qDebug() << "prefixes:" << prefixes.count() << prefixes;
> +//     qDebug() << "prefixes:" << d->contentsPrefixPaths.count() << d->contentsPrefixPaths;
>      foreach (const QString &contentsPrefix, d->contentsPrefixPaths) {

?

> package.cpp:594
> +            if (!it.value().endsWith(QLatin1Char('/'))) {
> +                it.setValue(it.value() % QLatin1Char('/'));
>              }

Just curious: what does the modulo operator on a string?

> package.cpp:946
>  
> -    if (isDir && QFile::exists(path + "/metadata.json")) {
> -        metadata = new KPluginMetaData(path + "/metadata.json");
> -    } else if (isDir && QFile::exists(path + "/metadata.desktop")) {
> -        auto md = KPluginMetaData::fromDesktopFile(path + "/metadata.desktop", {QStringLiteral(":/kservicetypes5/kpackage-generic.desktop")});
> +    delete metadata;
> +    if (isDir && QFile::exists(path + QStringLiteral("/metadata.json"))) {

This looks like a unrelated move of the line.

> kpackagetool.cpp:437
>      // This can happen in the case an application wanting to support kpackage based extensions includes in the same project both the packagestructure plugin and the packages themselves. In that case at build time the packagestructure plugin wouldn't be installed yet
> +
>      if (QFile::exists(pluginName + QStringLiteral("/metadata.desktop"))) {

unrelated?

REPOSITORY
  R290 KPackage

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

To: apol, #frameworks, #plasma, davidedmundson
Cc: graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171202/8e184b16/attachment.html>


More information about the Plasma-devel mailing list