This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5373/

Ship it!

After some discussions on IRC, I'm fine with the patch.  I still think that the documentation should be improved, though, and it would be good if you could put the explanation above in some file in the sources.

- Inge


On September 17th, 2010, 9:17 a.m., Johannes Simon wrote:

Review request for KOffice and bjbreitm.
By Johannes Simon.

Updated 2010-09-17 09:17:31

Description 

I'm putting this up on reviewboard because this is a rather large and that touches (a minor part of) KSpread as well.
This is partly in reply to http://reviewboard.kde.org/r/4866/diff/ .

This patch:
- fixes charts with more than one table as data source
- fixes charts with a sheet as data source that's not the sheet the chart is in (in KSpread)
- handles sheet renames
- allows a chart to reference both an internal table and an external table (e.g. sheet in KSpread), needed for OOXML bubble charts

Take, for instance, a workbook in KSpread with multiple tables. Right now a chart can take data only and exclusively from the sheet it was created in. Also it is limited to the very name the sheet had when the chart was created. If you take data (e.g. Table2.A2:B3) from another sheet, the chart pretends it's data from the current sheet (e.g. Table1). If you change the sheet name, the chart will use the old name when saved to ODF. All that is fixed in this patch.

The patch does the following:
- Introduce a TableSource that is connected to KSpread's SheetAccessModel to provide name/model pairs for all tables
- Bind a CellRegion to a Table* pointer ("handed out" by TableSource) instead of just a (fixed) table name
- changes a lot of the CellRegion(QRect) constructor calls to CellRegion(Table*,QRect)
- makes sure CellRegion instances are used instead of their string representations so that sheet renames are trivially handled (CellRegion::table()->name() changes automatically)
- adds lots of test cases

I admit that I started this work while we weren't in feature freeze yet so it originally was a bit bigger and included also a refactoring of two classes, but I removed the irrelevant bits there much as possible to restrain this patch to fix the original issues only. With that and the new test cases I think this patch should be compatible with us being in bug-fix mode.

Testing 

5 sets of test cases for classes relevant in these changes (CellRegion, ChartProxyModel, TableSource, KDChartModel, DataSet) are added or adapted from /kchart/tests. TestDataSet demonstrates a simple test case that can only pass after the change.

Diffs

  • trunk/koffice/interfaces/KoChartInterface.h (1175800)
  • trunk/koffice/kspread/BindingModel.h (1175800)
  • trunk/koffice/kspread/chart/ChartDatabaseSelector.cpp (1175800)
  • trunk/koffice/plugins/chartshape/Axis.h (1175800)
  • trunk/koffice/plugins/chartshape/Axis.cpp (1175800)
  • trunk/koffice/plugins/chartshape/CMakeLists.txt (1175800)
  • trunk/koffice/plugins/chartshape/CellRegion.h (1175800)
  • trunk/koffice/plugins/chartshape/CellRegion.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartConfigWidget.h (1175800)
  • trunk/koffice/plugins/chartshape/ChartConfigWidget.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartProxyModel.h (1175800)
  • trunk/koffice/plugins/chartshape/ChartProxyModel.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartShape.h (1175800)
  • trunk/koffice/plugins/chartshape/ChartShape.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartShapeFactory.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartTableModel.cpp (1175800)
  • trunk/koffice/plugins/chartshape/ChartTool.h (1175800)
  • trunk/koffice/plugins/chartshape/ChartTool.cpp (1175800)
  • trunk/koffice/plugins/chartshape/DataSet.h (1175800)
  • trunk/koffice/plugins/chartshape/DataSet.cpp (1175800)
  • trunk/koffice/plugins/chartshape/OdfLoadingHelper.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/PlotArea.cpp (1175800)
  • trunk/koffice/plugins/chartshape/TableSource.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/TableSource.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/dialogs/TableEditorDialog.h (1175800)
  • trunk/koffice/plugins/chartshape/dialogs/TableEditorDialog.cpp (1175800)
  • trunk/koffice/plugins/chartshape/kchart_global.h (1175800)
  • trunk/koffice/plugins/chartshape/tests/CMakeLists.txt (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/ModelObserver.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/ModelObserver.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestCellRegion.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestCellRegion.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestDataSet.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestDataSet.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestKDChartModel.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestKDChartModel.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestProxyModel.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestProxyModel.cpp (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestTableSource.h (PRE-CREATION)
  • trunk/koffice/plugins/chartshape/tests/TestTableSource.cpp (PRE-CREATION)

View Diff