Review Request 108865: First step in a cleanup of all border handling

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108865/#review27184
-----------------------------------------------------------


This review has been submitted with commit b385fa11e85afb8337e1c5b10929e8add288f42f by Inge Wallin to branch master.

- Commit Hook


On Feb. 8, 2013, 7:38 p.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108865/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2013, 7:38 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> 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?
> 
> 
> Diffs
> -----
> 
>   filters/libmsooxml/MsooXmlTableStyle.cpp 09b8598 
>   libs/kotext/KoTextEditor.cpp 98f22b8 
>   libs/kotext/styles/KoTableCellStyle.cpp 2a9ae4e 
>   libs/kotext/styles/tests/TestTableCellStyle.cpp 7db3fbc 
>   libs/odf/KoBorder.h fd7e5cc 
>   libs/odf/KoBorder.cpp 78e35e9 
>   libs/odf/KoPageLayout.cpp 4a825f3 
>   words/part/KWCanvasBase.cpp 46ca518 
> 
> Diff: http://git.reviewboard.kde.org/r/108865/diff/
> 
> 
> Testing
> -------
> 
> 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...
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130211/f67a5fa2/attachment.htm>