dfaure added a comment.
Probably still valid, yes. Needs investigation, and a unittest
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: kde-frameworks-devel, meven, LeGast00n, GB_2, michaelh, ngraham, bruns
meven added a comment.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.
ping @dfaure
Is it still valid ?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: kde-frameworks-devel, meven, LeGast00n, michaelh, ngraham,
aacid requested changes to this revision.
aacid added a comment.
This revision now requires changes to proceed.
Seems this needed changes, was pushed by mistake, then reverted and thus it
stayed in the "albert needs to review" state instead of the "david needs to do
changes" state.
REPOSITOR
dfaure reopened this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: #frameworks
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:07ef5b2d0d36: Use KDirWatch removeDir/addDir instead of
stopDirScan/restartDirScan (authored by dfaure).
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5856?vs=14545&id
aacid requested changes to this revision.
This revision now requires changes to proceed.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: #frameworks
dfaure added a comment.
I know what the problem is with this.
Previously, if nobody was watching the directory, stopDirScan/restartDirScan
would basically do nothing.
Now, after running a kio job in that directory, KDirWatch will start watching
it, for no purpose, due to the unconditional
aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.
About the code itself, it looks good, and i've tested dolphin with copy and
removes and it seems ok, but on the other hand i understand this is mostly an
optimization, so i'm not sure if i wo
aacid added a comment.
In https://phabricator.kde.org/D5856#109721, @dfaure wrote:
> So I think we need to remove the qDebug in KDirWatch so that removeDir is
silent if the dir wasn't watched [this is faster than checking before removing,
from the outside].
+1
REPOSITORY
R241
dfaure added a comment.
In fact, CopyJob has no way to know whether a view somewhere is watching that
directory. When an app other than Dolphin uses CopyJob, there isn't going to be
any watching, possibly.
So I think we need to remove the qDebug in KDirWatch so that removeDir is
silent if
dfaure updated this revision to Diff 14545.
dfaure added a comment.
avoid doing removeDir twice
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5856?vs=14515&id=14545
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D5856
AFFECTED FILES
src/cor
aacid added a comment.
It's kind of harmless i guess, but when doing a rename via F2 in dolphin we
do stopDirScan on the url twice, one in CopyJobPrivate::startRenameJob and
another in CopyJobPrivate::createNextDir
that means you get the following warning
org.kde.kcoreaddons: doesn't kno
dfaure added a comment.
Yes, if this is confirmed to work we can deprecate stopDirScan/restartDirScan.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: #frameworks
aacid added a comment.
Do we want to mark the "old" methods as deprecated?
Update the docu?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D5856
To: dfaure, aacid
Cc: #frameworks
dfaure created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
REVISION SUMMARY
This fixes notifications happening against our will given that inotify
is async.
TEST PLAN
copying/deleting files in dolphin still update
15 matches
Mail list logo
