This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/

Almost there :-)

Re your earlier comments: - trashimpl.h is completely internal to the kioslave, adding methods there is no problem - trashForMountPoint is called when trashing a file that is on a different partition than HOME. I can see it being called when I do this, on Linux:

cd /tmp
touch foo
export KDE_FORK_SLAVES=1
kde-cp foo trash:/

The debug output from kio_trash (which appears in the same terminal due to the use of KDE_FORK_SLAVES=1) says TrashImpl::findTrashDirectory: mountPoint= "/" trashDir= "" which is one line below the call to trashForMountPoint, and prints out its result. So it's called for sure :-)

kioslave/trash/kcmtrash.cpp (Diff revision 8)
void TrashConfigModule::writeConfig()
		219	#ifdef Q_OS_OSX

you're right, this didn't exist in Qt4. Please revert to Q_OS_MAC then :-)

kioslave/trash/trashimpl.h (Diff revision 8)
private:
		163	// create the trash infrastructure (called from TrashImpl::init); also called

the comment is wrong, this is called on-demand all over the place. Remove the (called from...) part.

kioslave/trash/trashimpl.cpp (Diff revision 8)
		149	if ( trashPath.endsWith(QString::fromLatin1("/KDE.trash")) ) {

Is this check still needed? We know that every entry in m_trashDirectories ends with KDE.trash, now, right?

Or is this "everything but the trash in the home dir"? then better test for that explicitely.

endsWith sounds fragile to me, like the code doesn't really know what it should be doing, and could get fooled by unexpected naming..

kioslave/trash/trashimpl.cpp (Diff revision 8)
		159	QString trashDir = (path)? *path : trashDirectoryPath(trashId);

Looks like path is an optional value, not a modifyable parameter... then don't use a pointer, use an empty string as the default value.
[...], const QString &path = QString() );
in the header.

kioslave/trash/trashimpl.cpp (Diff revision 8)
152	int err;	194	if (!createTrashInfraStructure(0, (QString*)&trashDir)) {

Eek, a C cast. My eyes bleed. Anyhow you can remove completely, with my above suggestion.

kioslave/trash/trashimpl.cpp (Diff revision 8)
		201	DIR *dp = opendir( QFile::encodeName(trashDir) );

Why not QFileInfo(trashDir).isDir(), to use Qt rather than unportable code?

- David Faure

On October 16th, 2014, 10:35 p.m. UTC, René J.V. Bertin wrote:

Review request for KDE Software on Mac OS X, KDE Runtime and David Faure.

By René J.V. Bertin.

Updated Oct. 16, 2014, 10:35 p.m.

Repository: kde-runtime

Description

KDE on OS X does not handle the desktop session (no "Plasma") nor can it rely on XDG to obtain the proper paths to use for something like the trash. As a result, all applications that propose to move things they manage to the wastebin (Dolphin, but also digiKam) will store those items in a place that has no particular meaning on OS X, and that will thus tend to fill up.

OS X stores trash in one of several locations. Files trashed from the boot volume (and/or the volume containing $HOME, I don't actually know that) end up in ~/.Trash. Files deleted from other volumes end up in /Volumes/volName/.Trashes/uid, where volName is the volume name (regardless whether it's an external or a remote drive; only mounted NFS shares are handled differently) and uid the numerical user id. Permissions on .Trashes are the same as those expected by KDE.

The kio_trash kioslave appears to support several actual trash directory locations, just like OS X. TrashImpl::init() creates a standard trash in ~/.local/share/Trash (at least under OS X) but also TrashImpl::trashForMountPoint() that is used in cases I have not yet encountered.

On OS X, my modified TrashImpl::init() sets the standard trash directory to ~/.Trash/KDE.trash and will create the files and info subdirectories as required, because they will of course be deleted when the user empties the OS X trash. TrashImpl::fileRemoved() has been modifie d to call a new function, deleteEmptyTrashInfraStructure to delete the KDE trash's internal infrastructure when the wastebin is empty so that OS X also sees the trash as emptied. (Since implementing deleteEmptyTrashInfraStructure this feature actually works, as expected as far as I can tell).

Remains to be done: - determine in what cases trashForMountPoint() is used, and finish the modifications for it to use /.Trashes/uid/KDE.trash

Testing

On OS X 10.6.8 with kdelibs and kde-runtime git/4.14, using Dolphin. Tested actions are - move items to wastebin from $HOME and a directory on a different volume - restore items to both places - empty wastebin through Dolphin - empty OS X trashcan

Diffs

kioslave/trash/kcmtrash.cpp (f4811fd)
kioslave/trash/trashimpl.h (bc68723)
kioslave/trash/trashimpl.cpp (30ee05b)

View Diff