Review Request 108459: Refactor in KoText

[C. Boemann]

> On Jan. 25, 2013, 4:25 a.m., Thorsten Zachmann wrote:
> > libs/kotext/KoAnchorInlineObject.h, line 96
> > <http://git.reviewboard.kde.org/r/108459/diff/7/?file=108921#file108921line96>
> >
> >     This function should be const.

ah but it's reimplemented so will have to be changed in many other places too then


> On Jan. 25, 2013, 4:25 a.m., Thorsten Zachmann wrote:
> > libs/kotext/opendocument/KoTextWriter_p.cpp, lines 861-882
> > <http://git.reviewboard.kde.org/r/108459/diff/7/?file=108944#file108944line861>
> >
> >     Is that a change belonging to this work?

Not really but once the change tracking branch is removing changetracking code, this will make it much easier to merge - and this code has never been called since before 2.4


> On Jan. 25, 2013, 4:25 a.m., Thorsten Zachmann wrote:
> > libs/kotext/commands/ChangeAnchorPropertiesCommand.cpp, line 49
> > <http://git.reviewboard.kde.org/r/108459/diff/7/?file=108934#file108934line49>
> >
> >     Should that not delete the m_newLocation/m_oldLocation depending on which one is set when destructing to avoid a memory leak?

I guess but then we risk double deletion, so I'd rather just let the system clean it up


- C.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108459/#review26151
-----------------------------------------------------------


On Jan. 23, 2013, 9:35 a.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108459/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2013, 9:35 a.m.)
> 
> 
> Review request for Calligra and Thorsten Zachmann.
> 
> 
> Description
> -------
> 
>     Change KoTextAnchor into two new classes
>      KoShapeANchor - which in theory should be moved to flake later
>      KoAnchorInlineObject - which is the kotext part responsible for being an inline object
>     
>     No realy user visible changes coming out of this but it's nessesary ground work for the next step
>     
>     I have had to disable several commands so we loos that functionality
>     This will be reenabled with the following commits which adds a third class
> 
> 
> Diffs
> -----
> 
>   libs/kotext/CMakeLists.txt 8b358ec 
>   libs/kotext/KoAnchorInlineObject.h PRE-CREATION 
>   libs/kotext/KoAnchorInlineObject.cpp PRE-CREATION 
>   libs/kotext/KoAnchorTextRange.h PRE-CREATION 
>   libs/kotext/KoAnchorTextRange.cpp PRE-CREATION 
>   libs/kotext/KoShapeAnchor.h PRE-CREATION 
>   libs/kotext/KoShapeAnchor.cpp PRE-CREATION 
>   libs/kotext/KoTextAnchor.h 4ec3f03 
>   libs/kotext/KoTextAnchor.cpp 1f2e8c5 
>   libs/kotext/KoTextEditor.h 10c59a6 
>   libs/kotext/KoTextEditor.cpp 1929e07 
>   libs/kotext/KoTextRange.h 78515d9 
>   libs/kotext/KoTextRange.cpp 61cbfa2 
>   libs/kotext/commands/ChangeAnchorPropertiesCommand.h f1448ad 
>   libs/kotext/commands/ChangeAnchorPropertiesCommand.cpp 50e9f55 
>   libs/kotext/commands/ChangeTrackedDeleteCommand.cpp 966d990 
>   libs/kotext/commands/DeleteAnchorsCommand.h d98b858 
>   libs/kotext/commands/DeleteAnchorsCommand.cpp d32f5f1 
>   libs/kotext/commands/DeleteCommand.h a0004a6 
>   libs/kotext/commands/DeleteCommand.cpp 5d39f4f7 
>   libs/kotext/opendocument/KoTextLoader.cpp 29165e1 
>   libs/kotext/opendocument/KoTextSharedLoadingData.h b247992 
>   libs/kotext/opendocument/KoTextSharedLoadingData.cpp ff609d1 
>   libs/kotext/opendocument/KoTextWriter_p.h f3466e1 
>   libs/kotext/opendocument/KoTextWriter_p.cpp 39cf629 
>   libs/textlayout/AnchorStrategy.h f0853f1 
>   libs/textlayout/AnchorStrategy.cpp a7dbec1 
>   libs/textlayout/FloatingAnchorStrategy.h 090efea 
>   libs/textlayout/FloatingAnchorStrategy.cpp 1364df1 
>   libs/textlayout/InlineAnchorStrategy.h 24d3051 
>   libs/textlayout/InlineAnchorStrategy.cpp c21f62b 
>   libs/textlayout/KoTextDocumentLayout.h ef7c197 
>   libs/textlayout/KoTextDocumentLayout.cpp dcb4c8a 
>   libs/textlayout/KoTextLayoutArea.cpp 4f96f19 
>   libs/textlayout/KoTextShapeContainerModel.h 93b8a4d 
>   libs/textlayout/KoTextShapeContainerModel.cpp f19efa5 
>   libs/textlayout/KoTextShapeData.cpp 9d096ef 
>   plugins/textshape/commands/ShowChangesCommand.cpp a486c113 
>   words/part/KWDocument.h dc64e5e 
>   words/part/KWDocument.cpp 67f808d 
>   words/part/KWOdfSharedLoadingData.h 7893cb9 
>   words/part/KWOdfSharedLoadingData.cpp 58f533b 
>   words/part/KWOdfWriter.cpp c9c72f7 
>   words/part/KWRootAreaProvider.cpp 9a86d5d 
>   words/part/KWView.h e86cc5c 
>   words/part/KWView.cpp c9405df 
>   words/part/commands/KWShapeCreateCommand.h a6e2852 
>   words/part/commands/KWShapeCreateCommand.cpp 4781c390 
>   words/part/dialogs/KWAnchoringProperties.h 14ac2d4 
>   words/part/dialogs/KWAnchoringProperties.cpp fe97a23 
>   words/part/frames/KWFrame.h 757c4ec 
>   words/part/frames/KWFrame.cpp e30e8f0 
>   words/part/frames/KWFrameLayout.cpp 5ad85ed 
>   words/part/frames/KWTextFrameSet.cpp 1088541 
> 
> Diff: http://git.reviewboard.kde.org/r/108459/diff/
> 
> 
> Testing
> -------
> 
> I've create two small documents and they load fine.
> 
> I'd like Thorsten to run cstester on this to be sure
> 
> This review is not for final approval - as i need a second step, but it's a nice logical point to do a review
> 
> In several commands i've disabled some stuff because it would be a vaste of time to fix it only to break it again with part2.
> But loading and saving and display should be just as good as before, and the code so far should make sense - so please review that
> 
> 
> Thanks,
> 
> C. Boemann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130126/6d4e81f4/attachment.htm>