Re: [Libreoffice] Advice needed about some 3.4 named range related problem

2012-01-19 Thread Noel Power 

Hi Markus
On 13/01/12 18:27, Markus Mohrhard wrote:

I think it makes sense to remove the findByName and replace them with
findByUpperName. The findByName is error-prone and therefore I removed
it for 3-5, internally we only use upper name (and in 3.5 we use it
even to store the range name). I will clean-up your patch a bit
because some changes to rangeutl.cxx only make sense with other
changes in master/3.5.
re. our discussion on IRC last night, please have a look at the attached 
patch. ok for 3.4 you think now?


Noel


0001-fix-getCellRangeByName-failure-for-named-range.patch
Description: application/mbox
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] Advice needed about some 3.4 named range related problem

2012-01-19 Thread Markus Mohrhard 
Hello Noel,

 I think it makes sense to remove the findByName and replace them with
 findByUpperName. The findByName is error-prone and therefore I removed
 it for 3-5, internally we only use upper name (and in 3.5 we use it
 even to store the range name). I will clean-up your patch a bit
 because some changes to rangeutl.cxx only make sense with other
 changes in master/3.5.

 re. our discussion on IRC last night, please have a look at the attached
 patch. ok for 3.4 you think now?

Yes, that looks much better. I will still try to run
ScFiltersTest::testRangeName on 3-4.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[Libreoffice] Advice needed about some 3.4 named range related problem

2012-01-13 Thread Noel Power 

Hi Guys,

We have a bug here in the suse system that highlights a regression 
attempting to resolve a range by name. Note: this is not a problem on 
master.


the failing macro code is something like so;

Sub test
ProjectPlanSheet = ThisComponent.Sheets.getByName( ProjectPlan )
chkCellControl = ProjectPlanSheet.getCellRangeByName( TASK3_ON )
End Sub

where TASK3_ON is a document global rangename

after some digging the commits on master that seem to be involved are 
e9159d142a4f25bff88da3dd90e163135ae0bdfa 

9e8ae1d56076474e4673a953d8ebd726cb5daa18

I attach a backport of these patches ( in so far as I could backport 
them with at times quite different code base )


Note: for this bug the patch to rangeutl.cxx seems sufficient, I 
backported the rest for completeness. I want to sound you both out about 
applying the patch either partially ( e.g. just the rangeutl.cxx part ) 
or even completely to 3.4 or... even find out if ye considered either 
version too risky.


thanks,

Noel



diff --git sc/inc/rangenam.hxx sc/inc/rangenam.hxx
index fdff278..7417094 100644
--- sc/inc/rangenam.hxx
+++ sc/inc/rangenam.hxx
@@ -202,8 +202,8 @@ public:
 SC_DLLPUBLIC const ScRangeData* findByRange(const ScRange rRange) const;
 SC_DLLPUBLIC ScRangeData* findByName(const rtl::OUString rName);
 SC_DLLPUBLIC const ScRangeData* findByName(const rtl::OUString rName) const;
-ScRangeData* findByUpperName(const rtl::OUString rName);
-const ScRangeData* findByUpperName(const rtl::OUString rName) const;
+SC_DLLPUBLIC ScRangeData* findByUpperName(const rtl::OUString rName);
+SC_DLLPUBLIC const ScRangeData* findByUpperName(const rtl::OUString rName) const;
 SC_DLLPUBLIC ScRangeData* findByIndex(sal_uInt16 i);
 void UpdateReference(UpdateRefMode eUpdateRefMode, const ScRange rRange,
  SCsCOL nDx, SCsROW nDy, SCsTAB nDz);
diff --git sc/source/core/data/documen2.cxx sc/source/core/data/documen2.cxx
index 97292ca..5720e63 100644
--- sc/source/core/data/documen2.cxx
+++ sc/source/core/data/documen2.cxx
@@ -991,7 +991,7 @@ sal_uLong ScDocument::TransferTab( ScDocument* pSrcDoc, SCTAB nSrcPos,
 bool bInUse = ( aUsedNames.find(nOldIndex) != aUsedNames.end() );
 if (bInUse)
 {
-const ScRangeData* pExistingData = GetRangeName()-findByName(itr-GetName());
+const ScRangeData* pExistingData = GetRangeName()-findByUpperName(itr-GetUpperName());
 if (pExistingData)
 {
 // the name exists already in the destination document
diff --git sc/source/core/data/document.cxx sc/source/core/data/document.cxx
index 48cf99b..e85ba1f 100644
--- sc/source/core/data/document.cxx
+++ sc/source/core/data/document.cxx
@@ -1867,7 +1867,7 @@ void ScDocument::CopyRangeNamesFromClip(ScDocument* pClipDoc, ScClipRangeNameDat
 A proper solution would ask the user how to proceed.
 The adjustment of the indices in the formulas is done later.
 */
-const ScRangeData* pExistingData = GetRangeName()-findByName(itr-GetName());
+const ScRangeData* pExistingData = GetRangeName()-findByUpperName(itr-GetUpperName());
 if (pExistingData)
 {
 sal_uInt16 nOldIndex = itr-GetIndex();
diff --git sc/source/core/tool/interpr1.cxx sc/source/core/tool/interpr1.cxx
index 3614896..eb5ab56 100644
--- sc/source/core/tool/interpr1.cxx
+++ sc/source/core/tool/interpr1.cxx
@@ -6628,7 +6628,7 @@ void ScInterpreter::ScIndirect()
 if (!pNames)
 break;
 
-ScRangeData* pData = pNames-findByName(sRefStr);
+ScRangeData* pData = pNames-findByUpperName(ScGlobal::pCharClass-upper(sRefStr));
 if (!pData)
 break;
 
diff --git sc/source/core/tool/rangenam.cxx sc/source/core/tool/rangenam.cxx
index 35c00dc..b648d1a 100644
--- sc/source/core/tool/rangenam.cxx
+++ sc/source/core/tool/rangenam.cxx
@@ -682,17 +682,6 @@ public:
 }
 };
 
-class MatchByName : public unary_functionScRangeData, bool
-{
-const OUString mrName;
-public:
-MatchByName(const OUString rName) : mrName(rName) {}
-bool operator() (const ScRangeData r) const
-{
-return mrName.equals(r.GetName());
-}
-};
-
 class MatchByUpperName : public unary_functionScRangeData, bool
 {
 const OUString mrName;
@@ -760,20 +749,6 @@ const ScRangeData* ScRangeName::findByRange(const ScRange rRange) const
 return itr == maData.end() ? NULL : (*itr);
 }
 
-ScRangeData* ScRangeName::findByName(const OUString rName)
-{
-DataType::iterator itr = std::find_if(
-maData.begin(), maData.end(), MatchByName(rName));
-return itr == maData.end() ? NULL : (*itr);
-}
-
-const ScRangeData* ScRangeName::findByName(const OUString rName) const
-{
-DataType::const_iterator itr = std::find_if(
-

Re: [Libreoffice] Advice needed about some 3.4 named range related problem

2012-01-13 Thread Markus Mohrhard 
Hello Noel,

2012/1/13 Noel Power nopo...@suse.com:
 Hi Guys,

 We have a bug here in the suse system that highlights a regression
 attempting to resolve a range by name. Note: this is not a problem on
 master.

 the failing macro code is something like so;

 Sub test
    ProjectPlanSheet = ThisComponent.Sheets.getByName( ProjectPlan )
    chkCellControl = ProjectPlanSheet.getCellRangeByName( TASK3_ON )
 End Sub

 where TASK3_ON is a document global rangename

 after some digging the commits on master that seem to be involved are
 e9159d142a4f25bff88da3dd90e163135ae0bdfa 
 9e8ae1d56076474e4673a953d8ebd726cb5daa18

 I attach a backport of these patches ( in so far as I could backport them
 with at times quite different code base )

 Note: for this bug the patch to rangeutl.cxx seems sufficient, I backported
 the rest for completeness. I want to sound you both out about applying the
 patch either partially ( e.g. just the rangeutl.cxx part ) or even
 completely to 3.4 or... even find out if ye considered either version too
 risky.


I think it makes sense to remove the findByName and replace them with
findByUpperName. The findByName is error-prone and therefore I removed
it for 3-5, internally we only use upper name (and in 3.5 we use it
even to store the range name). I will clean-up your patch a bit
because some changes to rangeutl.cxx only make sense with other
changes in master/3.5.

But before we push this changes to a stable branch I want to check
with backporting the tests from 3.5 to 3.4 and check my other range
name commits from my rework that I did not adjust one of these
patches. Just to make it clear, I don't think that this patch will
create any problems and is safe but I don't feel comfortable changing
such an important part without the tests.

Markus
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice