Review Request: Introduce KoTextRange instead of inline characters

[C. Boemann]

On Thursday 25 October 2012 10:21:15 Pierre Stirnweiss wrote:
> 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
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.