On Thursday 25 October 2012 10:21:15 Pierre Stirnweiss wrote: > On Thu, Oct 25, 2012 at 9:39 AM, Inge Wallin <[email protected]> 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 > > [email protected] > > 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 I definitely think it's safe to to merge. after all it's just a beta, and I'll be doing thorough testing over the coming weeks. And it's already a huge improvement over the brittle current system
As for undo, yes, indeed it doesn't restore them to be ui visible. I'll fix that before merging. _______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
