<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 25th, 2015, 9:32 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;">I think one needs to add a outer scope with some "KIO-Protocol-Info" or so key to be able to check if we have valid data inside a lib.
But I am not sure how to name that.</p></pre>
</blockquote>
<p>On October 26th, 2015, 10:48 a.m. UTC, <b>Alex Merry</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;">How about "KDE-KIO-Protocols"? I think having a KDE prefix in keys for KDE software is good practice.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd have thought the "exec" key is unnecessary if you're embedding it into the exec.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have to say that the "Class", "Icon" and "X-DocPath" entries look a little out of place; I don't know if we'd maybe want to take the opportunity to adjust those (to "class", "icon" and "docpath", I guess).</p></pre>
</blockquote>
<p>On October 26th, 2015, 11 a.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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">How about "KDE-KIO-Protocols"? I think having a KDE prefix in keys for KDE software is good practice.
Sound good too me.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd have thought the "exec" key is unnecessary if you're embedding it into the exec.
I have to say that the "Class", "Icon" and "X-DocPath" entries look a little out of place; I don't know if we'd maybe want to take the opportunity to adjust those (to "class", "icon" and "docpath", I guess).
Actually I would like to keep all keys "as is" to have some easy 1:1 mapping to the .protocol files that still will be kept like they are.
Thought I can add that renaming, if wanted by David, too.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Btw., if this approach would be OK, would you help me to create a cmake macro for that in kio like the one for desktoptojson?
My CMake Foo is very limited (beside copy'n'paste and adjust) ;=)</p></pre>
</blockquote>
<p>On October 26th, 2015, 11:08 a.m. UTC, <b>Alex Merry</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 can definitely see the advantage of keeping the keys the same, it just struck me that if we <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">do</em> want to change the keys, this is the perfect opportunity.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sure, I can help with the CMake stuff.</p></pre>
</blockquote>
<p>On October 26th, 2015, 11:12 a.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;">I was confused by the names during typing in the copy list, too ;)
Let's see what David thinks about the helper, if he is OK, I will come back to that help offer.</p></pre>
</blockquote>
<p>On October 26th, 2015, 2:25 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;">Aren't the "protocol" and "exec" key redundant? exec is not needed if we embed it in the library and the value for protocol seems to always(?) be the same as the object key.</p></pre>
</blockquote>
<p>On October 26th, 2015, 4: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;">Namespace "KDE-KIO-Protocols" added. For the redundant keys: I can remove them, if wanted, for the first version I would like more a 1:1 conversion, thought, until all steps are there to have this running.</p></pre>
</blockquote>
<p>On October 26th, 2015, 4:59 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;">I have very little input to give on the json and the conversion program, I didn't touch that stuff, please ask Alex Richardson for input.</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;">Ok, added him as reviewer, thanks for the pointer ;=)
He has already given some feedback, to answer that explicitly:</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Aren't the "protocol" and "exec" key redundant? exec is not needed if we embed it in the library and the value for protocol seems to always(?) be the same as the object key.
I can remove both, I think they are redudant, as exec shall be the plugin we look at and protocol seems to match always the basename of the protocol file anyway.
If you want that, I can do that, are there other things to keep in mind to have a json that is packable into the .so via the Qt Plugin meta data trick?
If I get a ship it, I think I will need Alex Merry to setup a ecm macro for me ;)</p>
</blockquote></pre>
<br />
<p>- Christoph</p>
<br />
<p>On October 26th, 2015, 4:50 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 and David Faure.</div>
<div>By Christoph Cullmann.</div>
<p style="color: grey;"><i>Updated Oct. 26, 2015, 4:50 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>