<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/103182/">http://git.reviewboard.kde.org/r/103182/</a>
</td>
</tr>
</table>
<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;">I am not sure (3) makes sense. Also would it be possible for you to send two separate patches, one with just the spacing and the other with the feature? It would make it really easier to review, since now you have to look twice to see if it's just whitespace changes or also there some code change in a given line.
Sorry for taking so much to answer but kmplot development is basically stalled and i just found your review request while doing a rourinary "janitor" review request round.</pre>
<br />
<p>- Albert</p>
<br />
<p>On November 18th, 2011, 11:26 p.m., Eike Krumbacher wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Edu.</div>
<div>By Eike Krumbacher.</div>
<p style="color: grey;"><i>Updated Nov. 18, 2011, 11:26 p.m.</i></p>
<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;">Hello Folks!
If you have a set of functions like
f(x) = g x²
g(x) = 2 x
* kmplot crashes without notice, because the parser reads "g" as a function and reports "Wrong number of arguments" but crashes.
* If "g" is meant to be a predefined constant the parser crashes because g is read as a function with wrong argument counts.
So there are several cases:
1) g(x) and f(x) exists in a file, reading the file should not crash
2) if g is a constant, whether in the file or "global", f(x) should read this constant first
3) a function set like
g(x) = 10 * g
f(x) = g(x) + g
should be read and displayed.
With the given diff, we can archive (1) and (2) but not (3), which now does not crash anymore but gives strange results. In the surrounding of the changes, I added some whitespaces to reflect the style guide of the given code. The main part is an introduction of the token "ERROR", adding this token to the stack in case of an error.
Have fun
Eike</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;">(1), (2) and (3) was tested. The original file from the bug report associated with this review was tested. No crashes in these special cases. </pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=204068">204068</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kmplot/parser.h <span style="color: grey">(c3fb92e)</span></li>
<li>kmplot/parser.cpp <span style="color: grey">(23569aa)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103182/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>