D7671: Fix automatic reload of files saved with QSaveFile

2017-09-12 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#144616, @rkflx wrote: > For completeness, for a file watch on "x" (not sure whether you may have confused file and dir watches as well as file and dir Okular code pathes in your reply), we get: > > - `touch x` (no x yet) →

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-10 Thread Henrik Fehlauer
rkflx added a comment. For completeness, for a file watch on "x" (not sure whether you may have confused file and dir watches as well as file and dir Okular code pathes in your reply), we get: - `touch x` (no x yet) → "created" signal - `touch x` (again) → "dirty" signal - `touch

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-09 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#143898, @rkflx wrote: > After further analysis, I can draw these conclusions (contradicting some assumptions from above, confirming others): > > 1. KDirWatch correctly emits at least one "dirty" each for directory watches

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-07 Thread Henrik Fehlauer
rkflx added a comment. After further analysis, I can draw these conclusions (contradicting some assumptions from above, confirming others): 1. KDirWatch correctly emits at least one "dirty" each for directory watches when files are created, modified, deleted or moved_to (i.e., KDirWatch

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-07 Thread Julian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R223:e3747ca3fd9b: Fix automatic reload of files saved with QSaveFile (authored by progwolff). REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7671?vs=19119=19269

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-06 Thread Albert Astals Cid
aacid accepted this revision. aacid added a comment. This revision is now accepted and ready to land. In https://phabricator.kde.org/D7671#143401, @progwolff wrote: > In https://phabricator.kde.org/D7671#143269, @aacid wrote: > > > Have you read my email? There clearly says what

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-06 Thread Albert Astals Cid
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

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-06 Thread 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

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-05 Thread Henrik Fehlauer
rkflx added a comment. 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

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-05 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#143187, @progwolff wrote: > In https://phabricator.kde.org/D7671#143118, @aacid wrote: > > > No, the code doesn't wait *only* for a dirty for path/file, read Part::slotFileDirty better > > > > The problem here is that

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-05 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#143202, @progwolff wrote: > ~~Okay, I modified KDirWatch so we actually get a dirty signal for the directory.~~ > > Now a new problem arised. > QSaveFile does not delete and recreate the file as we thought. It just

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-05 Thread Julian Wolff
progwolff added a comment. Okay, I modified KDirWatch so we actually get a dirty signal for the directory. Now a new problem arised. QSaveFile does not delete and recreate the file as we thought. It just moves the swap file to the old file's location. On move, KDirWatch still sends

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-05 Thread Julian Wolff
progwolff added a comment. In https://phabricator.kde.org/D7671#143118, @aacid wrote: > No, the code doesn't wait *only* for a dirty for path/file, read Part::slotFileDirty better > > The problem here is that dirty for the path is not being emitted,

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-04 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#142933, @progwolff wrote: > In https://phabricator.kde.org/D7671#142826, @aacid wrote: > > > I don't know who told you this is the correct behaviour of kdirwatch, but i kind of disagree. > > > > Since we're not watching

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-04 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D7671#142844, @rkflx wrote: > Albert: Do you remember why https://phabricator.kde.org/R223:f93ccd7923491c6b1412ba5cb1fe0711e44496d8 was necessary? There is no bug linked and no autotest. Using just the commit message as a testcase,

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-04 Thread Julian Wolff
progwolff added a comment. In https://phabricator.kde.org/D7671#142826, @aacid wrote: > I don't know who told you this is the correct behaviour of kdirwatch, but i kind of disagree. > > Since we're not watching the file but *also* the folder. From the KDirWatch class

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Henrik Fehlauer
rkflx added a comment. Albert: Do you remember why https://phabricator.kde.org/R223:f93ccd7923491c6b1412ba5cb1fe0711e44496d8 was necessary? There is no bug linked and no autotest. Using just the commit message as a testcase, with https://phabricator.kde.org/D7671 applied and

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Albert Astals Cid
aacid added a comment. I don't know who told you this is the correct behaviour of kdirwatch, but i kind of disagree. Since we're not watching the file but *also* the folder. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D7671 To: progwolff, aacid Cc: sander,

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Oliver Sander
sander added a comment. Such a @note seems like a good idea. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D7671 To: progwolff, aacid Cc: sander, rkflx, #okular, aacid

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Henrik Fehlauer
rkflx added a comment. Nice, solves the issues for me (and confirms my suspicion that this is not a regression or porting issue, as this is broken for me even on an ancient KDE4 distro), but I'll let Albert give the final "ship it". Do you think we should add a `@note` to that effect to

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Julian Wolff
progwolff edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D7671 To: progwolff, aacid Cc: #okular, aacid

D7671: Fix automatic reload of files saved with QSaveFile

2017-09-03 Thread Julian Wolff
progwolff created this revision. Restricted Application added a subscriber: Okular. Restricted Application added a project: Okular. REVISION SUMMARY Files saved with QSaveFile don't get dirty. The are deleted and replaced. Thus, inotify and KDirWatch don't emit a "dirty" signal (wich is the