<br><br><div class="gmail_quote">On Thu, Oct 25, 2012 at 9:39 AM, Inge Wallin <span dir="ltr"><<a href="mailto:inge@lysator.liu.se" target="_blank">inge@lysator.liu.se</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif"><div class="im">
<table style="border:1px #c9c399 solid" width="100%" bgcolor="#f9f3c9" cellpadding="8">
<tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/106983/" target="_blank">http://git.reviewboard.kde.org/r/106983/</a>
</td>
</tr>
</tbody></table>
<br>
</div><p>Ship it!</p>
<pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">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.</pre>
<br>
<p>- Inge</p><div class="im">
<br>
<p>On October 24th, 2012, 11:44 a.m., C. Boemann wrote:</p>
</div><table style="background-image:url('');background-repeat:repeat-x;border:1px black solid" width="100%" bgcolor="#fefadf" cellpadding="8" cellspacing="0">
<tbody><tr>
<td><div class="im">
<div>Review request for Calligra.</div>
<div>By C. Boemann.</div>
<p style="color:grey"><i>Updated Oct. 24, 2012, 11:44 a.m.</i></p>
</div><h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1><div class="im">
<table style="border:1px solid #b8b5a0" width="100%" bgcolor="#ffffff" cellpadding="10" cellspacing="0">
<tbody><tr>
<td>
<pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">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</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
</div><div><div class="h5"><ul style="margin-left:3em;padding-left:0">
<li>libs/kotext/CMakeLists.txt <span style="color:grey">(8c7d976)</span></li>
<li>libs/kotext/KoBookmark.h <span style="color:grey">(3591e9d)</span></li>
<li>libs/kotext/KoBookmark.cpp <span style="color:grey">(c8045fe)</span></li>
<li>libs/kotext/KoBookmarkManager.h <span style="color:grey">(08006ce)</span></li>
<li>libs/kotext/KoBookmarkManager.cpp <span style="color:grey">(b2a4ea3)</span></li>
<li>libs/kotext/KoInlineTextObjectManager.h <span style="color:grey">(56ce7cd)</span></li>
<li>libs/kotext/KoInlineTextObjectManager.cpp <span style="color:grey">(e82664d)</span></li>
<li>libs/kotext/KoText.h <span style="color:grey">(6eb02ab)</span></li>
<li>libs/kotext/KoTextDebug.cpp <span style="color:grey">(24cb4a0)</span></li>
<li>libs/kotext/KoTextDocument.h <span style="color:grey">(76d0a5e)</span></li>
<li>libs/kotext/KoTextDocument.cpp <span style="color:grey">(9e8727b)</span></li>
<li>libs/kotext/KoTextEditor.h <span style="color:grey">(67458e7)</span></li>
<li>libs/kotext/KoTextEditor.cpp <span style="color:grey">(0c5463d)</span></li>
<li>libs/kotext/KoTextInlineRdf.cpp <span style="color:grey">(eaf0ff7)</span></li>
<li>libs/kotext/KoTextRange.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>libs/kotext/KoTextRange.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>libs/kotext/KoTextRangeManager.h <span style="color:grey">(PRE-CREATION)</span></li>
<li>libs/kotext/KoTextRangeManager.cpp <span style="color:grey">(PRE-CREATION)</span></li>
<li>libs/kotext/commands/DeleteCommand.h <span style="color:grey">(036914b)</span></li>
<li>libs/kotext/commands/DeleteCommand.cpp <span style="color:grey">(d95b806)</span></li>
<li>libs/kotext/opendocument/KoTextLoader.cpp <span style="color:grey">(e373785)</span></li>
<li>libs/kotext/opendocument/KoTextWriter_p.cpp <span style="color:grey">(7667d8e)</span></li>
<li>libs/kotext/tests/TestKoBookmarkManager.h <span style="color:grey">(131eea7)</span></li>
<li>libs/kotext/tests/TestKoBookmarkManager.cpp <span style="color:grey">(24d27f8)</span></li>
<li>libs/kotext/tests/TestKoInlineTextObjectManager.h <span style="color:grey">(21c6ff9)</span></li>
<li>libs/kotext/tests/TestKoInlineTextObjectManager.cpp <span style="color:grey">(0c606b9)</span></li>
<li>libs/kotext/tests/TestKoTextEditor.cpp <span style="color:grey">(dde79cd)</span></li>
<li>libs/main/rdf/KoDocumentRdf.cpp <span style="color:grey">(dfbaf09)</span></li>
<li>libs/main/tests/TestKoDocumentRdf.cpp <span style="color:grey">(8fb279c)</span></li>
<li>libs/main/tests/rdf_test.h <span style="color:grey">(778fcbb)</span></li>
<li>libs/main/tests/rdf_test.cpp <span style="color:grey">(7550d3f)</span></li>
<li>libs/textlayout/KoPointedAt.h <span style="color:grey">(0924f0d)</span></li>
<li>libs/textlayout/KoPointedAt.cpp <span style="color:grey">(ac6c88a)</span></li>
<li>libs/textlayout/KoTextDocumentLayout.h <span style="color:grey">(ee317d0)</span></li>
<li>libs/textlayout/KoTextDocumentLayout.cpp <span style="color:grey">(ee6f31c)</span></li>
<li>libs/textlayout/KoTextLayoutArea.cpp <span style="color:grey">(b184b79)</span></li>
<li>libs/textlayout/ToCGenerator.h <span style="color:grey">(674a413)</span></li>
<li>libs/textlayout/ToCGenerator.cpp <span style="color:grey">(b7585ef)</span></li>
<li>plugins/textshape/TextShape.h <span style="color:grey">(75e985c)</span></li>
<li>plugins/textshape/TextShape.cpp <span style="color:grey">(574c493)</span></li>
<li>plugins/textshape/TextShapeFactory.cpp <span style="color:grey">(7e8d7b6)</span></li>
<li>plugins/textshape/TextTool.cpp <span style="color:grey">(be9f2eb)</span></li>
<li>plugins/textshape/dialogs/BibliographyPreview.h <span style="color:grey">(05d4560)</span></li>
<li>plugins/textshape/dialogs/BibliographyPreview.cpp <span style="color:grey">(e555121)</span></li>
<li>plugins/textshape/dialogs/SimpleParagraphWidget.cpp <span style="color:grey">(7246f7a)</span></li>
<li>plugins/textshape/dialogs/TableOfContentsPreview.h <span style="color:grey">(ede723a)</span></li>
<li>plugins/textshape/dialogs/TableOfContentsPreview.cpp <span style="color:grey">(1f24270)</span></li>
<li>words/part/KWDocument.h <span style="color:grey">(16a760c)</span></li>
<li>words/part/KWDocument.cpp <span style="color:grey">(ac734be)</span></li>
<li>words/part/KWView.cpp <span style="color:grey">(65f6165)</span></li>
<li>words/part/frames/KWTextFrameSet.cpp <span style="color:grey">(35f4d8e)</span></li>
<li>words/part/tests/TestKoBookmark.h <span style="color:grey">(3620385)</span></li>
<li>words/part/tests/TestKoBookmark.cpp <span style="color:grey">(cfb65bc)</span></li>
<li>words/part/tests/TestRdf.cpp <span style="color:grey">(b1cf93c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106983/diff/" style="margin-left:3em" target="_blank">View Diff</a></p>
</div></div></td>
</tr>
</tbody></table>
</div>
</div>
<br>_______________________________________________<br>
calligra-devel mailing list<br>
<a href="mailto:calligra-devel@kde.org">calligra-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/calligra-devel" target="_blank">https://mail.kde.org/mailman/listinfo/calligra-devel</a><br>
<br></blockquote></div><br>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.<br>
I probably won't have time to do full review this evening (especially not if we want to merge in the style sorting).<br>In any case, is it really wise to merge such a big refactoring so close to release?<br><br>PierreSt<br>