<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119607/">https://git.reviewboard.kde.org/r/119607/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Setembro 14th, 2014, 3:27 p.m. UTC, <b>Frank Reininghaus</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="https://git.reviewboard.kde.org/r/119607/diff/2/?file=301214#file301214line420" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoredirlister.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">public:</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">420</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">static</span> <span class="n">QSet</span><span class="o"><</span><span class="n">QString</span><span class="o">></span> <span class="n">filesInDotHiddenForDir</span><span class="p">(</span><span class="k">const</span> <span class="n">QUrl</span><span class="o">&</span> <span class="n">dir</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think that you should not make this part of the public API. AFAICS, you're not using it anywhere outside KCoreDirLister, right?</p></pre>
</blockquote>
<p>On Setembro 14th, 2014, 4:05 p.m. UTC, <b>Bruno Nova</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I left this method (and others) public because I thought that, maybe in the future, someone may want to use this method.
But you're right, it's not used anywhere else and should probably be private/protected.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I moved this method to KCoreDirListerCache, making it private.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Setembro 14th, 2014, 3:27 p.m. UTC, <b>Frank Reininghaus</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="https://git.reviewboard.kde.org/r/119607/diff/2/?file=301215#file301215line2799" style="color: black; font-weight: bold; text-decoration: underline;">src/core/kcoredirlister.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">2799</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QFile</span> <span class="n">dotHiddenFile</span><span class="p">(</span><span class="n">dir</span><span class="p">.</span><span class="n">toLocalFile</span><span class="p">()</span> <span class="o">+</span> <span class="s">"/.hidden"</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This will only work for local files. I'm not sure if we would want to support the ".hidden" mechanism also for other protocols, such as sftp or fish?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Moreover, I think that we might want to cache the contents of the .hidden file, to prevent that we read it, parse it and construct the QSet with the hidden files over and over again if a kioslave reports the UDSEntries in multiple batches.</p></pre>
</blockquote>
<p>On Setembro 17th, 2014, 8:05 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Supporting .hidden for remote protocols would be much more complex (async API).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Caching: it would have to include an mtime, to be able to detect changes. But indeed caching the last one read would be good, because while having a directory open, any change triggers an update job, which will re-read the .hidden file. Or while at it, could be a LRU cache, Qt makes it simple:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">struct CacheHiddenFile {
QDateTime <span style="color: #008000">mtime</span>;
QSet<span style="color: #666666"><</span>QString<span style="color: #666666">></span> listedFiles;
}
QCache<span style="color: #666666"><</span>QString <span style="color: #408080; font-style: italic">/*dot hidden file*/</span>, CacheHiddenFile<span style="color: #666666">></span> m_cacheHiddenFiles;
m_cacheHiddenFiles.setMaxCost(<span style="color: #666666">10</span>);
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This requires making the filesInDotHiddenForDir method not file-static as I previously suggested, but rather a method of KCoreDirLister::Private, which would also have the above member.</p></pre>
</blockquote>
<p>On Setembro 18th, 2014, 10:51 a.m. UTC, <b>Bruno Nova</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now that you mention remote protocols, I just found out that Nautilus ".hidden" also doesn't support them.
For example, ".hidden" is not interpreted when I access a computer through SFTP in Nautilus. However, when I mount it externally using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">sshfs</code>, it works.
Haven't checked Thunar.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the caching, I'll try to take a look at it, but I'll probably need help (I have very little experience with KDE and Qt).</p></pre>
</blockquote>
<p>On Setembro 24th, 2014, 3:29 p.m. UTC, <b>Bruno Nova</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just found a case where the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">.hidden</em> doesn't work.
If a <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">.hidden</em> file is in <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">~/Desktop</em>, it works correctly when accessed from that path, but it doesn't work when accessed from <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">desktop:/</em>.
Is <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">desktop:/</em> interpreted as a network path, and <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">dir.toLocalFile()</code> fails in that path?
I tested in Project Neon in Virtualbox using Kwrite's open dialog, plus LD_LIBRARY_PATH to use the patched library.</p></pre>
</blockquote>
<p>On Setembro 27th, 2014, 11:21 a.m. UTC, <b>Bruno Nova</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">David, for the caching, where should the struct and filesInDotHiddenForDir be declared? KCoreDirLister::Private or KCoreDirListerCache?
filesInDotHiddenForDir is called in two methods of KCoreDirListerCache.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also, you forgot the semi-colon after the struct declaration ;) (and, as usual, the C++ compiler error message was not very informative).</p></pre>
</blockquote>
<p>On Setembro 27th, 2014, 9:42 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">desktop:/ is almost like a remote protocol, except that we can resolve such urls to local files, using KFileItem::localPath(). So using this would be a way to make it work for desktop:/ urls as well as file:/ urls.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Both classes are private (i.e. not part of the public API), so it doesn't matter. So KCoreDirListerCache looks like the easiest solution.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry about the missing semicolon :-)</p></pre>
</blockquote>
<p>On Setembro 29th, 2014, 11:29 a.m. UTC, <b>Bruno Nova</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">From my tests, KFileItem::localPath() does not work for "desktop:/" URLs when the KFileItem is constructed from a QUrl, it just returns an empty string. Worse, when the URL is "file:///" (root folder), localPath() also returns an empty string (strange). And I've also seen it fail randomly in other folders.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It would probably work if the KFileItem was constructed/retrieved from a UDSEntry, but how can I do that in slotEntries() and slotUpdateResult() before iterating over the UDSEntryList?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I uploaded a new patch that caches the ".hidden" file, as advised.
Please check if it's correct.</p></pre>
<br />
<p>- Bruno</p>
<br />
<p>On Dezembro 4th, 2014, 9:55 p.m. UTC, Bruno Nova wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and David Faure.</div>
<div>By Bruno Nova.</div>
<p style="color: grey;"><i>Updated Dez. 4, 2014, 9:55 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=64740">64740</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=246260">246260</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This adds support for <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">.hidden</em> files to KDE.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When listing a directory, the files/folders listed in the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">.hidden</em> file will be hidden, unless the user has chosen to show hidden files.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch was initially based on the patch provided by Mark in Bug #246260.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Built and tested the patch in Project Neon.
Dolphin was still using KDE4/Qt4 version of the library, so I only tested using the desktop folder widget, and "folder desktop".
It worked correctly when I pointed it to "~" and "~/Desktop" (I added ".hidden" there).
However, it didn't work when I pointed it to the "Desktop folder" (the default option, not the custom location "~/Desktop").
More testing is required.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The version for KDE4/Qt4 submitted to Bug #246260 was tested in Kubuntu 14.04, and it worked everywhere I tested (Dolphin, open/save dialogs, folder widget and detailed/tree view in Dolphin).
It wasn't an intensive test, though.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/core/kcoredirlister.cpp <span style="color: grey">(a31d629)</span></li>
<li>src/core/kcoredirlister_p.h <span style="color: grey">(dce7dbc)</span></li>
<li>src/core/kfileitem.h <span style="color: grey">(bebc241)</span></li>
<li>src/core/kfileitem.cpp <span style="color: grey">(74dc069)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119607/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>