Re: ODS import with merged cells

Hey Daniel,

2012/5/26 Daniel Bankston <daniel.dev.libreoffice@gmail.com>:
> Hi, Kohei and Markus,
>
> I pushed my changes to ScXMLTableRowCellContext::DoMerge() where I have
> removed the ScDocFunc layer and simplified the whole method in general.

Great. I will have a look at it on Tuesday.

> As I mentioned to Kohei at some point, when I was originally removing the
> UNO calls from the code, I was trying not to question the logic too much and
> just focus on direct conversion to equivalent Sc calls.  I went back through
> the ScXMLTableRowCellContext merge methods closely following what occurs in
> the code and thinking about what is actually trying to be accomplished.
>
> It seems that the only thing that is actually needed here during import is a
> call to ScDocument::DoMerge().  As far as I could tell, everything else was
> unnecessary in the ScDocFunc layer.  Also,
> ScXMLTableRowCellContext::IsMerged() and the unmerge portion of
> ScXMLTableRowCellContext::DoMerge() that existed before this latest commit
> seem to have no purpose during import.  Whether a cell is merged or not is
> decided before ScXMLTableRowCellContext::DoMerge() is called (member
> bIsMerged), otherwise it wouldn't be called.  And unmerging a cell on import
> doesn't make sense, right?

I'm not 100% sure about that. If I recall correctly we need to unmerge
cells before we can merge bigger areas. But without having a closer
look at the code I can only speculate that it is for this.

> After these changes, I can still successfully compile and pass sc unit
> tests.  Also, when I manually run calc and open a spreadsheet with merged
> cells, I see that the file is imported correctly.

That our tests pass doe not mean much. It is always a good diea to
check if one of the featurres that you convert is covered by our
import tests. and even if it is it has often just some basic test
cases like in the merge case. In such cases it would be great if you
could add some more test cases especially for the corner cases you saw
in the code.

> Can you see anything that I might have missed or incorrectly removed?

No. From what you say it seems all reasonable.

Regards,
Markus