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

[Jan Hambrecht]

> 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?
> 
> Thorsten Zachmann wrote:
>     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.

Ok, the name made me relate it to the image collection class, so I thought it would be more consistent to make it similar.


> 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?
> 
> Thorsten Zachmann wrote:
>     Adding the 0 pointer here add the possibility to unset a maker form the end of the line. Added comment to clarify

Well I think that is something that should better be dealt with in the gui part of the code, e.g. the docker. Now you have to check everywhere when using the marker collection for null pointers, instead of making sure there is never a null pointer added to the marker collection and be done with it.


> 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?
> 
> Thorsten Zachmann wrote:
>     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

Hmm, I thought all exported classes should have a d-pointer?


- Jan


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


On Nov. 20, 2011, 7:09 a.m., Thorsten Zachmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103152/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2011, 7:09 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.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/20111122/3027bd70/attachment.htm>