----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122238/#review74674 -----------------------------------------------------------
I didn't understand all of the code, but I made some comment. I would be much more comfortable if the ChangeStyleCommand stored the changes in the actual styles instead of the changes in the low-level internal representation that we use to apply the styles. But I understand that that's a rest of the previous implementation and shouldn't be changed now. libs/kotext/OdfTextTrackStyles.h <https://git.reviewboard.kde.org/r/122238/#comment51754> That is a totally unsatisfying class doc. I have read it all and I still have no clue what the class does. libs/kotext/OdfTextTrackStyles.h <https://git.reviewboard.kde.org/r/122238/#comment51755> heh, here it was. Better move this up to the actual class doc. I also still think that the use of the term "document" is confusing in this context where it can mean either QTextDocument or ODF document. I even believe it means different things in different sentences here. Can we clarify the terminology? The last sentence confuses me more than it clarifies me. :) libs/kotext/OdfTextTrackStyles.cpp <https://git.reviewboard.kde.org/r/122238/#comment51757> So instead of a few methods in the style manager we have both a TrackStyles object and a helper class? Is this really necessary? The user of the word "Helper" in calligra has always struck me as very vague. Can we find a more concrete name? libs/kotext/commands/ChangeStylesCommand.h <https://git.reviewboard.kde.org/r/122238/#comment51759> This seems strange to me. Why would a style change store a single QTextDocument? An ODF style change affects all the ODF document with several QTextDocuments. libs/kotext/commands/ChangeStylesCommand.h <https://git.reviewboard.kde.org/r/122238/#comment51761> Can we have an explanation what this struct is good for? libs/kotext/commands/ChangeStylesMacroCommand.cpp <https://git.reviewboard.kde.org/r/122238/#comment51762> This should be commented out - Inge Wallin On Jan. 24, 2015, 8:27 p.m., Camilla Boemann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122238/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2015, 8:27 p.m.) > > > Review request for Calligra. > > > Repository: calligra > > > Description > ------- > > Make KoStyleManager independent of rest of libs/kotext - making it possible > to move it to libs/odf > also fixes a bug that stylechanges were never applied to the document > > > Diffs > ----- > > libs/kotext/CMakeLists.txt bfb0b2d > libs/kotext/KoTextDocument.cpp 07a47dc > libs/kotext/OdfTextTrackStyles.h PRE-CREATION > libs/kotext/OdfTextTrackStyles.cpp PRE-CREATION > libs/kotext/commands/ChangeStylesCommand.h 3762715 > libs/kotext/commands/ChangeStylesCommand.cpp 771caae > libs/kotext/commands/ChangeStylesMacroCommand.h 340ba12 > libs/kotext/commands/ChangeStylesMacroCommand.cpp 66f5326 > libs/kotext/styles/ChangeFollower.h 5786541 > libs/kotext/styles/ChangeFollower.cpp 7930cfe > libs/kotext/styles/KoStyleManager.h e1b400c > libs/kotext/styles/KoStyleManager.cpp 86108d6 > libs/textlayout/KoTextShapeData.cpp c51ec6e > sheets/Map.cpp a4fcc7c > > Diff: https://git.reviewboard.kde.org/r/122238/diff/ > > > Testing > ------- > > only basic editing and seeing that undo/redo works and that changes are now > actually appied to the document > > > Thanks, > > Camilla Boemann > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
