sc/source/core/tool/dbdata.cxx |   58 ++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 18 deletions(-)

New commits:
commit 48b9cbc742de3f6120986cb6cafc92eb5009da82
Author:     Eike Rathke <er...@redhat.com>
AuthorDate: Wed Sep 28 20:51:05 2022 +0200
Commit:     Eike Rathke <er...@redhat.com>
CommitDate: Thu Sep 29 00:42:12 2022 +0200

    Fix incrementing number in dbrange names during copying, tdf#145054 
follow-up
    
    lcl_IncrementNumberInNamedRange() during copying a sheet didn't do
    what it was supposed to do, it could generate names that would had
    been cell references, and the loop it was called from could had
    prematurely ended because it inserted into the set it was
    iterating over; also the loop ended as soon as it encountered just
    one dbrange that wasn't on the sheet that was copied. That never
    worked as intended with more than just a very few names only on
    the same sheet.
    
    Additionally after the actual change loplugin:stringviewparam
    forced me to pass a std::u16string_view parameter, for that some
    adaptions had to be made.
    
    Change-Id: Ib83d317c69d821e8e8a2f1cd6791da9616ed188d
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140717
    Reviewed-by: Eike Rathke <er...@redhat.com>
    Tested-by: Jenkins

diff --git a/sc/source/core/tool/dbdata.cxx b/sc/source/core/tool/dbdata.cxx
index 0b54c6504371..cf518d0e2890 100644
--- a/sc/source/core/tool/dbdata.cxx
+++ b/sc/source/core/tool/dbdata.cxx
@@ -1007,31 +1007,47 @@ public:
 };
 
 OUString lcl_IncrementNumberInNamedRange(ScDBCollection::NamedDBs& namedDBs,
-                                         const OUString& sOldName)
-{
-    sal_Int32 nLastIndex = sOldName.lastIndexOf('_');
+                                         std::u16string_view rOldName)
+{
+    // Append or increment a numeric suffix and do not generate names that
+    // could result in a cell reference by ensuring at least one underscore is
+    // present.
+    // "aa"     => "aa_2"
+    // "aaaa1"  => "aaaa1_2"
+    // "aa_a"   => "aa_a_2"
+    // "aa_a_"  => "aa_a__2"
+    // "aa_a1"  => "aa_a1_2"
+    // "aa_1a"  => "aa_1a_2"
+    // "aa_1"   => "aa_2"
+    // "aa_2"   => "aa_3"
+
+    size_t nLastIndex = rOldName.rfind('_');
     sal_Int32 nOldNumber = 1;
-    if (nLastIndex >= 0)
+    OUString aPrefix;
+    if (nLastIndex != std::u16string_view::npos)
     {
         ++nLastIndex;
-        std::u16string_view sLastPart(sOldName.subView(nLastIndex));
+        std::u16string_view sLastPart(rOldName.substr(nLastIndex));
         nOldNumber = o3tl::toInt32(sLastPart);
 
-        // When no number found, add number at the end.
-        // When there is a literal "0" at the end, keep the "lastIndex" from 
above
-        // (OUString::toInt32() also returns 0 on failure)
-        if (nOldNumber == 0 && sLastPart != u"0")
+        // If that number is exactly at the end then increment the number; else
+        // append "_" and number.
+        // toInt32() returns 0 on failure and also stops at trailing non-digit
+        // characters (toInt32("1a")==1).
+        if (OUString::number(nOldNumber) == sLastPart)
+            aPrefix = rOldName.substr(0, nLastIndex);
+        else
         {
+            aPrefix = OUString::Concat(rOldName) + "_";
             nOldNumber = 1;
-            nLastIndex = sOldName.getLength();
         }
     }
-    else // No "_" found, add number at the end
-        nLastIndex = sOldName.getLength();
+    else // No "_" found, append "_" and number.
+        aPrefix = OUString::Concat(rOldName) + "_";
     OUString sNewName;
     do
     {
-        sNewName = sOldName.subView(0, nLastIndex) + 
OUString::number(++nOldNumber);
+        sNewName = aPrefix + OUString::number(++nOldNumber);
     } while (namedDBs.findByName(sNewName) != nullptr);
     return sNewName;
 }
@@ -1544,17 +1560,23 @@ void ScDBCollection::UpdateMoveTab( SCTAB nOldPos, 
SCTAB nNewPos )
 
 void ScDBCollection::CopyToTable(SCTAB nOldPos, SCTAB nNewPos)
 {
+    // Create temporary copy of pointers to not insert in a set we are
+    // iterating over.
+    std::vector<const ScDBData*> aTemp;
+    aTemp.reserve( maNamedDBs.size());
     for (const auto& rxNamedDB : maNamedDBs)
     {
         if (rxNamedDB->GetTab() != nOldPos)
-            return;
-
-        OUString newName
-            = lcl_IncrementNumberInNamedRange(getNamedDBs(), 
rxNamedDB->GetName());
+            continue;
+        aTemp.emplace_back( rxNamedDB.get());
+    }
+    for (const auto& rxNamedDB : aTemp)
+    {
+        const OUString newName( lcl_IncrementNumberInNamedRange( maNamedDBs, 
rxNamedDB->GetName()));
         std::unique_ptr<ScDBData> pDataCopy = 
std::make_unique<ScDBData>(newName, *rxNamedDB);
         pDataCopy->UpdateMoveTab(nOldPos, nNewPos);
         pDataCopy->SetIndex(0);
-        getNamedDBs().insert(std::move(pDataCopy));
+        maNamedDBs.insert(std::move(pDataCopy));
     }
 }
 

Reply via email to