<table><tr><td style="">aacid accepted this revision.<br />aacid added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D7671" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D7671#143401" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D7671#143401</a>, <a href="https://phabricator.kde.org/p/progwolff/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@progwolff</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D7671#143269" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D7671#143269</a>, <a href="https://phabricator.kde.org/p/aacid/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@aacid</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>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).</p></div>
</blockquote>
<p>Sure, I read your mail. But I still don't think that KDirWatch behaves wrong.</p></div>
</blockquote>
<p>Why?</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>On saving a file with QSaveFile or on a plain <tt style="background: #ebebeb; font-size: 13px;">mv somefile watchedfile</tt> the watched file is never removed.</p></blockquote>
<p>According to KDirwatch, it is</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>
<p>According to KDirwatch, it is</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>
<p>The file may "in reality never be creater or removed", but KDirWatch is sending the removed and created signals, so if the documentation says "dirty is triggered when the file is removed and created", well it should send the signal, otherwise it's pretty confusing, and i don't know why we're discussing this here instead of on the mailing list where i actually asked for clarification.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>So, this behaviour is a little strange and confusing, but from my perspective it's still coherent with the documentation.</p></blockquote>
<p>I disagree.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Even if KDirWatch would work as you expect it to, there are cases where Okular still does not react to those signals:<br />
Consider the command <tt style="background: #ebebeb; font-size: 13px;">rm watchedfile && touch watchedfile</tt>.<br />
KDirWatch will send the signals: file removed, directory dirty, file added, directory dirty.<br />
Okular receives the first dirty signal asynchronously. Okular checks if the file exists via <tt style="background: #ebebeb; font-size: 13px;">QFile::exists</tt>. It is likely that the file has already been added (<tt style="background: #ebebeb; font-size: 13px;">touch</tt>) by that time, so Okular will miss to reload the file.<br />
Actually, this case seems to work with your code, but I hope you understand what I want to say. It just doesn't cover every potential case. Same problems apply for <tt style="background: #ebebeb; font-size: 13px;">mv</tt>.</p></blockquote>
<p>This is actually the only good explanation of why the code is not robust enough, you're right there's races involved in the logic. So yeah let's commit this fix then.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7671" rel="noreferrer">https://phabricator.kde.org/D7671</a></div></div><br /><div><strong>To: </strong>progwolff, aacid<br /><strong>Cc: </strong>sander, rkflx, Okular, aacid<br /></div>