Re: ODS import with merged cells

On 05/26/2012 10:22 AM, Markus Mohrhard wrote:
> 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
Hi, Markus and Kohei,

I had a few more fails, but I finally have a working build of 
feature/gsoc-calc-perf again.  Did you guys have a chance to look at my 
changes to the ScXMLTableRowCellContext merge methods to determine if 
they are appropriate or not?

Thanks.
--Daniel