Review Request: Complete support for ODF glue points

Jan Hambrecht jaham at gmx.net
Thu Jan 27 23:08:54 GMT 2011 


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > libs/flake/KoConnectionPoint.h, line 80
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7595#file7595line80>
> >
> >     The point are not stored in shape coordinates but in percentage value of the size

How it is stored internally is just an implementation detail. Once the connection point gets passed outside the shape via KoShape::connectionPoint, KoShape::connectionPoints, the position is converted to shape coordinates. So I think the comment is correct.


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > libs/flake/KoTextOnShapeContainer.cpp, line 340
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7606#file7606line340>
> >
> >     You are sure this should be committed? Yes I know with KoTextOnShapeContainer some of the stuff does not work

Oops, no. That was an accident.


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > libs/flake/KoConnectionShape.cpp, line 86
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7598#file7598line86>
> >
> >     Please define as const

Ok


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > plugins/defaultTools/connectionTool/ConnectionTool.h, line 106
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7623#file7623line106>
> >
> >     this method should be const

Ok


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > plugins/defaultTools/connectionTool/ConnectionTool.h, line 125
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7623#file7623line125>
> >
> >     shapes should be passed as const &
> >     Can the function be made const?

Sure, that was just an oversight.


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > plugins/defaultTools/connectionTool/ConnectionTool.h, line 131
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7623#file7623line131>
> >
> >     please make the function const.

Ok


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > plugins/defaultTools/connectionTool/ConnectionTool.cpp, line 529
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7624#file7624line529>
> >
> >     Can the function made const?

Sure.


> On Jan. 27, 2011, 5:53 a.m., Thorsten Zachmann wrote:
> > plugins/defaultTools/connectionTool/ConnectionTool.cpp, line 279
> > <http://git.reviewboard.kde.org/r/100447/diff/1/?file=7624#file7624line279>
> >
> >     there should be a blank after the if

Ok


- Jan


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


On Jan. 27, 2011, 11:08 p.m., Jan Hambrecht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100447/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2011, 11:08 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/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/a51d8bf8/attachment.htm>

More information about the calligra-devel mailing list