D9104: Properly do strings in the kpackage framework
Martin Flöser
noreply at phabricator.kde.org
Sat Dec 2 12:04:31 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/kde-frameworks-devel/attachments/20171202/35a3e937/attachment.html>
More information about the Kde-frameworks-devel
mailing list