Review Request: Implement saving for the KoUnavailShape

[Thorsten Zachmann]

> On Feb. 13, 2011, 5:09 a.m., Thorsten Zachmann wrote:
> > libs/odf/KoEmbeddedFileSaver.h, line 56
> > <http://git.reviewboard.kde.org/r/100629/diff/1/?file=8941#file8941line56>
> >
> >     Please pass contents as a const &
> 
> Inge Wallin wrote:
>     No, I don't think we should do that, and there is a good reason for it: When the call to saveFile and embedFile is done, there is no way for the KoEmbeddedFileSaver to know what the shape does with the QByteArray.  The QByteArray is implicitly shared, meaning it is ref counted and by creating a copy this way I protect against the contents being deleted between the call to saveFile and when the contents is actually saved to the store.  But I should really add a comment about that.

That does not mean the parameter passed can't be a const ref. Sure you need to store a copy inside your class. But there is no need to copy the object twice even if it is implicitly shared.


- Thorsten


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


On Feb. 11, 2011, 9:36 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100629/
> -----------------------------------------------------------
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> This patch implements saving in the KoUnavailShape.
> 
> The method it uses is to introduce a new class, KoEmbeddedFileSaver, in koodf. This class is a bit like the KoEmbeddedObjectSaver class, and is created and instantiated in the same places. It is also part of the KoShapeSavingContext.
> 
> As a bonus, this patch also implements saving for the Vector shape, since I used that for testing in early stages.
> 
> 
> Diffs
> -----
> 
>   braindump/src/SectionsIO.cpp 9d8949b 
>   karbon/common/KarbonDocument.cpp 1a6a2a5 
>   kformula/flake/FormulaDocument.cpp 84872fa 
>   kformula/flake/KoFormulaTool.cpp d9eada1 
>   krita/ui/flake/kis_shape_layer.cc f695b8a 
>   krita/ui/flake/kis_shape_selection.cpp f5e2f28 
>   libs/flake/KoDrag.cpp 8ed2656 
>   libs/flake/KoDragOdfSaveHelper.h b1e6212 
>   libs/flake/KoDragOdfSaveHelper.cpp 727e6c5 
>   libs/flake/KoShapeSavingContext.h a06e040 
>   libs/flake/KoShapeSavingContext.cpp fd8fb41 
>   libs/flake/KoUnavailShape.cpp 3f10acf 
>   libs/kokross/KoScriptingOdf.cpp 17a4f7d 
>   libs/kopageapp/KoPADocument.cpp 2ef4ac1 
>   libs/kopageapp/KoPAOdfPageSaveHelper.h d8f064a 
>   libs/kopageapp/KoPAOdfPageSaveHelper.cpp c60d0d8 
>   libs/kopageapp/KoPAPastePage.cpp 01bafe0 
>   libs/kopageapp/KoPASavingContext.h c678e2c 
>   libs/kopageapp/KoPASavingContext.cpp 52524bf 
>   libs/kotext/KoTextDrag.cpp f30cb5c 
>   libs/kotext/KoTextOdfSaveHelper.h 155aecd 
>   libs/kotext/KoTextOdfSaveHelper.cpp d012557 
>   libs/kotext/KoTextShapeSavingContext.h 4d27bc4 
>   libs/kotext/KoTextShapeSavingContext.cpp d5ade20 
>   libs/kotext/opendocument/tests/TestChangeLoading.cpp ac42ddd 
>   libs/kotext/opendocument/tests/TestLoading.cpp e6a790a 
>   libs/main/KoDocument.cpp 956e179 
>   libs/odf/CMakeLists.txt 1a6a131 
>   libs/odf/KoEmbeddedDocumentSaver.cpp a8e13d8 
>   libs/odf/KoEmbeddedFileSaver.h PRE-CREATION 
>   libs/odf/KoEmbeddedFileSaver.cpp PRE-CREATION 
>   libs/odf/KoOdfDocument.h c242776 
>   libs/odf/KoOdfLoadingContext.h 023b92f 
>   libs/odf/KoOdfLoadingContext.cpp ce53992 
>   libs/odf/KoOdfManifest.h PRE-CREATION 
>   libs/odf/KoOdfManifest.cpp PRE-CREATION 
>   plugins/chartshape/ChartDocument.cpp f85bb19 
>   plugins/chartshape/ChartShape.cpp edce6be 
>   plugins/vectorshape/VectorShape.cpp 9965d68 
>   tables/Cell.cpp 7aa8306 
>   tables/DocBase.cpp 5ccaef3 
>   words/part/KWDocument.cpp f3dc5d4 
>   words/part/KWOdfWriter.h 4264e99 
>   words/part/KWOdfWriter.cpp b507bbf 
> 
> Diff: http://git.reviewboard.kde.org/r/100629/diff
> 
> 
> Testing
> -------
> 
> Tested with a simple odt file with an embedded spreadsheet.
> 
> 
> Thanks,
> 
> Inge
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110213/57a58cf2/attachment.htm>