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

[Jan Hambrecht]

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


Apart from the comments below I noticed that there is quite a lot of documentation missing.


libs/flake/KoLineBorder.h
<http://git.reviewboard.kde.org/r/103152/#comment7091>

    I think it would be better to move the function to the private class



libs/flake/KoMarker.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7092>

    needs a better name, as d->d is confusing. maybe name it data?



libs/flake/KoMarkerCollection.h
<http://git.reviewboard.kde.org/r/103152/#comment7103>

    I would have expected the marker collection to be derived from KoDataCenterBase. Is there any reason not to do that?



libs/flake/KoMarkerCollection.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7093>

    Is there a specific reason to add a null pointer here?



libs/flake/KoMarkerData.h
<http://git.reviewboard.kde.org/r/103152/#comment7094>

    Maybe add a private class/struct?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7095>

    what does the magic number do?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7096>

    what does the magic number do?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7097>

    isn't that redundant?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7098>

    shouldn't the pen be passed as reference?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7099>

    where does the 1.5 come from?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7100>

    again a magic number which is not clear where it came from



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7101>

    any chance of spliiting it up in three line to be eadier to digest?



libs/flake/KoPathShape.cpp
<http://git.reviewboard.kde.org/r/103152/#comment7102>

    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.


- Jan Hambrecht


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/20111119/0e5f55fb/attachment.htm>