progwolff added a comment.

  In https://phabricator.kde.org/D7671#143269, @aacid wrote:
  
  > Have you read my email? There clearly says what happens and what the 
documentation says it should happen (at least to my understanding of reading 
it).
  
  
  Sure, I read your mail. But I still don't think that KDirWatch behaves wrong.
  On saving a file with QSaveFile or on a plain `mv somefile watchedfile` the 
watched file is never removed. As the docs say, the dirty signal of a directory 
is emitted when a file in this directory is removed or created. This is not the 
case here.
  KDirWatch yet does send a "created" signal on inotify's MOVE_TO flag and a 
"removed" signal on inotify's MOVE_FROM flag. This does not mean, the file 
actually has been removed or added at any time.
  
  So, this behaviour is a little strange and confusing, but from my perspective 
it's still coherent with the documentation.
  
  Even if KDirWatch would work as you expect it to, there are cases where 
Okular still does not react to those signals:
  Consider the command `rm watchedfile && touch watchedfile`.
  KDirWatch will send the signals: file removed, directory dirty, file added, 
directory dirty.
  Okular receives the first dirty signal asynchronously. Okular checks if the 
file exists via `QFile::exists`. It is likely that the file has already been 
added (`touch`) by that time, so Okular will miss to reload the file.
  
  -------------------
  
  In https://phabricator.kde.org/D7671#143360, @rkflx wrote:
  
  > Concerning 
https://phabricator.kde.org/R223:f93ccd7923491c6b1412ba5cb1fe0711e44496d8, I 
find the part re-adding the watch for removed files is also not needed with 
https://phabricator.kde.org/D7671.
  
  
  I agree. I think KDirWatch does this internally. Something that could be 
mentioned in the docs too.
  
  In https://phabricator.kde.org/D7671#143360, @rkflx wrote:
  
  > Here's my suggestion going forward:
  >
  > - Independently from Okular's case, evaluate accuracy of KDirWatch 
autotests and docs regarding "move/rename" (currently not mentioned at all, 
thus to be considered undefined behaviour…) as well as directory operations in 
general, fix potential code bugs and doc confusions
  > - Agree to accept https://phabricator.kde.org/D7671 in any case (reasons: 
code is simpler and less error-prone, fixes the issue even for users on LTS 
distros with old KF5 libs)
  > - Try to revert 
https://phabricator.kde.org/R223:f93ccd7923491c6b1412ba5cb1fe0711e44496d8 
including later fixups
  >
  >   TL;DR: Do both: Fix KDirWatch and apply https://phabricator.kde.org/D7671.
  
  
  I'd be perfectly happy with this. It's probably a good idea to recheck the 
docs of KDirWatch and mention the move_to and move_from cases there.
  Thanks for your constructive participation!

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7671

To: progwolff, aacid
Cc: sander, rkflx, #okular, aacid

Reply via email to