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

2015-01-13 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Jan. 13, 2015, 5:44 p.m.) Status -- This change has been

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

2015-01-10 Thread Albert Astals Cid
On gen. 9, 2015, 12:27 a.m., David Faure wrote: Sorry, this one got drowned in the ML noise and I missed it After renaming idForDevice to idForMountPoint, please push to kde-runtime once the Applications 14.12 freeze is over. Then you'll have to forward-port this commit to the

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

2015-01-10 Thread René J . V . Bertin
On Jan. 9, 2015, 1:27 a.m., David Faure wrote: Sorry, this one got drowned in the ML noise and I missed it After renaming idForDevice to idForMountPoint, please push to kde-runtime once the Applications 14.12 freeze is over. Then you'll have to forward-port this commit to the

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

2015-01-09 Thread René J . V . Bertin
On Jan. 9, 2015, 1:27 a.m., David Faure wrote: Sorry, this one got drowned in the ML noise and I missed it After renaming idForDevice to idForMountPoint, please push to kde-runtime once the Applications 14.12 freeze is over. Then you'll have to forward-port this commit to the

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

2015-01-08 Thread David Faure
On Oct. 19, 2014, 8:25 a.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 870 https://git.reviewboard.kde.org/r/120573/diff/9/?file=320645#file320645line870 Should be const QString mountPoint, you're not modifying it. Also, rename this method to

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

2015-01-08 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review73542 --- Ship it! Sorry, this one got drowned in the ML noise and I

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

2014-11-27 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Nov. 28, 2014, 12:04 a.m.) Review request for KDE Software on

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

2014-10-19 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68687 --- kioslave/trash/CMakeLists.txt

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

2014-10-18 Thread René J . V . Bertin
On Oct. 18, 2014, 11:28 a.m., David Faure wrote: 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.

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

2014-10-18 Thread David Faure
On oct. 18, 2014, 9:28 matin, David Faure wrote: 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.

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

2014-10-18 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 18, 2014, 8:08 p.m.) Review request for KDE Software on

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

2014-10-18 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 1043 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line1043 such a debug statement is more useful if it prints out the input to the method, i.e. topdir. René J.V.

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

2014-10-16 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 170 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line170 Shouldn't this return false like the other blocks? And then I would swap the if and else blocks,

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

2014-10-16 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 1043 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line1043 such a debug statement is more useful if it prints out the input to the method, i.e. topdir. René J.V.

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

2014-10-16 Thread Thomas Lübking
On Okt. 14, 2014, 9:13 nachm., David Faure wrote: kioslave/trash/trashimpl.cpp, line 170 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line170 Shouldn't this return false like the other blocks? And then I would swap the if and else blocks,

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

2014-10-16 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 170 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line170 Shouldn't this return false like the other blocks? And then I would swap the if and else blocks,

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

2014-10-16 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 170 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line170 Shouldn't this return false like the other blocks? And then I would swap the if and else blocks,

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

2014-10-16 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 854 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line854 deleteEmptyTrashInfraStructure is implemented on all OSes, but only called on Mac, which seems a bit

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

2014-10-16 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 17, 2014, 12:35 a.m.) Review request for KDE Software on

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 9:35 p.m., Thomas Lübking wrote: kioslave/trash/kcmtrash.cpp, line 220 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318518#file318518line220 don't know about the exotic OS policy on this, but changes to i18n strings need to happen for new minors only

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 9:35 p.m., Thomas Lübking wrote: kioslave/trash/trashimpl.cpp, line 362 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line362 This can only make sense when a) init() created $TRASH/KDE.trash b) deleteEmptyTrashInfraStructure()

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 854 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line854 deleteEmptyTrashInfraStructure is implemented on all OSes, but only called on Mac, which seems a bit

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 984 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line984 Damn, if only we had known, we could have made the XDG spec closer to the OSX implementation just by adding es

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 1043 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line1043 such a debug statement is more useful if it prints out the input to the method, i.e. topdir. Point(s) taken.

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

2014-10-15 Thread René J . V . Bertin
On Oct. 14, 2014, 11:13 p.m., David Faure wrote: kioslave/trash/trashimpl.cpp, line 170 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line170 Shouldn't this return false like the other blocks? And then I would swap the if and else blocks,

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

2014-10-15 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 15, 2014, 6:26 p.m.) Review request for KDE Software on

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

2014-10-15 Thread Thomas Lübking
On Okt. 14, 2014, 7:35 nachm., Thomas Lübking wrote: kioslave/trash/trashimpl.cpp, line 362 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318520#file318520line362 This can only make sense when a) init() created $TRASH/KDE.trash b) deleteEmptyTrashInfraStructure()

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

2014-10-14 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 14, 2014, 10:38 a.m.) Review request for KDE Software on

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

2014-10-14 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 14, 2014, 1:59 p.m.) Review request for KDE Software on

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

2014-10-14 Thread René J . V . Bertin
On Oct. 14, 2014, 9:35 p.m., Thomas Lübking wrote: kioslave/trash/kcmtrash.cpp, line 220 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318518#file318518line220 don't know about the exotic OS policy on this, but changes to i18n strings need to happen for new minors only

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

2014-10-14 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68418 --- Nice! Just some comments. kioslave/trash/trashimpl.cpp

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

2014-10-14 Thread David Faure
On Oct. 14, 2014, 7:35 p.m., Thomas Lübking wrote: kioslave/trash/kcmtrash.cpp, line 220 https://git.reviewboard.kde.org/r/120573/diff/6/?file=318518#file318518line220 don't know about the exotic OS policy on this, but changes to i18n strings need to happen for new minors only

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

2014-10-13 Thread René J . V . Bertin
--- 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

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

2014-10-13 Thread Lamarque Souza
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68315 --- kioslave/trash/trashimpl.cpp

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

2014-10-13 Thread Milian Wolff
On Oct. 13, 2014, 6:03 p.m., Lamarque Souza wrote: kioslave/trash/trashimpl.cpp, line 815 https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line815 QFile::remove() does not work with directories. You should use QDir().rmdir() instead, which only works as long

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

2014-10-13 Thread René J . V . Bertin
On Oct. 13, 2014, 8:03 p.m., Lamarque Souza wrote: kioslave/trash/trashimpl.cpp, line 815 https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line815 QFile::remove() does not work with directories. You should use QDir().rmdir() instead, which only works as long

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

2014-10-13 Thread Milian Wolff
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68318 --- kioslave/trash/trashimpl.cpp

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

2014-10-13 Thread René J . V . Bertin
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

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

2014-10-13 Thread Thomas Lübking
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68322 --- kioslave/trash/trashimpl.cpp

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

2014-10-13 Thread Thomas Lübking
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

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

2014-10-13 Thread René J . V . Bertin
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

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

2014-10-13 Thread Thomas Lübking
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

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

2014-10-13 Thread René J . V . Bertin
On Oct. 13, 2014, 9:31 p.m., Thomas Lübking wrote: kioslave/trash/trashimpl.cpp, line 363 https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line363 why dI assume you want to remove a *trailing* /KDE.trash? I would not call my login like this, but you never

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

2014-10-13 Thread Thomas Lübking
On Okt. 13, 2014, 7:31 nachm., Thomas Lübking wrote: kioslave/trash/trashimpl.cpp, line 363 https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line363 why dI assume you want to remove a *trailing* /KDE.trash? I would not call my login like this, but you

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

2014-10-13 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 13, 2014, 10:42 p.m.) Review request for KDE Software on

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

2014-10-13 Thread René J . V . Bertin
On Oct. 13, 2014, 8:03 p.m., Lamarque Souza wrote: kioslave/trash/trashimpl.cpp, line 815 https://git.reviewboard.kde.org/r/120573/diff/2/?file=318230#file318230line815 QFile::remove() does not work with directories. You should use QDir().rmdir() instead, which only works as long

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

2014-10-13 Thread Thomas Lübking
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/#review68331 --- kioslave/trash/trashimpl.cpp

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

2014-10-13 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 13, 2014, 11:02 p.m.) Review request for KDE Software on

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

2014-10-13 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 13, 2014, 11:09 p.m.) Review request for KDE Software on

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

2014-10-13 Thread René J . V . Bertin
On Oct. 13, 2014, 10:59 p.m., Thomas Lübking wrote: kioslave/trash/trashimpl.cpp, line 168 https://git.reviewboard.kde.org/r/120573/diff/3/?file=318241#file318241line168 nothing actuallly changed here - if this failed, the following testDir( trashDir + QString::fromLatin1(/info)

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

2014-10-13 Thread René J . V . Bertin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120573/ --- (Updated Oct. 13, 2014, 11:20 p.m.) Review request for KDE Software on