> On Okt. 13, 2014, 6:16 nachm., Milian Wolff wrote: > > kioslave/trash/trashimpl.cpp, line 812 > > <https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line812> > > > > why do you try to *delete* something in the check that should figure > > out if something is empty? that sounds very wrong to me. > > René J.V. Bertin wrote: > The goal is explained in the comment: delete an empty directory > infrastructure that no longer has a need to exist because the wastebin is > empty, in order to empty the hosting trashcan. > Yes, it gives a side-effect to the function, but one very much related to > its original function - one could say that for isEmpty to return true, said > infrastructure should have been deleted. > > I have no objection to adding an additional function that handles this > task, but it would have to be called from several locations (after isEmpty > returned true), and also loop over all known trashes. Adding a few lines to > isEmpty adds less overhead, and with proper comment doesn't increase > maintainance cost, IMHO. > > Thomas Lübking wrote: > You want to add this to "::fileRemoved()" i'd say (cause this is when the > state will change), eventually on ::init() - in case that's required to catch > trash emptying via safari. > > Random side effects on a state queries are really bad style, so please > don't do such (unless required, ie. you're hacking into other libs ;-) > > René J.V. Bertin wrote: > And there I was under the impression that I *am* hacking into > (an)other('s) lib O:-)
Nah... that's not *my* definition of hacking :-P You'd do so when eg. bringing a plugin and want to sneak your way into the process by abusing a function that will be called for sure to cause sth. entirely different (because there's no other way to impact the binary behavior) - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68318 ----------------------------------------------------------- On Okt. 13, 2014, 5:32 nachm., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120573/ > ----------------------------------------------------------- > > (Updated Okt. 13, 2014, 5:32 nachm.) > > > Review request for KDE Software on Mac OS X and KDE Runtime. > > > 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::isEmpty()` has been modified to delete the KDE trash's > internal infrastructure when the wastebin is empty so that OS X also see the > trash as emptied, but that doesn't work yet. > > Remains to be done: > - figure out why `isEmpty()` doesn't completely clean out the trash as > expected which causes OS X to show the trash as full even after emptying the > wastebin through KDE. > - determine in what cases `trashForMountPoint()` is used, and finish the > modifications for it to use `/.Trashes/uid/KDE.trash` > > > Diffs > ----- > > kioslave/trash/trashimpl.cpp 30ee05b > > Diff: https://git.reviewboard.kde.org/r/120573/diff/ > > > 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 > > > Thanks, > > René J.V. Bertin > >
