ODS import with merged cells

Daniel Bankston daniel.dev.libreoffice at gmail.com
Thu May 31 02:43:26 PDT 2012


On 05/26/2012 10:22 AM, Markus Mohrhard wrote:
> Hey Daniel,
>
> 2012/5/26 Daniel Bankston<daniel.dev.libreoffice at 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


More information about the LibreOffice mailing list