Review Request 122045: Calligra filters: refactor chart handling and make it sane

[Inge Wallin]

> On Jan. 18, 2015, 3:21 a.m., Friedrich W. H. Kossebau wrote:
> > filters/libodf2/CMakeLists.txt, line 4
> > <https://git.reviewboard.kde.org/r/122045/diff/1/?file=341663#file341663line4>
> >
> >     Including libmso files for libodf2 seems wrong as well, similar as Charting.h is non-odf stuff.

Yes, I totally agree. Unfortunately the current chart code stores unparsed number formats so that we have to parse them during saving. I don't want to rewrite that in a refactor-only patch so instead I added a big FIXME for this. I hope that's enough for now.


> On Jan. 18, 2015, 3:21 a.m., Friedrich W. H. Kossebau wrote:
> > filters/libodf2/chart/Charting.h, line 31
> > <https://git.reviewboard.kde.org/r/122045/diff/1/?file=341664#file341664line31>
> >
> >     Hm, this is MSO(OXML) specific stuff, no?
> >     
> >     Why would you make this part of KoChart/libodf2?
> >     
> >     Would this scale to make all data structures of other formats part of KoChart/libodf2?

No, not at all. It's a generic chart storage that can be used to store charts from anywhere. Note that the parsing code from various formats are in the filters, and not in libodf2.


- Inge


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122045/#review74212
-----------------------------------------------------------


On Jan. 13, 2015, 10:18 p.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122045/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 10:18 p.m.)
> 
> 
> Review request for Calligra, Lassi Nieminen and Jarosław Staniek.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch is a refactoring of how charts are handled in the filters. There are two purposes of it:
> 1. Make it sane in general. As it was before, the various filters where reaching into each other for bits and pieces of code, and the naming was strange in places. There was also a strange mixture of MS binary and MSOOXML concepts in the same files.
> 2. To prepare for the next step which is sharing storage classes for charts between the filters and the chart shape. As it is now, many defines and both loading and saving is duplicated all over Calligra. This is not the way we want things to be.
> 
> There is hardly any new code in here, only rearranging of what was already there. Here is a summary (everything happens inside the filters/ tree):
>  - Factor out common parts and move them to libodf2/charts
>  - Factor out the parts common to MS handling and move them to libmso/
>  - Move common parts within the ooxml filters from the xlsx tree to libmsooxml/
>  - Move things that only have to do with MS binary parts to libmso/
>  - Rename the vaguely named namespace "Charting" to "KoChart"
>  - Rename ChartExport to KoOdfChartWriter, which should be clearer
>  - Make the filters use and link to the libraries instead of each other
> 
> 
> Diffs
> -----
> 
>   filters/libmso/CMakeLists.txt 19aced3 
>   filters/libmso/MsoUtils.h PRE-CREATION 
>   filters/libmso/MsoUtils.cpp PRE-CREATION 
>   filters/libmso/NumberFormatParser.h PRE-CREATION 
>   filters/libmso/NumberFormatParser.cpp PRE-CREATION 
>   filters/libmso/XlsUtils.h PRE-CREATION 
>   filters/libmsooxml/CMakeLists.txt 7dc2d7f 
>   filters/libmsooxml/MsooXmlCommonReaderDrawingMLImpl.h e8748ef 
>   filters/libmsooxml/MsooXmlDrawingTableStyleReader.cpp c7bd7fe 
>   filters/libmsooxml/MsooXmlImport.cpp 49ad484 
>   filters/libmsooxml/MsooXmlTheme.h PRE-CREATION 
>   filters/libmsooxml/MsooXmlThemesReader.h 9682c34 
>   filters/libmsooxml/MsooXmlThemesReader.cpp 5335029 
>   filters/libmsooxml/MsooXmlUtils.h 055fa15 
>   filters/libmsooxml/MsooXmlUtils.cpp 31df41e 
>   filters/libodf2/CMakeLists.txt 3792771 
>   filters/libodf2/chart/Charting.h PRE-CREATION 
>   filters/libodf2/chart/KoOdfChartWriter.h PRE-CREATION 
>   filters/libodf2/chart/KoOdfChartWriter.cpp PRE-CREATION 
>   filters/libodf2/chart/PLAN PRE-CREATION 
>   filters/sheets/excel/export/CMakeLists.txt acc5a73 
>   filters/sheets/excel/import/CMakeLists.txt de653ee 
>   filters/sheets/excel/import/ExcelImport.cpp abf4c52 
>   filters/sheets/excel/import/excelimporttoods.cc 5d6887b 
>   filters/sheets/excel/sidewinder/CMakeLists.txt 097e2a8 
>   filters/sheets/excel/sidewinder/chartsubstreamhandler.h d22c02b 
>   filters/sheets/excel/sidewinder/chartsubstreamhandler.cpp ecd9071 
>   filters/sheets/excel/sidewinder/objects.h 5162cee 
>   filters/sheets/xlsx/CMakeLists.txt 294f048 
>   filters/sheets/xlsx/ChartExport.h aa9895d 
>   filters/sheets/xlsx/ChartExport.cpp 27b075b 
>   filters/sheets/xlsx/Charting.h 4fea470 
>   filters/sheets/xlsx/NumberFormatParser.h e877d40 
>   filters/sheets/xlsx/NumberFormatParser.cpp 433ffe8 
>   filters/sheets/xlsx/XlsxChartOdfWriter.h PRE-CREATION 
>   filters/sheets/xlsx/XlsxChartOdfWriter.cpp PRE-CREATION 
>   filters/sheets/xlsx/XlsxImport.cpp b0091ca 
>   filters/sheets/xlsx/XlsxUtils.h 2858ce4 
>   filters/sheets/xlsx/XlsxXmlChartReader.h a2b07e8 
>   filters/sheets/xlsx/XlsxXmlChartReader.cpp 0434961 
>   filters/sheets/xlsx/XlsxXmlCommentsReader.h e025a06 
>   filters/sheets/xlsx/XlsxXmlCommonReader.h e406b0a 
>   filters/sheets/xlsx/XlsxXmlDocumentReader.h 6395799 
>   filters/sheets/xlsx/XlsxXmlDocumentReader.cpp a4961ee 
>   filters/sheets/xlsx/XlsxXmlDrawingReader.h 69c75ac 
>   filters/sheets/xlsx/XlsxXmlDrawingReader.cpp def1ff6 
>   filters/sheets/xlsx/XlsxXmlStylesReader.h 4793ae3 
>   filters/sheets/xlsx/XlsxXmlWorksheetReader.h 559a029 
>   filters/sheets/xlsx/XlsxXmlWorksheetReader.cpp 74e1302 
>   filters/stage/pptx/CMakeLists.txt 36434f9 
>   filters/stage/pptx/PptxXmlSlideReader.cpp 702a381 
>   filters/words/docx/import/CMakeLists.txt 70490d4 
>   filters/words/docx/import/DocxXmlDocumentReader.h bfb9171 
>   filters/words/docx/import/DocxXmlDocumentReader.cpp ae7cb0e 
>   filters/words/msword-odf/CMakeLists.txt 2b975a1 
>   CMakeLists.txt 1f992dd 
> 
> Diff: https://git.reviewboard.kde.org/r/122045/diff/
> 
> 
> Testing
> -------
> 
> There is no new code at all, only rearrangement. But all the filters work as expected.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150118/7c370882/attachment.htm>