Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 

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

(Updated Nov. 21, 2015, 6:57 p.m.)


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


Changes
---

Resolves David's last open issue.

I looked at removing the ifdefs around `createInfrastructure()`, but that 
function cannot be a stub elsewhere but on OS X. So one would either have to 
add an argument with a platform-dependent default value, or else accept that 
the infrastructure is checked each time the function is called (even when it's 
unlikely to be discarded "behind our backs).

Since the topic came up, I've also taken the liberty to convert certain 
qDebug() calls to qWarning(), and comment most of the remaining ones. It's only 
on OS X that that output goes into a system log (I hope), and I think this 
workaround still leaves plenty to discuss on a future RR ;)


Repository: kio


Description
---

This is a "backport" of the [kde-runtime modification reviewed 
here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
use a subdirectory in the OS X trashcan.

>From the original review:
*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 patch from KDE4 was straightforward to apply as the trash implementation 
has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
Therefore, I have ported my code "as is", so as not to change anything that was 
discussed and negotiated during the rather lengthy previous review process. The 
only place where I've had to introduce a small additional change is in 
`trashForMountPoint`, where KF5 made `trashDir` a const. Appending the KDE 
trash-subdir in the `trashDir` declaration might seem the appropriate approach, 
but since there is no guarantee at that point that the subdirectory actually 
(still) exists, the `QT_LSTAT` test could fail, and the the appending has to be 
done after the check on the actual host trash directory.

I have also introduced a small change in `testtrash.cpp` allowing it to run 
instead of failing because the 1st location returned `trashDirectories()` 
doesn't match the one determined from QSP.


Diffs (updated)
-

  src/ioslaves/trash/CMakeLists.txt 05161cd 
  src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
  src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
  src/ioslaves/trash/tests/testtrash.cpp 339aa19 
  src/ioslaves/trash/trashimpl.h 9886011 
  src/ioslaves/trash/trashimpl.cpp 26d9ea8 

Diff: https://git.reviewboard.kde.org/r/126125/diff/


Testing
---

OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
unittests from testtrash succeed (with the Trash open in the Finder one sees 
the `KDE.trash` directory show up while the test runs, and disappear 
afterwards).


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 

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

(Updated Nov. 21, 2015, 7:57 p.m.)


Status
--

This change has been marked as submitted.


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


Repository: kio


Description
---

This is a "backport" of the [kde-runtime modification reviewed 
here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
use a subdirectory in the OS X trashcan.

>From the original review:
*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 patch from KDE4 was straightforward to apply as the trash implementation 
has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
Therefore, I have ported my code "as is", so as not to change anything that was 
discussed and negotiated during the rather lengthy previous review process. The 
only place where I've had to introduce a small additional change is in 
`trashForMountPoint`, where KF5 made `trashDir` a const. Appending the KDE 
trash-subdir in the `trashDir` declaration might seem the appropriate approach, 
but since there is no guarantee at that point that the subdirectory actually 
(still) exists, the `QT_LSTAT` test could fail, and the the appending has to be 
done after the check on the actual host trash directory.

I have also introduced a small change in `testtrash.cpp` allowing it to run 
instead of failing because the 1st location returned `trashDirectories()` 
doesn't match the one determined from QSP.


Diffs
-

  src/ioslaves/trash/CMakeLists.txt 05161cd 
  src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
  src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
  src/ioslaves/trash/tests/testtrash.cpp 339aa19 
  src/ioslaves/trash/trashimpl.h 9886011 
  src/ioslaves/trash/trashimpl.cpp 26d9ea8 

Diff: https://git.reviewboard.kde.org/r/126125/diff/


Testing
---

OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
unittests from testtrash succeed (with the Trash open in the Finder one sees 
the `KDE.trash` directory show up while the test runs, and disappear 
afterwards).


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread David Faure 

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


Right, I'm not sure that splitting code into a different file really works 
here. To avoid duplication, there would have to be a base class (interface) 
first, with only the platform-specific code in its two subclasses, all the 
common code staying where it is now. Not sure this is really applicable here 
though.

- David Faure


On Nov. 21, 2015, 12:50 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 12:50 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 2 p.m., David Faure wrote:
> > Right, I'm not sure that splitting code into a different file really works 
> > here. To avoid duplication, there would have to be a base class (interface) 
> > first, with only the platform-specific code in its two subclasses, all the 
> > common code staying where it is now. Not sure this is really applicable 
> > here though.

I was thinking that we could splice out really specific code like 
`idForMountPoint`, but I agree, the gain would really be minimal.


- René J.V.


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


On Nov. 21, 2015, 1:50 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 1:50 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 12:35 p.m., David Faure wrote:
> > src/ioslaves/trash/tests/testtrash.cpp, line 149
> > 
> >
> > Use .at(0) to avoid unnecessary detaching.
> 
> René J.V. Bertin wrote:
> I tried that first, but that fails to compile:
> ```
> kf5-kio/work/kio-5.16.0/src/ioslaves/trash/tests/testtrash.cpp:149:42: 
> error: no member named 'at' in 'QMap'
> m_trashDir = impl.trashDirectories().at(0);
>  ~~~ ^
> 6 warnings and 1 error generated.
> ```
> 
> David Faure wrote:
> Ah, it's a map, not a list, right. Use .value(0) then, to avoid creating 
> an empty item in case 0 isn't in the map. You could do this first of course: 
> QVERIFY(trashDirs.contains(0));
> m_trashDir = trashDirs.value(0);
> 
> (note that initTestCase already has a trashDirs local var which contains 
> the result of impl.trashDirectories())

Yeah, I had noted that. I'm not sure why I didn't decide to move its 
declaration upwards ...


- René J.V.


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


On Nov. 21, 2015, 1:50 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 1:50 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread David Faure 

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

Ship it!


Search/replace typo? Looks good after fixing that.


src/ioslaves/trash/trashimpl.cpp (line 121)


dDebug? did you mean qDebug?



src/ioslaves/trash/trashimpl.cpp (line 141)


seems to be contagious


- David Faure


On Nov. 21, 2015, 5:57 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 5:57 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 7:04 p.m., David Faure wrote:
> > Search/replace typo? Looks good after fixing that.

Dang, yes.

Evidently the compiler didn't catch that :)


- René J.V.


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


On Nov. 21, 2015, 6:57 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 6:57 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread David Faure 


> On Nov. 21, 2015, 11:35 a.m., David Faure wrote:
> > src/ioslaves/trash/tests/testtrash.cpp, line 149
> > 
> >
> > Use .at(0) to avoid unnecessary detaching.
> 
> René J.V. Bertin wrote:
> I tried that first, but that fails to compile:
> ```
> kf5-kio/work/kio-5.16.0/src/ioslaves/trash/tests/testtrash.cpp:149:42: 
> error: no member named 'at' in 'QMap'
> m_trashDir = impl.trashDirectories().at(0);
>  ~~~ ^
> 6 warnings and 1 error generated.
> ```

Ah, it's a map, not a list, right. Use .value(0) then, to avoid creating an 
empty item in case 0 isn't in the map. You could do this first of course: 
QVERIFY(trashDirs.contains(0));
m_trashDir = trashDirs.value(0);

(note that initTestCase already has a trashDirs local var which contains the 
result of impl.trashDirectories())


> On Nov. 21, 2015, 11:35 a.m., David Faure wrote:
> > src/ioslaves/trash/tests/CMakeLists.txt, line 34
> > 
> >
> > you can't commit that
> 
> René J.V. Bertin wrote:
> Oops, that one got through, sorry about that.
> 
> René J.V. Bertin wrote:
> BTW, you probably understood that is my (in)famous activator for the 
> ditto QSP patch that switches QSP to XDG-compliant mode.
> 
> I still have some, probably naïve, hope that someday something will be 
> integrated that will remove the need to maintain the build system patches and 
> check them at each KF5 FW version bump...

I still don't see why you can't just patch Qt on macports (given that you're 
already patching it anyway) rather than having to add something to each and 
every link line in KF5, but let's have that discussion separately on k-f-d.


- David


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


On Nov. 21, 2015, 12:50 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 21, 2015, 12:50 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 

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

(Updated Nov. 21, 2015, 7:23 p.m.)


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


Repository: kio


Description
---

This is a "backport" of the [kde-runtime modification reviewed 
here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
use a subdirectory in the OS X trashcan.

>From the original review:
*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 patch from KDE4 was straightforward to apply as the trash implementation 
has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
Therefore, I have ported my code "as is", so as not to change anything that was 
discussed and negotiated during the rather lengthy previous review process. The 
only place where I've had to introduce a small additional change is in 
`trashForMountPoint`, where KF5 made `trashDir` a const. Appending the KDE 
trash-subdir in the `trashDir` declaration might seem the appropriate approach, 
but since there is no guarantee at that point that the subdirectory actually 
(still) exists, the `QT_LSTAT` test could fail, and the the appending has to be 
done after the check on the actual host trash directory.

I have also introduced a small change in `testtrash.cpp` allowing it to run 
instead of failing because the 1st location returned `trashDirectories()` 
doesn't match the one determined from QSP.


Diffs (updated)
-

  src/ioslaves/trash/CMakeLists.txt 05161cd 
  src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
  src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
  src/ioslaves/trash/tests/testtrash.cpp 339aa19 
  src/ioslaves/trash/trashimpl.h 9886011 
  src/ioslaves/trash/trashimpl.cpp 26d9ea8 

Diff: https://git.reviewboard.kde.org/r/126125/diff/


Testing
---

OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
unittests from testtrash succeed (with the Trash open in the Finder one sees 
the `KDE.trash` directory show up while the test runs, and disappear 
afterwards).


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 12:35 p.m., David Faure wrote:
> > src/ioslaves/trash/tests/CMakeLists.txt, line 34
> > 
> >
> > you can't commit that
> 
> René J.V. Bertin wrote:
> Oops, that one got through, sorry about that.

BTW, you probably understood that is my (in)famous activator for the ditto QSP 
patch that switches QSP to XDG-compliant mode.

I still have some, probably naïve, hope that someday something will be 
integrated that will remove the need to maintain the build system patches and 
check them at each KF5 FW version bump...


- René J.V.


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 12:35 p.m., David Faure wrote:
> > src/ioslaves/trash/tests/CMakeLists.txt, line 34
> > 
> >
> > you can't commit that

Oops, that one got through, sorry about that.


> On Nov. 21, 2015, 12:35 p.m., David Faure wrote:
> > src/ioslaves/trash/tests/testtrash.cpp, line 149
> > 
> >
> > Use .at(0) to avoid unnecessary detaching.

I tried that first, but that fails to compile:
```
kf5-kio/work/kio-5.16.0/src/ioslaves/trash/tests/testtrash.cpp:149:42: error: 
no member named 'at' in 'QMap'
m_trashDir = impl.trashDirectories().at(0);
 ~~~ ^
6 warnings and 1 error generated.
```


> On Nov. 21, 2015, 12:35 p.m., David Faure wrote:
> > src/ioslaves/trash/trashimpl.cpp, line 182
> > 
> >
> > Any reason this uses Q_OS_MAC while the beginning of the patch uses 
> > Q_OS_OSX? That seems weird.

Just one that got through :-/


- René J.V.


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread David Faure 

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



src/ioslaves/trash/kcmtrash.cpp (line 227)


(here and everywhere else) please remove spaces inside parenthesis. 
Compared to the kde4 codebase, this code was fully reformatted by 
astyle-kdelibs.



src/ioslaves/trash/tests/CMakeLists.txt (line 34)


you can't commit that



src/ioslaves/trash/tests/testtrash.cpp (line 149)


Use .at(0) to avoid unnecessary detaching.



src/ioslaves/trash/trashimpl.cpp (line 154)


Add space before ?, and no need for () around path.isEmpty()



src/ioslaves/trash/trashimpl.cpp (line 182)


Any reason this uses Q_OS_MAC while the beginning of the patch uses 
Q_OS_OSX? That seems weird.



src/ioslaves/trash/trashimpl.cpp (line 1161)


could use += and QStringLiteral


- David Faure


On Nov. 20, 2015, 9:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 9:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 21, 2015, 3:05 a.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/trash/trashimpl.cpp, line 129
> > 
> >
> > Infrastructure is 1 word

I guess that depends on how far you want to decompose, but if you say so ... 
consider it done :)


On Nov. 21, 2015, 3:05 a.m., René J.V. Bertin wrote:
> > I'm wondering if ifdef'ing the shit out of the code is the best solution. 
> > Would it make sense to split it in a couple of files or get an `_apple.cpp` 
> > file? Or even get a completely different trash:/ kio implementation?

I didn't realise it was that bad? 

I think the reason why I never considered this (or that it didn't come up in 
the KDE4 RR) is that there is still much more common code than there is 
specific code. So splitting up is going to introduce much more code duplication 
than I think is wise to allow for. I know it's code that is unlikely to evolve 
a lot (at least I'd guess so), but duplication is about the opposite of the 
reasons to write shared libraries/frameworks in the first place, no?

Many of the ifdefs are only there to call a specific function. It would indeed 
be possible and trivial to move the specific functions to a separate file if 
there is a consensus that that would be beneficial. I could also provide shims 
for the 2 most frequently called added functions, basically moving most of the 
ifdefs into those 2 functions.

I suppose the MS Windows adaptations should be handled like that too, in that 
case :)


- René J.V.


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
>

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 


> On Nov. 20, 2015, 10:06 p.m., René J.V. Bertin wrote:
> > src/ioslaves/trash/trashimpl.cpp, line 947
> > 
> >
> > These were `kDebug()` statements; I guess they should become 
> > `qWarning()` now that `kDebug` is gone. I've left the qDebug() statements 
> > in because I noticed there are plenty of those in the code; shouldn't they 
> > be turned into `qCWarning()` or `qCDebug` to avoid polluting the terminal 
> > or (on OS X) the `system.log` ?
> 
> Aleix Pol Gonzalez wrote:
> Yes, they should, eventually.

Sooner rather than later as far as I'm concerned, at least those that are true 
debug statements and not warnings.

As I said, any output to a terminal that doesn't have one to be printed on will 
go to pollute the system log on OS X. For warnings that's - I actually prefer 
to have those output `unconditionally` rather than having to activate some Qt 
logging mechanism when something appears not to be running right to see if 
*maybe* there isn't a warning message that will show up then. But simple 
statements like "hey, we're going to move this file to the trash" don't belong 
in a system log.

Not in any kind of log unless the user asked for it, in fact ... privacy 
implications!


- René J.V.


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-21 Thread René J . V . Bertin 

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

(Updated Nov. 21, 2015, 1:50 p.m.)


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


Changes
---

Incorporated all feedback except the issues left open. I'll investigate 
splitting up and reducing the ifdeffing later.


Repository: kio


Description
---

This is a "backport" of the [kde-runtime modification reviewed 
here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
use a subdirectory in the OS X trashcan.

>From the original review:
*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 patch from KDE4 was straightforward to apply as the trash implementation 
has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
Therefore, I have ported my code "as is", so as not to change anything that was 
discussed and negotiated during the rather lengthy previous review process. The 
only place where I've had to introduce a small additional change is in 
`trashForMountPoint`, where KF5 made `trashDir` a const. Appending the KDE 
trash-subdir in the `trashDir` declaration might seem the appropriate approach, 
but since there is no guarantee at that point that the subdirectory actually 
(still) exists, the `QT_LSTAT` test could fail, and the the appending has to be 
done after the check on the actual host trash directory.

I have also introduced a small change in `testtrash.cpp` allowing it to run 
instead of failing because the 1st location returned `trashDirectories()` 
doesn't match the one determined from QSP.


Diffs (updated)
-

  src/ioslaves/trash/CMakeLists.txt 05161cd 
  src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
  src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
  src/ioslaves/trash/tests/testtrash.cpp 339aa19 
  src/ioslaves/trash/trashimpl.h 9886011 
  src/ioslaves/trash/trashimpl.cpp 26d9ea8 

Diff: https://git.reviewboard.kde.org/r/126125/diff/


Testing
---

OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
unittests from testtrash succeed (with the Trash open in the Finder one sees 
the `KDE.trash` directory show up while the test runs, and disappear 
afterwards).


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-20 Thread René J . V . Bertin 

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



src/ioslaves/trash/trashimpl.cpp (line 941)


These were `kDebug()` statements; I guess they should become `qWarning()` 
now that `kDebug` is gone. I've left the qDebug() statements in because I 
noticed there are plenty of those in the code; shouldn't they be turned into 
`qCWarning()` or `qCDebug` to avoid polluting the terminal or (on OS X) the 
`system.log` ?


- René J.V. Bertin


On Nov. 20, 2015, 10:03 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:03 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-20 Thread René J . V . Bertin 

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

(Updated Nov. 20, 2015, 10:09 p.m.)


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


Repository: kio


Description
---

This is a "backport" of the [kde-runtime modification reviewed 
here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
use a subdirectory in the OS X trashcan.

>From the original review:
*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 patch from KDE4 was straightforward to apply as the trash implementation 
has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
Therefore, I have ported my code "as is", so as not to change anything that was 
discussed and negotiated during the rather lengthy previous review process. The 
only place where I've had to introduce a small additional change is in 
`trashForMountPoint`, where KF5 made `trashDir` a const. Appending the KDE 
trash-subdir in the `trashDir` declaration might seem the appropriate approach, 
but since there is no guarantee at that point that the subdirectory actually 
(still) exists, the `QT_LSTAT` test could fail, and the the appending has to be 
done after the check on the actual host trash directory.

I have also introduced a small change in `testtrash.cpp` allowing it to run 
instead of failing because the 1st location returned `trashDirectories()` 
doesn't match the one determined from QSP.


Diffs
-

  src/ioslaves/trash/CMakeLists.txt 05161cd 
  src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
  src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
  src/ioslaves/trash/tests/testtrash.cpp 339aa19 
  src/ioslaves/trash/trashimpl.h 9886011 
  src/ioslaves/trash/trashimpl.cpp 26d9ea8 

Diff: https://git.reviewboard.kde.org/r/126125/diff/


Testing
---

OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
unittests from testtrash succeed (with the Trash open in the Finder one sees 
the `KDE.trash` directory show up while the test runs, and disappear 
afterwards).


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-20 Thread Aleix Pol Gonzalez 


> On Nov. 20, 2015, 10:06 p.m., René J.V. Bertin wrote:
> > src/ioslaves/trash/trashimpl.cpp, line 947
> > 
> >
> > These were `kDebug()` statements; I guess they should become 
> > `qWarning()` now that `kDebug` is gone. I've left the qDebug() statements 
> > in because I noticed there are plenty of those in the code; shouldn't they 
> > be turned into `qCWarning()` or `qCDebug` to avoid polluting the terminal 
> > or (on OS X) the `system.log` ?

Yes, they should, eventually.


- Aleix


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126125: [OS X] make KDE's trash use the OS X trash

2015-11-20 Thread Aleix Pol Gonzalez 

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



src/ioslaves/trash/trashimpl.cpp (line 129)


Infrastructure is 1 word


I'm wondering if ifdef'ing the shit out of the code is the best solution. Would 
it make sense to split it in a couple of files or get an `_apple.cpp` file? Or 
even get a completely different trash:/ kio implementation?

- Aleix Pol Gonzalez


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *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 patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel