D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added a comment. Yep, that's it. Renaming trash:/A to trash:/B calls KIO::moveAs which stats trash:/ [useful to resolve desktop:/ to a local dir for instance], which now updates the on-disk cache... with 'A', before it gets renamed to 'B'. I think I know what the best solution

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added inline comments. INLINE COMMENTS > trashimpl.cpp:1092 > +TrashSizeCache trashSize(trashPath); > +TrashSizeCache::SizeAndModTime res = > trashSize.calculateSizeAndLatestModDate(); > +size += res.size; It's actually this call which triggers the unittest

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added a comment. TestTrash::renameDirInTrash makes the dir cache invalid (it still lists trashDirFromHome). I think because of the optimization that we don't always care about the size... but this had the side effect of keeping the cache uptodate I wonder if the test is too

D24773: kio_trash: Add size, modification, access and create date for trash:/

ahmadsamir added a comment. Hello. This broke the testtrash unit test https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.14/45/execution/node/36/log/ REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio,

D24773: kio_trash: Add size, modification, access and create date for trash:/

This revision was automatically updated to reflect the committed changes. Closed by commit R241:570b48e656c7: kio_trash: Add size, modification, access and create date for trash:/ (authored by meven). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D24773?vs=80016=80061#toc REPOSITORY

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D24773_1 REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh,

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 80016. meven marked 2 inline comments as done. meven added a comment. Fix, typo, pass max_mtime by ref to lambda REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=80008=80016 BRANCH arcpatch-D24773_1 REVISION

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added inline comments. INLINE COMMENTS > dfaure wrote in trashsizecache.cpp:131 > Interesting. One benefit of lambdas is that they can work on local variable; > I would have captured max_mtime by reference and modified it inside the > lambda. > Written this way (which I guess more "pure

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a dependent revision: D28794: Allow to display to use UDS_RECURSIVE_SIZE in status bar UDS_RECURSIVE_SIZE. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n,

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > trashsizecache.cpp:131 > +qint64 max_mtime = 0; > +const auto checMaxTime = [max_mtime] (const qint64 lastModTime) -> > qint64 { > +return

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a comment. > But I'm wondering how much effort we want to put into finding the latest mtime, this is starting to sound like a very expensive operation. I agree let's not got too deep down the rabbit hole. This is not a very common use case > When kio_trash takes care

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 80008. meven marked 3 inline comments as done. meven added a comment. Add lambdas to factorize code, comments update, move a couple of DiscSpaceUtil::sizeOfPath to where there are needed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > trashimpl.cpp:1098 > + > +// Find lateest modification date > +if (res.mtime > latestModifiedDate) { typo: latest > trashsizecache.cpp:144 >

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 79943. meven added a comment. Improve naming of struct CacheData to SizeAndModTime REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=79942=79943 BRANCH arcpatch-D24773_1 REVISION DETAIL

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 79942. meven added a comment. This revision is now accepted and ready to land. Improve latest modification time of trash implementation, fix case when a symlink is in the trash returning incorrect trash size REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven planned changes to this revision. meven added a comment. The modification date is not what to expect currently, working on it. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel,

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D24773_1 REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh,

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 79813. meven marked 4 inline comments as done. meven added a comment. Fix typo, avoid allocations, improve variable naming REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=79804=79813 BRANCH arcpatch-D24773_1

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added inline comments. INLINE COMMENTS > kfileitem.h:349 > + * > + * Initialy only implemented for trash:/ > + * typo: Initially > trashimpl.cpp:1099 > + > +info = QFileInfo(trashPath); > +modified = info.lastModified(); Reusing info is more expensive (and

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a dependent revision: D28738: Use KFileItem::recursiveSize to Display recursize size as total size. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2,

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 79804. meven marked 3 inline comments as done. meven added a comment. Rebase, update @since, add precision to KFileItem comment REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=68829=79804 BRANCH arcpatch-D24773_1

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added a comment. This looks good. I think I failed to notice the last update, sorry for the delay. Can you rebase (I suspect a few conflicts) and update version numbers to 5.70? Then it'll be good to go. Ah, and the documentation for the KFileItem getter needs to say that

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 68829. meven added a comment. Add KIO::UDSEntry::UDS_RECURSIVE_SIZE to store recursive size of folders REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=68402=68829 BRANCH arcpatch-D24773 REVISION DETAIL

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a comment. In D24773#554409 , @dfaure wrote: > You can do whatever you want in the GUI -- I'll happily step out of that part of the discussion --, as long as you don't abuse UDS_SIZE for what it was not meant for, thus creating an

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure added a comment. You can do whatever you want in the GUI -- I'll happily step out of that part of the discussion --, as long as you don't abuse UDS_SIZE for what it was not meant for, thus creating an ambiguous meaning for it. My suggestion would be to add a UDS_RECURSIVE_SIZE

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a comment. In D24773#551497 , @ngraham wrote: > In D24773#551245 , @meven wrote: > > > I would argue that "Disk space usage" and "Size" might be confusing to users. A folder size is the

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a comment. In D24773#551497 , @ngraham wrote: > In D24773#551245 , @meven wrote: > > > I would argue that "Disk space usage" and "Size" might be confusing to users. A folder size is the

D24773: kio_trash: Add size, modification, access and create date for trash:/

ngraham added a comment. In D24773#551245 , @meven wrote: > I would argue that "Disk space usage" and "Size" might be confusing to users. A folder size is the size of his content idiomatically. > That is our current usage of Size field for

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven marked 2 inline comments as done. meven added a comment. In D24773#551087 , @dfaure wrote: > I meant in the Dolphin view. RMB / Properties and calculating the size is available for trash:/. See

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I meant in the Dolphin view. RMB / Properties and calculating the size is available for trash:/. See http://www.davidfaure.fr/2019/dolphin_properties_dialog_for_trash.png I

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven marked 3 inline comments as done. meven added inline comments. INLINE COMMENTS > dfaure wrote in trashimpl.cpp:1088 > This method could be const, right? No becauseof the call to `list()` that cannot be const. > dfaure wrote in trashimpl.cpp:1104 > This looks very slow. We have a cache

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven updated this revision to Diff 68402. meven added a comment. Use TrashSizeCache to file directory size, do not add creation date to the UDSEntry, better use UDSEntry reserve/clear REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24773?vs=68281=68402

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

ngraham added a comment. Works great. Not sure creation date makes sense to show for the trash (if it's available) though. Can we suppress that? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc:

D24773: kio_trash: Add size, modification, access and create date for trash:/

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kio_trash.cpp:500 > KIO::UDSEntry entry; > +entry.clear(); > createTopLevelDirEntry(entry); why? > trashimpl.cpp:1088 > > +KIO::UDSEntry

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven removed a dependent revision: D24774: Allow non-local directories to provide UDS_SIZE. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven marked 3 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven marked 2 inline comments as done. meven added inline comments. INLINE COMMENTS > elvisangelaccio wrote in trashimpl.cpp:1091 > Is this really needed? We just created `entry`. It was done previously in void TrashProtocol::createTopLevelDirEntry to save some memory, so I just kept it here

D24773: kio_trash: Add size, modification, access and create date for trash:/

elvisangelaccio added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

elvisangelaccio added a comment. +1 INLINE COMMENTS > trashimpl.cpp:1091 > +KIO::UDSEntry entry; > +entry.clear(); > + Is this really needed? We just created `entry`. > trashimpl.cpp:1094 > +// refresh list of trashes and get the list of files in them > +const

D24773: kio_trash: Add size, modification, access and create date for trash:/

ngraham added a dependent revision: D24774: Allow non-local directories to provide UDS_SIZE. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven added a reviewer: elvisangelaccio. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24773: kio_trash: Add size, modification, access and create date for trash:/

meven created this revision. meven added reviewers: Frameworks, ngraham. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY BUG: 413091 REPOSITORY R241 KIO BRANCH master REVISION DETAIL