Review Request: Add markers (arrows and other stuff put at the end of a line)

[Thorsten Zachmann]

> On Nov. 19, 2011, 12:14 a.m., Jan Hambrecht wrote:
> > libs/flake/KoMarkerCollection.h, line 32
> > <http://git.reviewboard.kde.org/r/103152/diff/2/?file=41215#file41215line32>
> >
> >     I would have expected the marker collection to be derived from KoDataCenterBase. Is there any reason not to do that?

It doesn't need any of the virtual abstract methods KoDataCenterBase so it is not needed. It is only needed when you have binary blobs you want to load/save.


> On Nov. 19, 2011, 12:14 a.m., Jan Hambrecht wrote:
> > libs/flake/KoMarkerCollection.cpp, line 39
> > <http://git.reviewboard.kde.org/r/103152/diff/2/?file=41216#file41216line39>
> >
> >     Is there a specific reason to add a null pointer here?

Adding the 0 pointer here add the possibility to unset a maker form the end of the line. Added comment to clarify


> On Nov. 19, 2011, 12:14 a.m., Jan Hambrecht wrote:
> > libs/flake/KoMarkerData.h, lines 63-66
> > <http://git.reviewboard.kde.org/r/103152/diff/2/?file=41217#file41217line63>
> >
> >     Maybe add a private class/struct?

I was thinking about it to but decided against it as the KoMarkerData is already something like a private class that gets new/delete when it is needed. Adding the private it will just another unneeded new/delete. I can change it if we think it makes sense to do the change


> On Nov. 19, 2011, 12:14 a.m., Jan Hambrecht wrote:
> > libs/flake/KoPathShape.cpp, line 1425
> > <http://git.reviewboard.kde.org/r/103152/diff/2/?file=41222#file41222line1425>
> >
> >     what does the magic number do?

use a const with a name for it


> On Nov. 19, 2011, 12:14 a.m., Jan Hambrecht wrote:
> > libs/flake/KoPathShape.cpp, lines 1592-1603
> > <http://git.reviewboard.kde.org/r/103152/diff/2/?file=41222#file41222line1592>
> >
> >     I think there should be a strong warning in the documentation of this method that is does change internal data while being marked as const.

I added a comment where it is done. Note: This does not break the constness of the method


- Thorsten


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


On Nov. 16, 2011, 5:51 a.m., Thorsten Zachmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103152/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2011, 5:51 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> At the sprint it was suggested to have this feature/bug fix in 2.4. Therefore positing for review. It adds one new string.
> 
> This add support for markers to calligra. 
> 
> Loading, saving, manipulation, work without problems
> 
> At the moment the document loaded need to have markers so that the user can select a marker on a path shape. I'm working on adding markers per default to the docker at the moment.
> 
> 
> This addresses bugs 260421, 260423 and 260431.
>     http://bugs.kde.org/show_bug.cgi?id=260421
>     http://bugs.kde.org/show_bug.cgi?id=260423
>     http://bugs.kde.org/show_bug.cgi?id=260431
> 
> 
> Diffs
> -----
> 
>   libs/flake/CMakeLists.txt 5face45 
>   libs/flake/KoDocumentResourceManager.h 2d6c8c0 
>   libs/flake/KoLineBorder.h 7f7d088 
>   libs/flake/KoLineBorder.cpp a2f0645 
>   libs/flake/KoMarker.h PRE-CREATION 
>   libs/flake/KoMarker.cpp PRE-CREATION 
>   libs/flake/KoMarkerCollection.h PRE-CREATION 
>   libs/flake/KoMarkerCollection.cpp PRE-CREATION 
>   libs/flake/KoMarkerData.h PRE-CREATION 
>   libs/flake/KoMarkerData.cpp PRE-CREATION 
>   libs/flake/KoMarkerSharedLoadingData.h PRE-CREATION 
>   libs/flake/KoMarkerSharedLoadingData.cpp PRE-CREATION 
>   libs/flake/KoPathShape.h 5b5b91b 
>   libs/flake/KoPathShape.cpp 1898862 
>   libs/flake/KoPathShapeFactory.cpp f3a98b8 
>   libs/flake/KoPathShape_p.h e1e2843 
>   libs/flake/KoShapeLoadingContext.cpp 9d3d1da 
>   libs/flake/KoShapeSavingContext.h a06e040 
>   libs/flake/KoShapeSavingContext.cpp 34f55c8 
>   libs/flake/commands/KoPathShapeMarkerCommand.h PRE-CREATION 
>   libs/flake/commands/KoPathShapeMarkerCommand.cpp PRE-CREATION 
>   libs/widgets/CMakeLists.txt 473a264 
>   libs/widgets/KoMarkerItemDelegate.h PRE-CREATION 
>   libs/widgets/KoMarkerItemDelegate.cpp PRE-CREATION 
>   libs/widgets/KoMarkerModel.h PRE-CREATION 
>   libs/widgets/KoMarkerModel.cpp PRE-CREATION 
>   libs/widgets/KoMarkerSelector.h PRE-CREATION 
>   libs/widgets/KoMarkerSelector.cpp PRE-CREATION 
>   libs/widgets/KoStrokeConfigWidget.h 3ae44b9 
>   libs/widgets/KoStrokeConfigWidget.cpp 9b2532f 
>   marker_todo.txt PRE-CREATION 
>   plugins/dockers/strokedocker/StrokeDocker.h dde8db7 
>   plugins/dockers/strokedocker/StrokeDocker.cpp 82f18b5 
> 
> Diff: http://git.reviewboard.kde.org/r/103152/diff/diff
> 
> 
> Testing
> -------
> 
> Tested various documents with markers and they all worked without problems. Tested manipulation of path shapes that have a marker attached.
> 
> 
> Thanks,
> 
> Thorsten Zachmann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20111120/1b5314ea/attachment.htm>