<table><tr><td style="">meven added inline comments.
</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/D22144">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22144#inline-131962">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:29</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">You should also set LIBRARY_OUTPUT_DIRECTORY, see <a href="https://community.kde.org/Guidelines_and_HOWTOs/Making_apps_run_uninstalled" class="remarkup-link" target="_blank" rel="noreferrer">https://community.kde.org/Guidelines_and_HOWTOs/Making_apps_run_uninstalled</a></p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This will be the first ioslave in kio-extras using it...<br />
Some of the ioslave even use the old protocol files...<br />
Might be a followup maintenance Diff...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22144#inline-131972">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">recentlyused.cpp:91</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I may be missing something, but if you don't really reimplement rewriteUrl, then what's the point of using ForwardingSlaveBase in the first place, instead of just SlaveBase?</p>
<p style="padding: 0; margin: 8px;">IIRC *all* of ForwardingSlaveBase's code is based on the rewriteUrl idea.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I started my work upon the recentdocument ioslave that used it and ForwardingSlaveBase documentation states "It has been designed to support only local filesystem like ioslaves.. If the resulting ioslave should be a simple proxy, you only need to implement the ForwardingSlaveBase::rewriteUrl() method." makes it sound like that is what I needed.<br />
Using SlaveBase would need a lot more code added also that I don't particularly need to implement here.<br />
But please correct me.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22144#inline-131974">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">recentlyused.cpp:109</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">(hmm, someone should implement operator |= in kactivities-stat, to make such code simpler and possibly faster)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Tried implementing this in <a href="https://phabricator.kde.org/D23372" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D23372</a> but <a href="https://phabricator.kde.org/p/ivan/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@ivan</a> was not found of it because of induced complexity it entails to work properly.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22144#inline-131977">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">recentlyused.cpp:256</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Isn't that a problem? Any user of this ioslave might stat() a URL representing a file or directory, coming from this kioslave.</p>
<p style="padding: 0; margin: 8px;">E.g. if you paste a URL of a subdir from one dolphin window to another, I suspect it will happen then. At least in konqueror it does, maybe dolphin assumes everything is a directory since it can't do much with a file URL....</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I implemented the kioslave to allow only stating and listing "/" path on purpose : I could not find a use case for subdirs.<br />
I chose url parameters to pass pararmeters for simplicity albeit it makes it less appealing in UI as you cannot have subpath breaking down your parameters.<br />
Any files or folders appearing in the ioslave keep their true urls.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D22144#inline-131969">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">recentlyused.h:57</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">That's ambiguous. Is it a path or a URL?</p>
<p style="padding: 0; margin: 8px;">A "path that can contain schemes" is an invalid mixture of two different things.<br />
If it's a path, a '#' will mean an actual '#' in the filename.<br />
If it's a URL, a '#' in the filename will require encoding as %23, since a '#' in the URL would actually mean a fragment.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It is path currently due to limitation in RecentlyUsed::udsEntryFromResource being able to create valid UDS::Entry only for files.<br />
But it could evolve later to support any url that the sqlite database ~/.local/share/kactivitymanagerd/resources/database stores in its targettedResource column, including ioslave urls, kcm urls, applications desktop files...</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R320 KIO Extras</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22144">https://phabricator.kde.org/D22144</a></div></div><br /><div><strong>To: </strong>meven, ivan, Frameworks, ngraham, dfaure<br /><strong>Cc: </strong>elvisangelaccio, kde-frameworks-devel, kfm-devel, aprcela, vmarinescu, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov<br /></div>