----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108913/#review27237 -----------------------------------------------------------
Ship it! The xmlWriter & mainStyles parameters pre-date the introduction of OdfSavingContext (and KoShapeSavingContext), so yeah, I think it is a good idea to clean this up a bit. - Marijn Kruisselbrink On Feb. 11, 2013, 8:20 p.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108913/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2013, 8:20 p.m.) > > > Review request for Calligra and Marijn Kruisselbrink. > > > Description > ------- > > It confused me when I read the code of saving a Sheet document to ODF: > "KoXmlWriter& xmlWriter" and "KoGenStyles &mainStyles" are passed explicitly > as parameters to Sheet::saveOdfColRowCell(...), Sheet::saveOdfCells(...), and > Cell::saveOdf(...), even though they are already members of the also passed > "OdfSavingContext& tableContext". By just looking at the method it seemed > that xmlWriter and mainStyles were different objects to those which are part > of tableContext, which has such objects via the shapeContext member. Only > when going up the callstack it could be found that these references are just > duplicates. > > This confusion seems also the reason why in Cell::saveOdf(...) a shapeContext > was recreated with these objects, instead of using the shapeContext of > tableContext. > > Attached patch changes the signatures of those methods to only pass > "OdfSavingContext& tableContext", like also done in similar methods, and to > simply create helper references to xmlWriter and mainStyles in the method > bodies, as needed. It also removes the unneeded additional creation of a > shapeContext. > > Resulting code should be less confusing :) > > (Came across the code during of my currently done big tuning of > KoOdfWriteStore) > > > Diffs > ----- > > sheets/Sheet.cpp 1b11a6f > sheets/Sheet.h 9d9f1a7 > sheets/Cell.cpp fef7846 > sheets/Cell.h 015e847 > > Diff: http://git.reviewboard.kde.org/r/108913/diff/ > > > Testing > ------- > > all Sheets tests pass as before, richtext in cell roundtrip also worked > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
