<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103555/">http://git.reviewboard.kde.org/r/103555/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 2nd, 2013, 3:18 p.m., <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/103555/diff/1/?file=44960#file44960line1080" style="color: black; font-weight: bold; text-decoration: underline;">kio/kio/kdirlister.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KDirListerCache::slotFileDirty( const QString& path )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1080</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">itemsInUse</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">url</span><span class="p">.</span><span class="n">path</span><span class="p">())</span> <span class="o">&&</span> <span class="o">!</span><span class="n">itemsCached</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">url</span><span class="p">.</span><span class="n">path</span><span class="p">()))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't understand the if(). Why should the behavior differ depending on whether we have this url currently shown (itemsInUse) or in the cache (itemsCached), compared to when we don't know about this URL?
The if() is wrongly written anyway: if it should be there, then it should be url.url(), not url.path(), since itemsInUse and itemsCached use URLs as keys. I should change that to QUrl in KF5.
And in case you remove the if, then my old comment meant: in that case, we can simplify the code to remove the if (isDir), and simply do
Q_FOREACH(const QString& dir, directoriesForCanonicalPath(url.directory())) {
handleDirDirty(dir);
}
in all cases (file or directory).
Because as you say, it's the parent that needs to be re-listed when the permissions of anything changes.
Also, this changes misses a corresponding unittest in kdirlistertest.cpp.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ahh... I see what you mean. I should have realized what I was doing is utterly unnecessary. I will make the change as you suggested and add a test case for this. In fact, I will add the unittest first to test for the failure.</pre>
<br />
<p>- Dawit</p>
<br />
<p>On December 29th, 2012, 8:31 p.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs and David Faure.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Dec. 29, 2012, 8:31 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If you open a directory that contains other directories in Konqueror or Dolphin, change the permission of one of these directories from outside, say the command line, and right click on the same directory to look at the permission tab in the properties dialog, you will see that the permission change has not been updated. This patch addresses that bug.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">1. In konsole, create a test directory within another test directory:
mkdir -p test/test1
2. Open Dolphin or Konqueror and navigate to the top newly created directory, i.e. test.
3. In konsole, cd into the first test directory:
cd test
4. In konsole, change the permission of 'test1' from konsole. For example,
chmod go-rx
5. In the open Dolphin or Konqueror, right click on "test1", select properties and click on permission tab.
6. Validate whether or not the permission shown in the GUI matches what you get in the command line.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=173733">173733</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kio/kio/kdirlister.cpp <span style="color: grey">(ec3d622)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103555/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>