Re: [PATCH] Bug 48140 - [EasyHack] remove uno calls from xlsx import

On 15/04/12 05:20, Daniel Bankston [danthedev] wrote:
> Hello, everyone,
>
> I have attached my patches for EasyHack 48140.  I have taken baby 
> steps, so this just the first part of completing EasyHack 48140.  I 
> changed WorkbookHelper::createNamedRangeObject and 
> createLocalNamedRangeObject to return ScRangeData* instead of 
> Reference<XNamedRanged>, and then I adjusted all callers.  You will 
> notice I commented the old code out instead of deleting it.  I did 
> this as this is just partial update for the EasyHack.
>
> I was able to successfully make sc with these changes.  "make -sr 
> slowcheck" was also successful.  My own manual tests were also 
> successful.  I tested by running calc and opening xlsx spreadsheets 
> containing ranges with names.  I also created names for ranges and 
> created names for cells in those spreadsheets.  I referred to those 
> ranges and cells by name in other cells.
>
> Although my tests were successful, I still fear that there is 
> something that I may have missed due to my current beginner level of 
> understanding of the sc model.  I am eager to see what suggestions you 
> all may have.
>
> When I have more time, I later plan to make similar changes to the 
> database ranges and eventually remove all UNO calls in these methods 
> where possible.  (School semester is still in for me and quite busy as 
> it nears the end.)
First please don't comment out code, it just makes the diff even more 
confusing to look at, git already shows the deletions and annotating 
that in the source really isn't necessary. Also, it's not a good idea to 
have patches to patches especially for a standalone task like this. 
Could you resubmit a cummulative patch  ( I think you can use git commit 
-i for that ). From a quick look it seems the patch is probably ok, one 
problem I see with a test document I have is that I now see in the 
'Insert | Names | Manage' dialog many many '__sharedxxxx' entries. At a 
wild guess this would seem to step from the fact that the old code used 
to set 'IsSharedFormula' on some named object entries and your patch 
doesn't seem to do that. Markus or Kohei probably would know more about 
the specifics of that stuff

However I think the patch is most likely "nearly there" so don't give 
up.  I think at least you have proved that you can download, build and 
at least manipulate the libreoffice code which is the main purpose of 
the easy hack :-)

thanks for the patch and effort
Noel