> On Oct. 13, 2014, 8:16 p.m., 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 ;-)
And there I was under the impression that I *am* hacking into (an)other('s) lib
O:-)
- René J.V.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120573/#review68318
-----------------------------------------------------------
On Oct. 13, 2014, 7:32 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120573/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2014, 7:32 p.m.)
>
>
> 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
>
>
