Review Request: Introduce KoTextRange instead of inline characters

[Pierre Stirnweiss]

On Thu, Oct 25, 2012 at 9:39 AM, Inge Wallin <inge at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20121025/c1044bb2/attachment.htm>