D17816: Support for xattrs on kio copy/move

[David Faure]

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

INLINE COMMENTS

> jobtest.cpp:118
> +            } else {
> +                qWarning() << "Xatr command not foud.";
> +            }

typo in Xatr, and in "foud". I suggest:

  qWarning() << "Neither getfattr, getextattr nor xattr was found";

Although I have to wonder why this looks for all three, to then only use getfattr and skip tests in all other cases....

> jobtest.cpp:480
> +    attrs["user.name with space"] = "value with spaces";
> +	return attrs;
> +}

this method has weird indentation

> jobtest.cpp:736
> +    }
> +    setXattr(filePath);
>      copyLocalFile(filePath, dest);

I fail to understand the code here.
This is the same call as two lines above, line 734, which is inside an if().
This one should be removed?

> jobtest.cpp:793
>      copyLocalSymlink(filePath, dest, QStringLiteral("relative"));
> +    
>      QFile::remove(filePath);

please revert no-op changes

> jobtest.cpp:805
>      const QString dest = otherTmpDir() + "testlink_copied";
> +    
>      createTestSymlink(filePath, QFile::encodeName(homeTmpDir()));

no-op (just empty lines)

> jobtest.cpp:808
>      copyLocalSymlink(filePath, dest, homeTmpDir());
> +    
>      QFile::remove(filePath);

empty line

> file_unix.cpp:164
> +#endif
> +    QList<QByteArray> m_keyList = keylist.split('\0');
> +    if (m_keyList.last().isEmpty()) m_keyList.removeLast(); // the last item may be empty

Can this *ever* return an empty list, because keylist was empty?

(Then last() will assert on the next line)

REPOSITORY
  R241 KIO

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

To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta
Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200317/e7667f4f/attachment-0001.html>