Review Request 108459: Refactor in KoText
C. Boemann
cbr at boemann.dk
Mon Jan 21 06:59:45 GMT 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108459/#review25878
-----------------------------------------------------------
libs/kotext/KoAnchorInlineObject.h
<http://git.reviewboard.kde.org/r/108459/#comment19754>
this needs to be updated
libs/kotext/KoAnchorInlineObject.h
<http://git.reviewboard.kde.org/r/108459/#comment19755>
not needed
libs/kotext/KoAnchorInlineObject.h
<http://git.reviewboard.kde.org/r/108459/#comment19756>
Use the new description in koanchortextrange as basis for this description
libs/kotext/KoAnchorInlineObject.h
<http://git.reviewboard.kde.org/r/108459/#comment19757>
move to .cpp
libs/kotext/KoAnchorInlineObject.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19758>
is this a new regression?
libs/kotext/KoAnchorInlineObject.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19759>
let's make this method empty - there is nothing in it for us anymore
libs/kotext/KoAnchorTextRange.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19760>
hmm but this is no longer true - possible regression ?
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19761>
Review this description
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19762>
this method really needs a better name
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19763>
rename to just document()
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19764>
oh really - we don't support that at all
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19765>
here we dont need KoShapeAnchor::
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19766>
can we hide this somehow
not nice to have in public api
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19773>
document we don't take owner ship -or should we?
libs/kotext/KoShapeAnchor.h
<http://git.reviewboard.kde.org/r/108459/#comment19772>
document we take ownership and will delete it when done with it
libs/kotext/KoShapeAnchor.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19767>
call this distance like it's getter/setter og or rename offset() to distance()
libs/kotext/KoShapeAnchor.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19768>
or not parented at all like anchor to page
how do we save that - bug?
libs/kotext/KoShapeAnchor.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19769>
do this for all types - document in .h
don't repeat this in textloader
libs/kotext/KoShapeAnchor.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19770>
we should probably extend this to make sure the other type changes don't leave tiih invalid rel and pos
libs/kotext/KoShapeAnchor.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19771>
see there is nothing that binds this here
libs/kotext/commands/ChangeAnchorPropertiesCommand.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19774>
we definitely need to fix this
libs/kotext/commands/ChangeAnchorPropertiesCommand.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19775>
and fix this too
libs/kotext/commands/DeleteCommand.h
<http://git.reviewboard.kde.org/r/108459/#comment19776>
fix spelling while we are at it
libs/kotext/opendocument/KoTextLoader.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19777>
this can be removed as loadOdf will do that from now on
libs/kotext/opendocument/KoTextLoader.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19778>
this can be removed as loadodf will do this from now on
libs/textlayout/AnchorStrategy.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19779>
we need better documention here
libs/textlayout/FloatingAnchorStrategy.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19780>
let's use position() instead
libs/textlayout/FloatingAnchorStrategy.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19781>
let's use position() instead
libs/textlayout/FloatingAnchorStrategy.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19782>
let's use position() instead
libs/textlayout/KoTextDocumentLayout.h
<http://git.reviewboard.kde.org/r/108459/#comment19783>
update apidox to reflect what it does
libs/textlayout/KoTextDocumentLayout.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19784>
update comment
libs/textlayout/KoTextDocumentLayout.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19785>
comment that this is cleared regularly so will be entered whenever te page is relayouted (but not for the inner placement loops obviously)
words/part/dialogs/KWAnchoringProperties.h
<http://git.reviewboard.kde.org/r/108459/#comment19786>
let's remove this then
words/part/dialogs/KWAnchoringProperties.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19787>
maybe time to implemet this
words/part/dialogs/KWAnchoringProperties.cpp
<http://git.reviewboard.kde.org/r/108459/#comment19788>
rephrase
- C. Boemann
On Jan. 21, 2013, 5:34 a.m., C. Boemann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108459/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2013, 5:34 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/KoAnchorInlineObject.cpp PRE-CREATION
> libs/kotext/KoAnchorInlineObject.h PRE-CREATION
> libs/kotext/CMakeLists.txt 8b358ec
> 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 4d4cbcf
> 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
>
> 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/20130121/83cda075/attachment.htm>
More information about the calligra-devel
mailing list