D7671: Fix automatic reload of files saved with QSaveFile

[Julian Wolff]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20170906/f381ae46/attachment-0001.html>