Review Request: Fix missing images in excel filter

[Lukáš Tvrdý]

> On Nov. 4, 2011, 9:41 a.m., Jarosław Staniek wrote:
> > filters/tables/excel/import/ODrawClient.cpp, line 118
> > <http://git.reviewboard.kde.org/r/103044/diff/1/?file=40230#file40230line118>
> >
> >     maybe isEmpty() could be better?

Why?


> On Nov. 4, 2011, 9:41 a.m., Jarosław Staniek wrote:
> > filters/tables/excel/sidewinder/cell.h, line 124
> > <http://git.reviewboard.kde.org/r/103044/diff/1/?file=40232#file40232line124>
> >
> >     BTW, someone will have to transform these comments to Doxygen comments...

I removed that comment, why to Doxygen?


> On Nov. 4, 2011, 9:41 a.m., Jarosław Staniek wrote:
> > filters/tables/excel/sidewinder/excel.h, line 864
> > <http://git.reviewboard.kde.org/r/103044/diff/1/?file=40234#file40234line864>
> >
> >     let's return values not references; difference in performance is minimal (or none in new compilers?) while it's far safer

Why is the performance minimal? Implicit sharing?
Why it is safer? I'm not returning reference to local object.


- Lukáš


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


On Nov. 4, 2011, 9:16 a.m., Lukáš Tvrdý wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103044/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2011, 9:16 a.m.)
> 
> 
> Review request for Calligra, Marijn Kruisselbrink and Sebastian Sauer.
> 
> 
> Description
> -------
> 
> This patch uses more of the libmso in excel filter.
> 
> So far the pictures were handled as PictureObject class. One problem was that
> there was no support for groups for this object. So when you put two images into groups, they
> are not displayed because the anchoring information is saved in different data-structure - in the 
> group structure.
> 
> Now I let the records to be handled by libmso, which provides this functionality.
> It supports groups and even graphical styles like like borders etc.
> 
> 
> This addresses bug 262865.
>     http://bugs.kde.org/show_bug.cgi?id=262865
> 
> 
> Diffs
> -----
> 
>   filters/libmso/pictures.cpp f8bfa7c 
>   filters/tables/excel/import/ExcelImport.cpp 1bb7f17 
>   filters/tables/excel/import/ODrawClient.cpp 5a6507b 
>   filters/tables/excel/import/excelimporttoods.cc 287aef7 
>   filters/tables/excel/sidewinder/cell.h bc080f9 
>   filters/tables/excel/sidewinder/cell.cpp 8b8c0e1 
>   filters/tables/excel/sidewinder/excel.h 2be1f5a 
>   filters/tables/excel/sidewinder/excel.cpp b475e5a 
>   filters/tables/excel/sidewinder/globalssubstreamhandler.h 77c6e3a 
>   filters/tables/excel/sidewinder/globalssubstreamhandler.cpp f9c03b1 
>   filters/tables/excel/sidewinder/objects.h 2412f07 
>   filters/tables/excel/sidewinder/sheet.h 9ff346d 
>   filters/tables/excel/sidewinder/sheet.cpp 440f44b 
>   filters/tables/excel/sidewinder/workbook.h 2e3c2fa 
>   filters/tables/excel/sidewinder/workbook.cpp 9ad61bc 
>   filters/tables/excel/sidewinder/worksheetsubstreamhandler.cpp 879f76f 
> 
> Diff: http://git.reviewboard.kde.org/r/103044/diff/diff
> 
> 
> Testing
> -------
> 
> I did regression testing against master. My testing set consists of 976 xls files.
> I found only positive changes.
> 
> Missing frames around pictures are displayed now, missing images are displayed now.
> You can test the document.
> 
> 
> Thanks,
> 
> Lukáš Tvrdý
> 
>

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