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

Inge Wallin inge at lysator.liu.se
Fri Feb 8 19:38:35 GMT 2013


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

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/20130208/6635671d/attachment.htm>


More information about the calligra-devel mailing list