<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113402/">http://git.reviewboard.kde.org/r/113402/</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 23rd, 2013, 2:40 p.m. UTC, <b>Chusslove Illich</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;">I can only say that whatever is the proper fix here, it is fine with me.
Since non-installed headers are being used, maybe ki18n is using KJS in a
way which became deprecated somewhere along the way? If so, I've nothing
against fixing that instead.
</pre>
</blockquote>
<p>On October 23rd, 2013, 2:50 p.m. UTC, <b>Aleix Pol Gonzalez</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;">Well the thing is that ki18n was using private API (since it was in the same module, kdelibs).
Aurélien: what about installing these headers in a private/ directory?</pre>
</blockquote>
<p>On October 23rd, 2013, 3:09 p.m. UTC, <b>Chusslove Illich</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;">If I recall, when implementing that I mimicked what khtml was doing.
And apparently is still doing.
</pre>
</blockquote>
<p>On October 23rd, 2013, 3:15 p.m. UTC, <b>Aleix Pol Gonzalez</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;">khtml is in kdelibs as well, nobody has tried to build it standalone just yet, I guess.</pre>
</blockquote>
<p>On October 23rd, 2013, 3:20 p.m. UTC, <b>Chusslove Illich</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;">Right, but, how is then KJS supposed to be used in a different way? Are
these private headers really private, or actually necessary to use KJS as a
JS interpreter in random client code?
</pre>
</blockquote>
<p>On October 23rd, 2013, 3:22 p.m. UTC, <b>Aurélien Gâteau</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;">The ideal solution would be for ktranscript to only use public headers, but I assume there is no way to do so (I haven't taken the time to study ktranscript enough to understand what it does), Chusslove, it would be awesome if you could have a look at it.
If it is definitely not possible, then I am going to update my changes to install to a private/ directory, like Aleix suggests.</pre>
</blockquote>
<p>On October 24th, 2013, 10:15 a.m. UTC, <b>Aurélien Gâteau</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;">Actually, the even more ideal solution would be for ktranscript to use QtScript instead of KJS. In the future we will have more and more applications developed with QtQuick, so they will already link to QtScript, loading another JavaScript interpreter sounds wasteful to me. Do you think this is doable? I would be quite interested in helping making it happen.</pre>
</blockquote>
<p>On October 24th, 2013, 10:18 a.m. UTC, <b>Aurélien Gâteau</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;">Mmm, actually I am wrong, QtScript is provided for Qt 4.x compatibility, QtQuick uses the QJS* classes from QtQML, so this is what we should be using.</pre>
</blockquote>
<p>On October 24th, 2013, 10:37 a.m. UTC, <b>Kevin Ottens</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;">I would definitely welcome such a port... I remember asking ktranscript to be ported away from KJS. Would also make ki18n tier 1.</pre>
</blockquote>
<p>On October 24th, 2013, 4:25 p.m. UTC, <b>Aurélien Gâteau</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;">I started diving into this today. No change for now, but I wrote some unit tests for ktranscript, so that a qjs port can be evaluated. Patchset is here: http://agateau.com/tmp/ki18n-qjs.patch but it's missing loads of tests for now. Chusslove: would you be interested in extending the test coverage?</pre>
</blockquote>
<p>On October 24th, 2013, 5:03 p.m. UTC, <b>Chusslove Illich</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;">Yes, I most certainly would be interested in extending the test coverage.
But I intended to do it "full-range", that is starting from test PO files
(which are already there), once I get to it. And first I need to document it
better :) Currently all there is is this wiki page:
http://techbase.kde.org/Localization/Concepts/Transcript
As for the port away from KJS, I have to state again that I see no value in
that. Having ki18n be tier 1 is of no use: if someone is so tight on
dependencies that using a tier 2 framework is a problem, then ki18n is the
first thing to dump in favor of built-in Qt translation system, tier 1 or
not. Note that ki18n also brings into play all the Gettext tools, as opposed
to self-contained Qt translation tools. The only possible advantage of ki18n
in tier 1 is that it can be used by tier 2 frameworks, of which there is not
much by my estimate (and between tier 1 and tier 3, I also fail to see the
reason for existence of tier 2).
</pre>
</blockquote>
<p>On October 24th, 2013, 7:23 p.m. UTC, <b>Aurélien Gâteau</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;">There is value in both full-range tests and tests exercising smaller parts of the code as well IMO.
Thanks for the link, I am going to look at it.
Regarding the port to QJSEngine, I see the following advantages to it:
- It solves the build issue I have been trying to fix with this request: no need for uninstalled headers anymore.
- The code is going to be simpler: from what I read of QJSEngine and KJS, exposing an object using QJSEngine is simpler than with KJS: no need for lookup tables or things like that, just a QObject with slots.
- I expect QJSEngine to be better maintained than KJS, given that it is the backbone of QtQuick.
- Having ki18n in tier1 would make it more attractive to Qt developers since it brings superior translation support. Right now the cost of dragging in a separate JavaScript interpreter, can be perceived as a burden especially for QtQuick developers.</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;">Yes, it's not only about the tiers (it's just a nice side-effect). But it's clear that at that point ktranscript is built on top of something huge and unmaintained. It'd be safer built on top of something with proper maintenance if possible.</pre>
<br />
<p>- Kevin</p>
<br />
<p>On October 23rd, 2013, 2:30 p.m. UTC, Aurélien Gâteau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated Oct. 23, 2013, 2:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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;">KI18n depends on KJS to build the ktranscript plugin. This plugin requires an internal KJS Perl script as well as headers which were not installed.
This is a 3-commit patches, which does the following:
1. Install kjs headers
2. Install wtf headers, using wtf/ and kjs/ in include directives (because some kjs headers includes wtf headers)
3. Fix KI18n:
- Format top-level CMakeLists.txt according to CMake template
- Duplicate KJS create_hash_table script
- Add call to find_package(KJS)
Individual patches available from http://agateau.com/tmp/ki18n-standalone.patch</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;">Builds within kdelibs and standalone.</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>superbuild/CMakeLists.txt <span style="color: grey">(5cdec94)</span></li>
<li>tier1/kjs/src/CMakeLists.txt <span style="color: grey">(8629716)</span></li>
<li>tier1/kjs/src/kjs/CMakeLists.txt <span style="color: grey">(9523e89)</span></li>
<li>tier1/kjs/src/wtf/CMakeLists.txt <span style="color: grey">(83b4417)</span></li>
<li>tier1/kjs/src/wtf/FastMalloc.h <span style="color: grey">(29a72a5)</span></li>
<li>tier1/kjs/src/wtf/HashCountedSet.h <span style="color: grey">(be3c387)</span></li>
<li>tier1/kjs/src/wtf/HashFunctions.h <span style="color: grey">(f665447)</span></li>
<li>tier1/kjs/src/wtf/HashMap.h <span style="color: grey">(ba2971c)</span></li>
<li>tier1/kjs/src/wtf/HashSet.h <span style="color: grey">(e84b349)</span></li>
<li>tier1/kjs/src/wtf/HashTable.h <span style="color: grey">(0b2c22c)</span></li>
<li>tier1/kjs/src/wtf/HashTable.cpp <span style="color: grey">(e08d09a)</span></li>
<li>tier1/kjs/src/wtf/HashTraits.h <span style="color: grey">(4d01726)</span></li>
<li>tier1/kjs/src/wtf/ListRefPtr.h <span style="color: grey">(0a704d8)</span></li>
<li>tier1/kjs/src/wtf/OwnArrayPtr.h <span style="color: grey">(3b77871)</span></li>
<li>tier1/kjs/src/wtf/OwnPtr.h <span style="color: grey">(188a1dc)</span></li>
<li>tier1/kjs/src/wtf/PassRefPtr.h <span style="color: grey">(25b9906)</span></li>
<li>tier1/kjs/src/wtf/RefPtr.h <span style="color: grey">(493ab05)</span></li>
<li>tier1/kjs/src/wtf/Vector.h <span style="color: grey">(9b0f38a)</span></li>
<li>tier1/kjs/src/wtf/VectorTraits.h <span style="color: grey">(31ae028)</span></li>
<li>tier2/ki18n/CMakeLists.txt <span style="color: grey">(4cc8e30)</span></li>
<li>tier2/ki18n/src/CMakeLists.txt <span style="color: grey">(7f8259b4)</span></li>
<li>tier2/ki18n/src/create_hash_table <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113402/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>