sc/source/core/tool/dbdata.cxx | 59 ++++++++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 18 deletions(-)
New commits:
commit 253354e829777d0667c44d6da8e2ed71f894b89c
Author: Eike Rathke <er...@redhat.com>
AuthorDate: Fri Sep 30 00:23:34 2022 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Oct 6 10:11:32 2022 +0200
Fix incrementing number in dbrange names during copying, tdf#145054
follow-up
This is a combination of 2 commits.
fix bug in lcl_IncrementNumberInNamedRange
from
commit 690e4852809ea21b5fd909298c5fa2a053fa0458
Date: Mon Jan 31 09:02:33 2022 +0100
Fix copying range when it doesn't contain a number
OUString::lastIndexOf() eithers returns an index or -1, so adding 1 to
it, means that nLastIndex is __always__ >= 0.
xChange-Id: I197f8c856a52534690d7a0d28e49cecf296dccb3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140704
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
(cherry picked from commit 36003e7644014cde9330bf45fee3815278a74035)
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.
xChange-Id: Ib83d317c69d821e8e8a2f1cd6791da9616ed188d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140717
Reviewed-by: Eike Rathke <er...@redhat.com>
Tested-by: Jenkins
(cherry picked from commit 48b9cbc742de3f6120986cb6cafc92eb5009da82)
Change-Id: Ib83d317c69d821e8e8a2f1cd6791da9616ed188d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140719
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140901
diff --git a/sc/source/core/tool/dbdata.cxx b/sc/source/core/tool/dbdata.cxx
index e060fb25e422..0e36eb9604ea 100644
--- a/sc/source/core/tool/dbdata.cxx
+++ b/sc/source/core/tool/dbdata.cxx
@@ -1010,30 +1010,47 @@ public:
};
OUString lcl_IncrementNumberInNamedRange(ScDBCollection::NamedDBs& namedDBs,
- const OUString& sOldName)
-{
- sal_Int32 nLastIndex = sOldName.lastIndexOf('_') + 1;
+ 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)
{
- std::u16string_view sLastPart(sOldName.subView(nLastIndex));
+ ++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;
}
@@ -1546,17 +1563,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));
}
}
