<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/125797/">https://git.reviewboard.kde.org/r/125797/</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 26th, 2015, 5:27 p.m. UTC, <b>Alex Richardson</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;">The protocoltojson program will have to read the plugin metadata.json file to insert the "KDE-KIO-Protocols" to that json file as it is not possible to embed more than one JSON file into a Qt plugin.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am not sure whether we need a CMake macro, to me this is a conversion that should be done once and then we store the JSON file in the repository so that we don't increase the build time.</p></pre>
</blockquote>
<p>On October 26th, 2015, 5:39 p.m. UTC, <b>Christoph Cullmann</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;">Actually ioslaves have atm no other json stored, this would be the only meta data added to them, I think no ioslave is at all a qt plugin and to stay compatible, I would only add a dummy class there with this json embedded to have the meta data available. But perhaps I am mistaken.
For the one time conversion: I am not sure that the .protocol files will be deprecated, I thought to do it like ATM for the .desktop files for the plugins for which we kept the .desktop files, too.</p></pre>
</blockquote>
<p>On October 26th, 2015, 5:48 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;">IIRC we kept .desktop files because there was no other way to translate them, back then.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But that's not an issue anymore, and not an issue for .protocol files anyway.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So when converting an ioslave to json, rather than just change its cmakelists, we could also just convert the .protocol to json, either with a one-time conversion tool (now that you wrote it...) or by hand, reading some documentation on the format.
Seems simpler than more cmake magic.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KIO will need to keep being able to parse .protocol files anyway, for compat reasons, that's unrelated.</p></pre>
</blockquote>
<p>On October 26th, 2015, 5:51 p.m. UTC, <b>Christoph Cullmann</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;">Ok ;) That makes its easier, no cmake foo needed, cool.
For the second issue: I am right, or? There is no other meta data we need inside the io slave?</p></pre>
</blockquote>
<p>On October 26th, 2015, 6:18 p.m. UTC, <b>Christoph Cullmann</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;">My idea would be to add some:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">/*<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
* Pseudo plugin class to embedd meta data
</em>/
class KIOPluginForMetaData : public QObject
{
Q_OBJECT
Q_PLUGIN_METADATA(IID "org.kde.kio.slave.http" FILE "http.json")
};</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">to each slave that then uses the meta data json as above.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Moc will then embedd that like needed and no more stuff is needed to have the protocols meta data in the .so files, at least that seems to work for me.</p></pre>
</blockquote>
<p>On October 27th, 2015, 12:52 p.m. UTC, <b>Alex Richardson</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;">Ah I thought there was already some JSON embedded. This makes it much simpler.
I don't think any of the default KPluginMetaData values are useful for KIOSlaves (e.g. Icon can be different for each protocol) so only having these JSON keys should be fine.
We can always add KPlugin { foo: "" } later if we need it.</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;">I would keep the </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">{
"KDE-KIO-Protocols": {
"http": {
....</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">}</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">namespace, later than like you said we could have some "KPlugin" in addition and I don't need to sort such stuff out then during the collection of the protocols.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is that otherwise fine? If yes, I would like to commit this and then continue in a new review request the embedding of this in one example slave, http, and the extension of the protocolinfo/factory to read this in addition to the .protocol files.</p></pre>
<br />
<p>- Christoph</p>
<br />
<p>On October 26th, 2015, 5:03 p.m. UTC, Christoph Cullmann 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, Alex Richardson and David Faure.</div>
<div>By Christoph Cullmann.</div>
<p style="color: grey;"><i>Updated Oct. 26, 2015, 5:03 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;">Application to convert multiple .protocol files into on json map.
If this is acceptable my next steps would be:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) use this, to generate json files and embedded them in io slaves
2) extend the protocol factory to search in the embedded files</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That would make the protocol factory parts of: https://git.reviewboard.kde.org/r/125778/ not needed.</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;">Feed in http slave files: </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">./protocoltojson -o http.json /local/cullmann/kf5/src/frameworks/kio/src/ioslaves/http/*.protocol</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We get mapping:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">{
"http": {
"Class": ":internet",
"Icon": "text-html",
"X-DocPath": "kioslave5/http/index.html",
"defaultMimetype": "application/octet-stream",
"deleting": true,
"determineMimetypeFromExtension": false,
"exec": "kf5/kio/http",
"input": "none",
"maxInstances": 20,
"maxInstancesPerHost": 5,
"output": "filesystem",
"protocol": "http",
"reading": true,
"writing": true
},
"https": {
"Class": ":internet",
"Icon": "text-html",
"X-DocPath": "kioslave5/http/index.html",
"config": "http",
"defaultMimetype": "application/octet-stream",
"deleting": true,
"determineMimetypeFromExtension": false,
"exec": "kf5/kio/http",
"input": "none",
"maxInstances": 20,
"maxInstancesPerHost": 5,
"output": "filesystem",
"protocol": "https",
"reading": true,
"writing": true
},
"webdav": {
"Class": ":internet",
"Icon": "folder-remote",
"X-DocPath": "kioslave5/webdav/index.html",
"defaultMimetype": "application/octet-stream",
"deleteRecursive": true,
"deleting": true,
"determineMimetypeFromExtension": false,
"exec": "kf5/kio/http",
"input": "none",
"listing": [
"Name",
"Type",
"Size",
"Date",
"AccessDate",
"Access"
],
"makedir": true,
"maxInstances": 20,
"maxInstancesPerHost": 5,
"moving": true,
"output": "filesystem",
"protocol": "webdav",
"reading": true,
"writing": true
},
"webdavs": {
"Class": ":internet",
"Icon": "folder-remote",
"X-DocPath": "kioslave5/webdav/index.html",
"config": "webdav",
"defaultMimetype": "application/octet-stream",
"deleteRecursive": true,
"deleting": true,
"determineMimetypeFromExtension": false,
"exec": "kf5/kio/http",
"input": "none",
"listing": [
"Name",
"Type",
"Size",
"Date",
"AccessDate",
"Access"
],
"makedir": true,
"maxInstances": 20,
"maxInstancesPerHost": 5,
"moving": true,
"output": "filesystem",
"protocol": "webdavs",
"reading": true,
"writing": true
}
}</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/CMakeLists.txt <span style="color: grey">(f65ad8e)</span></li>
<li>src/protocoltojson/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/protocoltojson/main.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125797/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>