Review Request: Introduce roundtripping for annotations

Friedrich W. H. Kossebau kossebau at kde.org
Sun Oct 28 13:01:58 GMT 2012


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


Good to see code coming into direction of master with first support for annotations :)

Did a quick review at sourcecode level, did not check the logic of the annotation support though, left for somebody else.

Another question: I guess you also tested with odfautotests (annotation, annotation-end)?
Perhaps nice to mention this here as well, including version (git-commit) of odfautotests you used.


libs/kotext/KoAnnotation.h
<http://git.reviewboard.kde.org/r/107088/#comment16532>

    bool isEndMarker better is an enum-based parameter, for improved readability



libs/kotext/KoAnnotationManager.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16533>

    Why the while-loop and not this:
    
    KoAnnotation *annotation = d->annotationHash.take(oldName);
    if (annotation) {
        /*here the update of name*/
    }



libs/kotext/KoTextRangeManager.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16534>

    Remove rebase-conflict markers



libs/kotext/opendocument/KoTextLoader.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16527>

    Please remove such spaces at the line end, here and at the very end of quite some files of this diff.
    Unless it is your personal fingerprint ;)



libs/widgets/KoAnnotationBalloon.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16535>

    hardcoded font size, use global font settings, e.g. for tool fonts



libs/widgets/KoAnnotationBalloon.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16528>

    hardcoded color ? TODO



libs/widgets/KoAnnotationSideBar.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16530>

    hardcoded color ? TODO



libs/widgets/KoAnnotationSideBar.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16536>

    Is this Calligra code style?
    Other code also sometimes may not be fitting perfectly the style.



libs/widgets/KoBalloon.h
<http://git.reviewboard.kde.org/r/107088/#comment16537>

    the parameter "position" better is named "y" or "yPosition", to match the property it is for



libs/widgets/KoBalloon.h
<http://git.reviewboard.kde.org/r/107088/#comment16531>

    const



libs/widgets/KoBalloon.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16526>

    Hard coded color, please add TODO.



words/part/KWCanvas.cpp
<http://git.reviewboard.kde.org/r/107088/#comment16538>

    hard-coded color!


- Friedrich W. H. Kossebau


On Oct. 28, 2012, 2:47 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107088/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2012, 2:47 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> I really wanted to have annotations for 2.6. But this patch at least brings loading, storing and saving of annotations even if they cannot yet be be created or edited or even shown.  
> 
> The patch contained some parts that had to do with the view in Words but I disabled that. It's easy to remove if necessary.
> 
> For reasons you can read on the mailing list I would really like to get this patch into 2.6.
> 
> 
> Diffs
> -----
> 
>   libs/kotext/CMakeLists.txt 28faa22 
>   libs/kotext/KoAnnotation.h PRE-CREATION 
>   libs/kotext/KoAnnotation.cpp PRE-CREATION 
>   libs/kotext/KoAnnotationManager.h PRE-CREATION 
>   libs/kotext/KoAnnotationManager.cpp PRE-CREATION 
>   libs/kotext/KoTextInlineRdf.h 91dbccd 
>   libs/kotext/KoTextInlineRdf.cpp 1017cf4 
>   libs/kotext/KoTextRangeManager.h fc6fb95 
>   libs/kotext/KoTextRangeManager.cpp 5417be8 
>   libs/kotext/opendocument/KoTextLoader.cpp 5cf9543 
>   libs/widgets/CMakeLists.txt 61df2d6 
>   libs/widgets/KoAnnotationBalloon.h PRE-CREATION 
>   libs/widgets/KoAnnotationBalloon.cpp PRE-CREATION 
>   libs/widgets/KoAnnotationSideBar.h PRE-CREATION 
>   libs/widgets/KoAnnotationSideBar.cpp PRE-CREATION 
>   libs/widgets/KoBalloon.h PRE-CREATION 
>   libs/widgets/KoBalloon.cpp PRE-CREATION 
>   words/part/KWCanvas.cpp 08060d2 
>   words/part/KWCanvasBase.cpp e74835e 
>   words/part/KWCanvasItem.cpp dd90755 
>   words/part/KWView.cpp b1004ed 
>   words/part/tests/CMakeLists.txt 9fa7787 
> 
> Diff: http://git.reviewboard.kde.org/r/107088/diff/
> 
> 
> Testing
> -------
> 
> Test file created by LibreOffice.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20121028/36e0c1f5/attachment.htm>


More information about the calligra-devel mailing list