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