Review Request: Complete support for ODF glue points

Thorsten Zachmann t.zachmann at zagge.de
Thu Jan 27 06:53:23 CET 2011


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


Very nice and impressive work. That are some comments on the code when looking at it. I will test it once it is compiled and then give more feedback.


libs/flake/KoConnectionPoint.h
<http://git.reviewboard.kde.org/r/100447/#comment891>

    The point are not stored in shape coordinates but in percentage value of the size



libs/flake/KoConnectionShape.cpp
<http://git.reviewboard.kde.org/r/100447/#comment886>

    Please define as const



libs/flake/KoTextOnShapeContainer.cpp
<http://git.reviewboard.kde.org/r/100447/#comment892>

    You are sure this should be committed? Yes I know with KoTextOnShapeContainer some of the stuff does not work



plugins/defaultTools/connectionTool/ConnectionTool.h
<http://git.reviewboard.kde.org/r/100447/#comment893>

    this method should be const



plugins/defaultTools/connectionTool/ConnectionTool.h
<http://git.reviewboard.kde.org/r/100447/#comment894>

    shapes should be passed as const &
    Can the function be made const?



plugins/defaultTools/connectionTool/ConnectionTool.h
<http://git.reviewboard.kde.org/r/100447/#comment895>

    please make the function const.



plugins/defaultTools/connectionTool/ConnectionTool.cpp
<http://git.reviewboard.kde.org/r/100447/#comment896>

    there should be a blank after the if



plugins/defaultTools/connectionTool/ConnectionTool.cpp
<http://git.reviewboard.kde.org/r/100447/#comment897>

    Can the function made const?


- Thorsten


On Jan. 26, 2011, 10:48 p.m., Jan Hambrecht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100447/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2011, 10:48 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> This patch properly implements support for odf glue points (=connection points in flake) as defined in the ODF specification.
> The following are some of the main points tackled:
> 
> * a new struct KoConnectionPoint which holds the relevant data of a connection point
> * some new API in KoShape to handle connection points
> * loading of custom connection points and their attributes like escape direction and align
> * saving of connection points
> * huge improvement to the connection tool which now can also create, delete and edit connection points
> 
> 
> This addresses bug 251529.
>     http://bugs.kde.org/show_bug.cgi?id=251529
> 
> 
> Diffs
> -----
> 
>   kexi/shapes/relationdesign/kexirelationdesignshape.cpp 27604d4 
>   libs/flake/CMakeLists.txt 0447004 
>   libs/flake/KoConnectionPoint.h PRE-CREATION 
>   libs/flake/KoConnectionPoint.cpp PRE-CREATION 
>   libs/flake/KoConnectionShape.h 862d41b 
>   libs/flake/KoConnectionShape.cpp 8430eb0 
>   libs/flake/KoConnectionShapeLoadingUpdater.cpp 0bd834c 
>   libs/flake/KoConnectionShape_p.h ea3b0ba 
>   libs/flake/KoOdfWorkaround.h e6a5c63 
>   libs/flake/KoOdfWorkaround.cpp 36fd86b 
>   libs/flake/KoShape.h f7179d7 
>   libs/flake/KoShape.cpp c5aee86 
>   libs/flake/KoShape_p.h f52ba8c 
>   libs/flake/KoTextOnShapeContainer.cpp 07921ca 
>   libs/flake/tools/KoParameterChangeStrategy.h a10e54f 
>   libs/flake/tools/KoParameterChangeStrategy.cpp 925602f 
>   libs/flake/tools/KoParameterChangeStrategy_p.h PRE-CREATION 
>   libs/flake/tools/KoPathConnectionPointStrategy.h 2ec9467 
>   libs/flake/tools/KoPathConnectionPointStrategy.cpp 15764bc 
>   libs/flake/tools/KoPathConnectionPointStrategy_p.h PRE-CREATION 
>   libs/flake/tools/KoPathTool.h c863f11 
>   libs/flake/tools/KoPathTool.cpp fe9351a 
>   plugins/defaultTools/CMakeLists.txt 7bd62ce 
>   plugins/defaultTools/connectionTool/AddConnectionPointCommand.h PRE-CREATION 
>   plugins/defaultTools/connectionTool/AddConnectionPointCommand.cpp PRE-CREATION 
>   plugins/defaultTools/connectionTool/ChangeConnectionPointCommand.h PRE-CREATION 
>   plugins/defaultTools/connectionTool/ChangeConnectionPointCommand.cpp PRE-CREATION 
>   plugins/defaultTools/connectionTool/ConnectionPointWidget.h PRE-CREATION 
>   plugins/defaultTools/connectionTool/ConnectionPointWidget.cpp PRE-CREATION 
>   plugins/defaultTools/connectionTool/ConnectionPointWidget.ui PRE-CREATION 
>   plugins/defaultTools/connectionTool/ConnectionTool.h c0af323 
>   plugins/defaultTools/connectionTool/ConnectionTool.cpp a25b0d7 
>   plugins/defaultTools/connectionTool/ConnectionToolWidget.h 17e072e 
>   plugins/defaultTools/connectionTool/ConnectionToolWidget.cpp d7211ad 
>   plugins/defaultTools/connectionTool/ConnectionToolWidget.ui 6249ba4 
>   plugins/defaultTools/connectionTool/MoveConnectionPointStrategy.h PRE-CREATION 
>   plugins/defaultTools/connectionTool/MoveConnectionPointStrategy.cpp PRE-CREATION 
>   plugins/defaultTools/connectionTool/RemoveConnectionPointCommand.h PRE-CREATION 
>   plugins/defaultTools/connectionTool/RemoveConnectionPointCommand.cpp PRE-CREATION 
>   plugins/defaultTools/pics/CMakeLists.txt c64a256 
>   plugins/defaultTools/pics/hi16-action-escape-direction-all.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-down.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-horizontal.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-left.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-right.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-up.png PRE-CREATION 
>   plugins/defaultTools/pics/hi16-action-escape-direction-vertical.png PRE-CREATION 
>   plugins/treeshape/Layout.cpp 3b1a7ea 
>   plugins/treeshape/TreeShapeMoveStrategy.cpp cf20400 
> 
> Diff: http://git.reviewboard.kde.org/r/100447/diff
> 
> 
> Testing
> -------
> 
> Tested with the document attached to the bug report as well as with documents created with LibreOffice.
> 
> 
> Thanks,
> 
> Jan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/calligra-devel/attachments/20110127/a43e7cfb/attachment-0001.htm 


More information about the calligra-devel mailing list