On Thu, Oct 25, 2012 at 9:39 AM, Inge Wallin <i...@lysator.liu.se> wrote:
> This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106983/ > > Ship it! > > Looks good now. > > Only thing was that copyright notice is still missing in some files and some > files still have code that is commented away. > > Why don't I point out the exact places? Because I got server errors from the > review board and it had to be fixed by removing the review and I don't have > enough energy to go through the whole thing again. Anyway it was just > trivialities, just go ahead and merge. > > > - Inge > > On October 24th, 2012, 11:44 a.m., C. Boemann wrote: > Review request for Calligra. > By C. Boemann. > > *Updated Oct. 24, 2012, 11:44 a.m.* > Description > > Using inline characters for things like anchors, bookmarks, annotations, > softbreaks means that the inlinecharacter show up like invisible characters. > > This is very undesirably, and prone to all kinds of bugs. Qt has a clas that > already handles this mostly correctly: QTextCursor. We just need to put our > own handling on top. > > This up our requirement to qt 4.7 because we need a special feature, and > there is even a bug in that feature i had to work around for the time being. > > I suspect lots of changes is needed before it's mergable, and expect a > thorough review by interested parties > > Diffs > > - libs/kotext/CMakeLists.txt (8c7d976) > - libs/kotext/KoBookmark.h (3591e9d) > - libs/kotext/KoBookmark.cpp (c8045fe) > - libs/kotext/KoBookmarkManager.h (08006ce) > - libs/kotext/KoBookmarkManager.cpp (b2a4ea3) > - libs/kotext/KoInlineTextObjectManager.h (56ce7cd) > - libs/kotext/KoInlineTextObjectManager.cpp (e82664d) > - libs/kotext/KoText.h (6eb02ab) > - libs/kotext/KoTextDebug.cpp (24cb4a0) > - libs/kotext/KoTextDocument.h (76d0a5e) > - libs/kotext/KoTextDocument.cpp (9e8727b) > - libs/kotext/KoTextEditor.h (67458e7) > - libs/kotext/KoTextEditor.cpp (0c5463d) > - libs/kotext/KoTextInlineRdf.cpp (eaf0ff7) > - libs/kotext/KoTextRange.h (PRE-CREATION) > - libs/kotext/KoTextRange.cpp (PRE-CREATION) > - libs/kotext/KoTextRangeManager.h (PRE-CREATION) > - libs/kotext/KoTextRangeManager.cpp (PRE-CREATION) > - libs/kotext/commands/DeleteCommand.h (036914b) > - libs/kotext/commands/DeleteCommand.cpp (d95b806) > - libs/kotext/opendocument/KoTextLoader.cpp (e373785) > - libs/kotext/opendocument/KoTextWriter_p.cpp (7667d8e) > - libs/kotext/tests/TestKoBookmarkManager.h (131eea7) > - libs/kotext/tests/TestKoBookmarkManager.cpp (24d27f8) > - libs/kotext/tests/TestKoInlineTextObjectManager.h (21c6ff9) > - libs/kotext/tests/TestKoInlineTextObjectManager.cpp (0c606b9) > - libs/kotext/tests/TestKoTextEditor.cpp (dde79cd) > - libs/main/rdf/KoDocumentRdf.cpp (dfbaf09) > - libs/main/tests/TestKoDocumentRdf.cpp (8fb279c) > - libs/main/tests/rdf_test.h (778fcbb) > - libs/main/tests/rdf_test.cpp (7550d3f) > - libs/textlayout/KoPointedAt.h (0924f0d) > - libs/textlayout/KoPointedAt.cpp (ac6c88a) > - libs/textlayout/KoTextDocumentLayout.h (ee317d0) > - libs/textlayout/KoTextDocumentLayout.cpp (ee6f31c) > - libs/textlayout/KoTextLayoutArea.cpp (b184b79) > - libs/textlayout/ToCGenerator.h (674a413) > - libs/textlayout/ToCGenerator.cpp (b7585ef) > - plugins/textshape/TextShape.h (75e985c) > - plugins/textshape/TextShape.cpp (574c493) > - plugins/textshape/TextShapeFactory.cpp (7e8d7b6) > - plugins/textshape/TextTool.cpp (be9f2eb) > - plugins/textshape/dialogs/BibliographyPreview.h (05d4560) > - plugins/textshape/dialogs/BibliographyPreview.cpp (e555121) > - plugins/textshape/dialogs/SimpleParagraphWidget.cpp (7246f7a) > - plugins/textshape/dialogs/TableOfContentsPreview.h (ede723a) > - plugins/textshape/dialogs/TableOfContentsPreview.cpp (1f24270) > - words/part/KWDocument.h (16a760c) > - words/part/KWDocument.cpp (ac734be) > - words/part/KWView.cpp (65f6165) > - words/part/frames/KWTextFrameSet.cpp (35f4d8e) > - words/part/tests/TestKoBookmark.h (3620385) > - words/part/tests/TestKoBookmark.cpp (cfb65bc) > - words/part/tests/TestRdf.cpp (b1cf93c) > > View Diff <http://git.reviewboard.kde.org/r/106983/diff/> > > _______________________________________________ > calligra-devel mailing list > calligra-devel@kde.org > https://mail.kde.org/mailman/listinfo/calligra-devel > > I haven't taken time to review in deep detail since i understood it would not be for right now. Has the impact on undo/redo been tested? From the initial version of DeleteCommand, I had the impression that undo/redo would be messed up a bit. The doDelete would delete text ranges, but I have not seen anything adding them again in the manager, for example. I probably won't have time to do full review this evening (especially not if we want to merge in the style sorting). In any case, is it really wise to merge such a big refactoring so close to release? PierreSt
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel