<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/129205/">https://git.reviewboard.kde.org/r/129205/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 30th, 2016, 10:38 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;">KCoreDirLister is complex enough, I'd rather not add features to it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let's take a step back.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This looks like an already solved problem to me, if I understand it correctly:
- a kioslave creates a virtual filesystem on top of the local file system (there are many doing that: kio_desktop, kio_stash, 3 baloo ones (baloosearch, timeline, tags) in fact there are at least 21 (those with Class=:local) but they don't all care about deletion, e.g. because readonly)
- the user deletes/renames a local file (or even adds a file, that should be added to the virtual filesystem)
- somehow (see below) the app (via KCoreDirLister) needs to be notified using a URL of that virtual filesystem (not just file:///).
right?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The way this has always been done, where implemented (kio_desktop, kio_stash, and in the past some others) is the following
- because kioslaves are short-lived, they can't do the watching (this replies to Mark's suggestion number 1)
- a kded module is created, to do the watching using KDirWatch - of only those files that the kioslave is listing
- the kded module emits KDirNotify DBus signals when a file is removed/renamed, using the kioslave's URL scheme.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You can see these existing kded modules here:
- for desktop: plasma-workspace/kioslave/desktop/desktopnotifier.cpp
- for baloo: frameworks/baloo/src/kioslaves/kded/
- for stash: playground/utils/kio-stash/src/iodaemon/</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It seems to me that simply filenamesearch needs its own similar kded module. I don't see any in kio-extras/filenamesearch.</p></pre>
</blockquote>
<p>On October 30th, 2016, 11 p.m. UTC, <b>Albert Astals Cid</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;">Given we seem to have "a few" of those kded modules, is there something we can do to help people making them and not having the same trouble over and over again?</p></pre>
</blockquote>
<p>On October 31st, 2016, 8:22 a.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;">What do you mean? Documentation, yes. Shared code? I'm not sure. We're talking about one class with one constructor and 2 or 3 methods, and what to react to, depends on the kioslave. And how to react (converting from file:// to custom://) depends on the kioslave too.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(BTW use desktopnotifier or kio-stash as example, baloo is a bit different because it's listening to another daemon (so it didn't really need a kded module in the first place)</p></pre>
</blockquote>
<p>On October 31st, 2016, 11:21 a.m. UTC, <b>Albert Astals Cid</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;">Some documentation i guess, this is not the first time this question has come up, and i guess people just don't know what to do to update stuff when slaves are short lived and stateless.</p></pre>
</blockquote>
</blockquote>
<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;">Documentation added in SlaveBase: http://commits.kde.org/kio/6a17de1b68d36f87d1fd9813750805f78d069bc3</p></pre>
<br />
<p>- David</p>
<br />
<p>On October 31st, 2016, 3:53 p.m. UTC, Anthony Fieroni 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 Anthony Fieroni.</div>
<p style="color: grey;"><i>Updated Oct. 31, 2016, 3:53 p.m.</i></p>
<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;">David, i will discard review if you don't like it, cause watching files changes can be <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">really</em> expensive. I try to:
1. to not break abi compability
2. to extend filenamesearch with this option
3. to fix https://git.reviewboard.kde.org/r/129141/</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;">For 3. i still can't figure out why in filenamesearch signal for delete item(s) is not triggered.
1. Search for file (by name) in dolphin
2. When appear in view delete him
3. Signal ItemsDeleted is not triggered, file stays in the view even if new search is performed and reload is needed. The cache look good, tests pass, works but in filenamesearch.</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.h <span style="color: grey">(e6ba2ac)</span></li>
<li>src/core/kcoredirlister.cpp <span style="color: grey">(508516e)</span></li>
<li>src/core/kcoredirlister_p.h <span style="color: grey">(9a3cc7b)</span></li>
<li>tests/kdirlistertest_gui.h <span style="color: grey">(8316b20)</span></li>
<li>tests/kdirlistertest_gui.cpp <span style="color: grey">(11e89cf)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129205/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>