<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/118904/">https://git.reviewboard.kde.org/r/118904/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 27th, 2014, 5:19 a.m. UTC, <b>David Narváez</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;">Do you need to take the perpendicular bisector construction out of this patch now that the SegmentAxisABType implementation was changed?</pre>
</blockquote>
<p>On June 27th, 2014, 8:43 a.m. UTC, <b>Aniket Anvit</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 will update the patch. I wanted to ask if you like the idea of having that switch-case in endElement(). We need some way to extra handling in case of many objects. </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;">It looks suspicious indeed, and I see it is related to the perpendicular bisector and the way we interpret the parents (whether we are bisecting a segment object or a segment defined by two points) which makes me think this should be handled in the object constructor. But your comments around that switch statement seem to indicate you have other examples of objects that require that handling, I would like to see those too.
In any case, it looks like removing support for perpendicular bisector in this patch removes the need to merge the switch case here and we can have this discussion later, am I right?</pre>
<br />
<p>- David</p>
<br />
<p>On June 23rd, 2014, 5:01 p.m. UTC, Aniket Anvit 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 and David Narváez.</div>
<div>By Aniket Anvit.</div>
<p style="color: grey;"><i>Updated June 23, 2014, 5:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kig
</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;">This patch adds support for Geogebra's Polygon-by-n-points, Perpendicular-Bisector( both with two points as input and a segment as input ), Reflection-in-point, Reflection-in-Line objects in Kig.</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;">Tested with multiple objects of types- polygons, perpendicular-bisectors, reflections.</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>filters/geogebra-filter.h <span style="color: grey">(ebf5713)</span></li>
<li>filters/geogebra-filter.cpp <span style="color: grey">(d40f976)</span></li>
<li>filters/geogebra.xsl <span style="color: grey">(668d758)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118904/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>