<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/108865/">http://git.reviewboard.kde.org/r/108865/</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;">This review has been submitted with commit b385fa11e85afb8337e1c5b10929e8add288f42f by Inge Wallin to branch master.</pre>
<br />
<p>- Commit</p>
<br />
<p>On February 8th, 2013, 7:38 p.m. UTC, Inge Wallin 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 Calligra.</div>
<div>By Inge Wallin.</div>
<p style="color: grey;"><i>Updated Feb. 8, 2013, 7:38 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;">This is the first step in a multi-step plan to unify the handling of
borders inside Calligra. There are currently at least 3 different
implementations of borders and the painting of borders which seems
pretty wasteful to me. I am sure that KoBorder could be used in many
more places than now, which would both increase the functionality as
well as make the code simpler.
The plan looks something like this:
1. Simplify the API of KoBorder
2. Simplify the code that uses KoBorder in other places given the now
simpler API.
3. Move painting of page borders in Words from KWCanvasBase into
KoBorder.
4. Investigate other places in Calligra that handles borders and port
them to using KoBorder. Examples include cell borders in Sheets
paragraph borders in text and cell borders in tables
This review is step 1 in this plan. I wanted to get a review before I
continue so that I can get feedback on the plan as well as if I'm on
the right track with this approach.
The way that the simplified API results in simpler and shorter code
can be seen in e.g. KoTextEditor.cpp or KoTableCellStyle.cpp. It's
simply much easier to use and it will lend it self to compacting many
places where there are repeated code segments into simpler loops. That
is step 2 in the plan.
Finally, I have also been thinking of changing the names of the sides
in KoBorder to unify them with other border implementations. This
would also make it clearer. I thought these names would be good
(importing some better names from other places):
Top -> TopBorder
Bottom -> BottomBorder
Left -> LeftBorder
Right -> RightBorder
TopLeftToBottomRight -> TlbrBorder
BottomLeftToTopRight -> BLtrBorder
In some other places the word "Edge" is used instead of "Side". I'm
not sure which one is best but it should definitely be the same
everywhere.
Comments?
</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 all the test documents that I have with the string "border" in its name. I couldn't find any regressions. But a change like this could perhaps use a biggish run of cstester...</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/libmsooxml/MsooXmlTableStyle.cpp <span style="color: grey">(09b8598)</span></li>
<li>libs/kotext/KoTextEditor.cpp <span style="color: grey">(98f22b8)</span></li>
<li>libs/kotext/styles/KoTableCellStyle.cpp <span style="color: grey">(2a9ae4e)</span></li>
<li>libs/kotext/styles/tests/TestTableCellStyle.cpp <span style="color: grey">(7db3fbc)</span></li>
<li>libs/odf/KoBorder.h <span style="color: grey">(fd7e5cc)</span></li>
<li>libs/odf/KoBorder.cpp <span style="color: grey">(78e35e9)</span></li>
<li>libs/odf/KoPageLayout.cpp <span style="color: grey">(4a825f3)</span></li>
<li>words/part/KWCanvasBase.cpp <span style="color: grey">(46ca518)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108865/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>