dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Thanks for the fix!
INLINE COMMENTS
> karchive.cpp:471
> } else {
> - //qCWarning(KArchiveLog) << "Found" << path << "but it's not a
> directory";
> + qCWarning(KArchiveLog) << "Found" << path << "but it's not a
> directory";
> + if (ent->isFile()) {
Please remove this warning, it's ambiguous at this point what the problem is;
better have only one clear warning once we know which case we're in.
> karchive.cpp:472
> + qCWarning(KArchiveLog) << "Found" << path << "but it's not a
> directory";
> + if (ent->isFile()) {
> + const KArchiveFile *file = static_cast<const KArchiveFile
> *>(ent);
You can remove this if(), every entry is either a file or a directory, by
design.
And the if() confused me, e.g. if it was false, the code below would still talk
about an empty file ;). But it can't ever be false, so let's just remove that
if().
> karchive.cpp:475
> + if (file->size() > 0) {
> + qCWarning(KArchiveLog) << "It's a normal file, bailing
> out";
> + return nullptr;
Obviously that means mentioning "path" in this warning, something about that
file being a duplicate.
> karchive.cpp:480
> +
> + qCWarning(KArchiveLog) << "It's an empty file, assuming it is
> actually a directory and replacing";
> + KArchiveEntry *myEntry = const_cast<KArchiveEntry*>(ent);
If this can happen, since there's no data loss, maybe a qCDebug is enough?
(this also needs to mention "path" of course)
> kzip.cpp:756
> + } else {
> + setErrorString(tr("File %1 is in folder %2, but %3
> is actually a file.").arg(entryName).arg(path).arg(path));
> + delete entry;
Use multi-arg to avoid issues in case one of these strings contains %1.
.arg(entryName, path, path)
REPOSITORY
R243 KArchive
REVISION DETAIL
https://phabricator.kde.org/D10543
To: sandsmark, dfaure, #frameworks
Cc: apol, michaelh
