<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="https://git.reviewboard.kde.org/r/115439/">https://git.reviewboard.kde.org/r/115439/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115439/diff/3/?file=243156#file243156line52" style="color: black; font-weight: bold; text-decoration: underline;">analitza/expressiontypechecker.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">52</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">virtual</span> <span class="n">Q<span class="hl">String</span></span> <span class="nf">result</span><span class="p">()</span> <span class="k">const</span> <span class="p">{</span> <span class="k">return</span> <span class="n">QString</span><span class="p">();</span> <span class="p">}</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">52</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="n">virtual</span> <span class="n">Q<span class="hl">Variant</span></span> <span class="nf">result</span><span class="p">()</span> <span class="k">const</span> <span class="p">{</span> <span class="k">return</span> <span class="n">QString</span><span class="p">();</span> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">These should be return QVariant() here.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115439/diff/3/?file=243157#file243157line88" style="color: black; font-weight: bold; text-decoration: underline;">analitza/expressiontypechecker.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">ExpressionTypeChecker::ExpressionTypeChecker(Variables* v)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">88</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n"><span class="hl">QString</span></span> <span class="n">ExpressionTypeChecker</span><span class="o">::</span><span class="n"><span class="hl">accep</span>t</span><span class="p">(</span><span class="k">const</span> <span class="n">Operator</span><span class="o">*</span><span class="p">)</span> <span class="p">{</span> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="nb">false</span> <span class="o">&&</span> <span class="s">"should not get here"</span><span class="p">);</span> <span class="k">return</span> <span class="nf">QString</span><span class="p">();</span> <span class="p">}</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">88</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n"><span class="hl">QVariant</span></span> <span class="n">ExpressionTypeChecker</span><span class="o">::</span><span class="n"><span class="hl">visi</span>t</span><span class="p">(</span><span class="k">const</span> <span class="n">Operator</span><span class="o">*</span><span class="p">)</span> <span class="p">{</span> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="nb">false</span> <span class="o">&&</span> <span class="s">"should not get here"</span><span class="p">);</span> <span class="k">return</span> <span class="nf">QString</span><span class="p">();</span> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Same</pre>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">All in all, I would say it's fine. I'm unsure how better it will be, but it's certainly more flexible. Still I would go with void as I already said, but then we would have to port things, with little gain, so QVariant is a good compromise.
Commit when you've fixed the issues I mentioned.</pre>
<p>- Aleix Pol Gonzalez</p>
<br />
<p>On February 11th, 2014, 4:48 a.m. UTC, Percy Camilo Triveño Aucahuasi wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Edu.</div>
<div>By Percy Camilo Triveño Aucahuasi.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2014, 4:48 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
analitza
</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;">Currently ExpressionWriter allows a great flexibility to visit each node (by type) of the parser's tree, but it returns only QString as result.
I propose open this API by this generic interface: AbstractExpressionWriter. This interface can be used by any client to visit the tree and, in addition, the client can choose the type of the results for each visit.
This patch adds AbstractExpressionWriter, ExpressionWriter, List, CustomObject, Matrix, Apply and Container as public API (headers will be installed)
</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;">build & install ok, also each class has the ANALITZA_EXPORT macro.</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>analitza/CMakeLists.txt <span style="color: grey">(af270b6)</span></li>
<li>analitza/abstractexpressionvisitor.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>analitza/abstractexpressionvisitor.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>analitza/analitzautils.cpp <span style="color: grey">(c38eeaa)</span></li>
<li>analitza/apply.h <span style="color: grey">(84b09f5)</span></li>
<li>analitza/apply.cpp <span style="color: grey">(e787372)</span></li>
<li>analitza/container.h <span style="color: grey">(f0deec6)</span></li>
<li>analitza/container.cpp <span style="color: grey">(aabd780)</span></li>
<li>analitza/customobject.h <span style="color: grey">(b6f200c)</span></li>
<li>analitza/customobject.cpp <span style="color: grey">(da8bdb9)</span></li>
<li>analitza/expression.cpp <span style="color: grey">(48fd8d8)</span></li>
<li>analitza/expressiontypechecker.h <span style="color: grey">(8263c2b)</span></li>
<li>analitza/expressiontypechecker.cpp <span style="color: grey">(8616127)</span></li>
<li>analitza/expressionwriter.h <span style="color: grey">(ea9c92d)</span></li>
<li>analitza/expressionwriter.cpp <span style="color: grey">(8aff488)</span></li>
<li>analitza/htmlexpressionwriter.h <span style="color: grey">(f8d0060)</span></li>
<li>analitza/htmlexpressionwriter.cpp <span style="color: grey">(cf7d4a0)</span></li>
<li>analitza/list.h <span style="color: grey">(9719f9e)</span></li>
<li>analitza/list.cpp <span style="color: grey">(fd1e411)</span></li>
<li>analitza/mathmlexpressionwriter.h <span style="color: grey">(70b08ae)</span></li>
<li>analitza/mathmlexpressionwriter.cpp <span style="color: grey">(b70a1ca)</span></li>
<li>analitza/mathmlpresentationexpressionwriter.h <span style="color: grey">(6c6419e)</span></li>
<li>analitza/mathmlpresentationexpressionwriter.cpp <span style="color: grey">(3efc35b)</span></li>
<li>analitza/matrix.h <span style="color: grey">(69c109d)</span></li>
<li>analitza/matrix.cpp <span style="color: grey">(6714ac8)</span></li>
<li>analitza/object.h <span style="color: grey">(aafd114)</span></li>
<li>analitza/object.cpp <span style="color: grey">(b485d07)</span></li>
<li>analitza/operator.h <span style="color: grey">(0e61b21)</span></li>
<li>analitza/operator.cpp <span style="color: grey">(a2d8501)</span></li>
<li>analitza/stringexpressionwriter.h <span style="color: grey">(e8c0ccd)</span></li>
<li>analitza/stringexpressionwriter.cpp <span style="color: grey">(90594e8)</span></li>
<li>analitza/value.h <span style="color: grey">(89bc2ef)</span></li>
<li>analitza/value.cpp <span style="color: grey">(7d4e49a)</span></li>
<li>analitza/variable.h <span style="color: grey">(2f8514c)</span></li>
<li>analitza/variable.cpp <span style="color: grey">(b98365e)</span></li>
<li>analitza/vector.h <span style="color: grey">(338bf81)</span></li>
<li>analitza/vector.cpp <span style="color: grey">(d7b5836)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115439/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>