D7671: Fix automatic reload of files saved with QSaveFile

Wed Sep 6 19:24:19 UTC 2017

aacid added a comment.

  In https://phabricator.kde.org/D7671#143360, @rkflx wrote:

  > In https://phabricator.kde.org/D7671#143117, @aacid wrote:
  >
  > > It was added to make this work, and it did work at some point, i don't add code for nothing ;)
  >
  >
  > Of course, and your code still works today: it fixes the rm/touch case (with https://phabricator.kde.org/D7671 not applied yet). I guess for Kate it broke in 2012 (your patch is from 2008) with the introduction of KSaveFile in https://phabricator.kde.org/R40:a188aa837b7e609a1d555414603fb8d2ed7d70fa. My question was more about why you went for the directory approach instead of just listening for `KDirWatch::created`.

  I don't know, that was long time ago.

  > 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. Will do some more testing over the weekend and maybe prepare a patch. (You can tell I'm not a fan of encoding state in bools with updates spread over the codebase. As far as I can see, in conjunction with the directory watch this led to several bugs, e.g. 384185, 234139, 321880 and maybe more.)

  Because there's a much better way of encoding state that using variables that encode state, right?

  > 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'm hesitant of applying something that just workarounds a library not working.

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/6e97f48a/attachment.html>