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