Re: [Libreoffice] [REVIEW] fix for fdo#39850 and fdo#39820: update range names and database ranges in formula cells

Hello Eike,

2011/8/16 Eike Rathke <ooo@erack.de>

> Hi Markus,
>
> On Tuesday, 2011-08-16 16:25:22 +0200, Markus Mohrhard wrote:
>
> > > First thing that caught my eye: using toAsciiUpperCase() (or any other
> > > ASCII method) on anything that the user inputs as names is a no-no as
> it
> > > doesn't handle Unicode. Use ScGlobal::pCharClass->upper() instead.
> >
> > I think I inherite it from some other places that use range names. So
> then
> > we need to check for all other places too and adjust them accordingly.
>
> Yes please.
>
>
> > > The change to the ScFormulaCell ctor apparently unconditionally tries
> to
> > > adjust name tokens, this is completely unnecessary in most situations
> > > and should be triggered only if the sheets of new and old position
> > > differ.
> >
> > No it's not that easy. If we have the same sheet and two different
> > documents, we need to adjust too. And we can't simply check that the
> > ScDocument instances are different because they will always be. Either we
> > have a pointer to the original ScDocument instance in the copy document
> or
> > we must always adjust our range names. I used the second approach because
> I
> > didn't like the other approach.
>
> So at least compare document instances and sheets and if both are
> identical don't do the adjustment, which I think is the most used
> scenario when copying cells. Sounds plausible?

So just to understand you correctly: You suggest to introduce a new variable
ScDocument* pOriginalDoc; in ScDocument and check if this pointer and the
address of rNewDoc are the same? I didn't like this very much but I have no
strong opinion here. If you think that this would be the better version i
will change this.


> Btw, how well does this all cope with undo/redo?

I don't see any problems with undo/redo. Local range names are copied either
way and if global range names are not copied then my check that a global
ScRangeName exists will fail and don't adjust anything. But I think we can
simply add a check for IsUndo. Then we don't have to think about it anymore.

Markus