----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108865/#review27068 -----------------------------------------------------------
Ship it! although cstester showed a lot of changes (zagge and i believe it to be all ranom cstester issues) so good work! - C. Boemann 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
