Rebased ref, commits from common ancestor:
commit aa614666336f6e23720cd7acea88399b88f9deff
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sat May 9 20:35:04 2015 +0100
Store pre-conversion value in cell annotation.
Change-Id: I67d8d1a7b0190b91107987a1ae4f03f2e91b06ca
diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx
index 605b79a..b156587 100644
--- a/sc/source/ui/view/viewfunc.cxx
+++ b/sc/source/ui/view/viewfunc.cxx
@@ -2971,11 +2971,27 @@ IMPL_LINK( ScViewFunc,
UnitConversionRecommendedHandler, UnitConversionPushButto
ScDocShellModificator aModificator( *pButton->mpDocSh );
+ OUString sOriginalValue = pButton->mpDoc->GetString( pButton->aCellAddress
);
+
pUnits->convertCellToHeaderUnit( pButton->aCellAddress,
pButton->mpDoc,
pButton->sHeaderUnit,
pButton->sCellUnit );
+ ScPostIt* pNote = pButton->mpDoc->GetOrCreateNote( pButton->aCellAddress );
+ OUString sCurrentNote = pNote->GetText();
+
+ OUString sConversionNote("Original input: " + sOriginalValue);
+
+ if (sCurrentNote.isEmpty())
+ {
+ pNote->SetText( pButton->aCellAddress, sConversionNote );
+ }
+ else
+ {
+ pNote->SetText( pButton->aCellAddress, sCurrentNote + "\n\n" +
sConversionNote );
+ }
+
aModificator.SetDocumentModified();
#endif
commit e5265133586501c5dccffed083fa2e1955ff765f
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sat May 9 20:34:10 2015 +0100
Set document modified on local unit conversion.
Change-Id: I6666668817a7987d14728fb1de1abe3711e34d9a
diff --git a/sc/source/ui/inc/viewfunc.hxx b/sc/source/ui/inc/viewfunc.hxx
index 20d6872..a6b36b8 100644
--- a/sc/source/ui/inc/viewfunc.hxx
+++ b/sc/source/ui/inc/viewfunc.hxx
@@ -380,7 +380,8 @@ private:
ScDocument* pDoc,
const OUString&
sHeaderUnit,
const ScAddress&
rHeaderAddress,
- const OUString& sCellUnit
);
+ const OUString& sCellUnit,
+ ScDocShell* pDocSh );
DECL_LINK( UnitConversionRecommendedHandler, UnitConversionPushButton* );
};
diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx
index 595c5a9..605b79a 100644
--- a/sc/source/ui/view/viewfunc.cxx
+++ b/sc/source/ui/view/viewfunc.cxx
@@ -586,7 +586,7 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB
nTab,
ScAddress aHeaderAddress;
if ( pUnits->isCellConversionRecommended( aAddress, pDoc,
sHeaderUnit, aHeaderAddress, sCellUnit ) ) {
- NotifyUnitConversionRecommended( aAddress, pDoc, sHeaderUnit,
aHeaderAddress, sCellUnit );
+ NotifyUnitConversionRecommended( aAddress, pDoc, sHeaderUnit,
aHeaderAddress, sCellUnit, pDocSh );
} else {
SfxViewFrame* pViewFrame =
GetViewData().GetViewShell()->GetFrame();
OUString sAddress = aAddress.Format( SCA_BITS, pDoc );
@@ -2904,26 +2904,30 @@ struct UnitConversionPushButton: public PushButton
ScDocument* mpDoc;
const OUString sHeaderUnit;
const OUString sCellUnit;
+ ScDocShell* mpDocSh;
UnitConversionPushButton( vcl::Window* pParent,
const ResId& rResId,
const ScAddress& rCellAddress,
ScDocument* pDoc,
const OUString& rsHeaderUnit,
- const OUString& rsCellUnit ):
+ const OUString& rsCellUnit,
+ ScDocShell* pDocSh ):
PushButton( pParent, rResId ),
aCellAddress( rCellAddress ),
mpDoc( pDoc ),
sHeaderUnit( rsHeaderUnit ),
- sCellUnit( rsCellUnit )
+ sCellUnit( rsCellUnit ),
+ mpDocSh( pDocSh )
{}
};
void ScViewFunc::NotifyUnitConversionRecommended( const ScAddress&
rCellAddress,
- ScDocument* pDoc,
- const OUString& rsHeaderUnit,
- const ScAddress&
rHeaderAddress,
- const OUString& rsCellUnit ) {
+ ScDocument* pDoc,
+ const OUString& rsHeaderUnit,
+ const ScAddress&
rHeaderAddress,
+ const OUString& rsCellUnit,
+ ScDocShell* pDocSh ) {
SfxViewFrame* pViewFrame = GetViewData().GetViewShell()->GetFrame();
// As with NotifyUnitErrorInFormula we use the cell address as the infobar
id.
@@ -2947,7 +2951,8 @@ void ScViewFunc::NotifyUnitConversionRecommended( const
ScAddress& rCellAddress,
rCellAddress,
pDoc,
rsHeaderUnit,
-
rsCellUnit );
+
rsCellUnit,
+
pDocSh );
pButtonConvertCell->SetClickHdl( LINK( this, ScViewFunc,
UnitConversionRecommendedHandler ) );
OUString sConvertText = pButtonConvertCell->GetText();
@@ -2964,10 +2969,14 @@ IMPL_LINK( ScViewFunc,
UnitConversionRecommendedHandler, UnitConversionPushButto
#ifdef ENABLE_CALC_UNITVERIFICATION
boost::shared_ptr< sc::units::Units > pUnits =
sc::units::Units::GetUnits();
+ ScDocShellModificator aModificator( *pButton->mpDocSh );
+
pUnits->convertCellToHeaderUnit( pButton->aCellAddress,
pButton->mpDoc,
pButton->sHeaderUnit,
pButton->sCellUnit );
+
+ aModificator.SetDocumentModified();
#endif
OUString sAddress;
commit 0a928a6c2111f87e0377222157340637400d22a7
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sat May 9 11:09:24 2015 +0100
Upgrade units test to use SfxModelFlags (rebase fixup).
Necessary due to afc728fe76fbf1afea725afd6ff5e9af92e10b08
Change-Id: Iea6dfb08a36f56485ed43a9c4cd2dcf652ff0b97
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index 3fbcaa9..e6ed779 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -72,9 +72,9 @@ void UnitsTest::setUp() {
ScDLL::Init();
m_xDocShRef = new ScDocShell(
- SFXMODEL_STANDARD |
- SFXMODEL_DISABLE_EMBEDDED_SCRIPTS |
- SFXMODEL_DISABLE_DOCUMENT_RECOVERY);
+ SfxModelFlags::EMBEDDED_OBJECT |
+ SfxModelFlags::DISABLE_EMBEDDED_SCRIPTS |
+ SfxModelFlags::DISABLE_DOCUMENT_RECOVERY);
mpDoc = &m_xDocShRef->GetDocument();
commit 420142127bf69385e3d5ebfeccda34914d7db1f3
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sat May 9 11:07:10 2015 +0100
loplugin:staticmethods
Change-Id: I31969836cc9e9147aaa370779fa281792efa9de2
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index 7b5fd25..3fbcaa9 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -363,8 +363,8 @@ void UnitsTest::testUnitVerification() {
}
void UnitsTest::testUnitFromFormatStringExtraction() {
- CPPUNIT_ASSERT(mpUnitsImpl->extractUnitStringFromFormat("\"weight:
\"0.0\"kg\"") == "kg");
- CPPUNIT_ASSERT(mpUnitsImpl->extractUnitStringFromFormat("#\"cm\"") ==
"cm");
+ CPPUNIT_ASSERT(UnitsImpl::extractUnitStringFromFormat("\"weight:
\"0.0\"kg\"") == "kg");
+ CPPUNIT_ASSERT(UnitsImpl::extractUnitStringFromFormat("#\"cm\"") == "cm");
}
void UnitsTest::testUnitValueStringSplitting() {
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index 379a6a7..0eea129 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -119,8 +119,8 @@ private:
bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
- OUString extractUnitStringFromFormat(const OUString& rFormatString);
- OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument*
pDoc);
+ static OUString extractUnitStringFromFormat(const OUString& rFormatString);
+ static OUString extractUnitStringForCell(const ScAddress& rAddress,
ScDocument* pDoc);
/**
* Retrieve the units for a given cell. This probes based on the usual
rules
commit cca98afc030ebde742df57a91bfbbb80953338c8
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Fri Apr 10 11:03:33 2015 +0100
Move and rename Range/Unit Stack.
This in preparation for implementing a combined Unit and Range iterator.
Change-Id: I08d28e175453f65c3696e9d1c6c20c7076d9b164
diff --git a/sc/Library_sc.mk b/sc/Library_sc.mk
index 5d8f086..04052c6 100644
--- a/sc/Library_sc.mk
+++ b/sc/Library_sc.mk
@@ -689,6 +689,7 @@ $(call gb_Library_add_exception_objects,sc,\
ifeq ($(ENABLE_CALC_UNITVERIFICATION),TRUE)
$(eval $(call gb_Library_add_exception_objects,sc,\
+ sc/source/core/units/raustack \
sc/source/core/units/units \
sc/source/core/units/unitsimpl \
sc/source/core/units/util \
diff --git a/sc/source/core/units/raustack.cxx
b/sc/source/core/units/raustack.cxx
new file mode 100644
index 0000000..f838acf
--- /dev/null
+++ b/sc/source/core/units/raustack.cxx
@@ -0,0 +1,54 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+#include "raustack.hxx"
+
+using namespace sc::units;
+
+RangeListIterator::RangeListIterator(ScDocument* pDoc, const ScRangeList&
rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+{
+}
+
+bool RangeListIterator::first() {
+ if (mRangeList.size() > 0) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[0]);
+ return mIt.first();
+ } else {
+ return false;
+ }
+}
+
+const ScAddress& RangeListIterator::GetPos() const {
+ return mIt.GetPos();
+}
+
+bool RangeListIterator::next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ // TODO: if emtpy - skip to next...?
+ return true;
+ } else {
+ return false;
+ }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
+
diff --git a/sc/source/core/units/raustack.hxx
b/sc/source/core/units/raustack.hxx
new file mode 100644
index 0000000..b5cf48b
--- /dev/null
+++ b/sc/source/core/units/raustack.hxx
@@ -0,0 +1,70 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+#ifndef INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+#define INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+
+#include <boost/variant.hpp>
+
+#include <address.hxx>
+#include <dociter.hxx>
+#include <rangelst.hxx>
+
+
+#include "utunit.hxx"
+
+namespace sc {
+namespace units {
+
+enum class RAUSItemType {
+ UNITS,
+ RANGE
+};
+
+struct RAUSItem {
+ RAUSItemType type;
+ boost::variant< ScRange, UtUnit > item;
+};
+
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
+
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList);
+
+ bool first();
+
+ const ScAddress& GetPos() const;
+
+ bool next();
+};
+
+}} // sc::units
+
+// class RangeAndUnitStack {
+
+
+
+// };
+
+// class RATSIterator {
+// public:
+// // TODO: need to be able to return non-initialisation
+// static RATSIterator getIterator(RangeAndTokenStack& rStack, ScDoc*
pDoc, int nItems);
+
+// }
+
+#endif // INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 99e03af..e8fb261 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -11,7 +11,6 @@
#include "util.hxx"
-#include "dociter.hxx"
#include "document.hxx"
#include "refdata.hxx"
#include "stringutil.hxx"
@@ -85,55 +84,7 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-class RangeListIterator {
-private:
- const ScRangeList mRangeList;
- ScDocument* mpDoc;
-
- ScCellIterator mIt;
- size_t nCurrentIndex;
-
-public:
- RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
- :
- mRangeList(rRangeList),
- mpDoc(pDoc),
- mIt(pDoc, ScRange()),
- nCurrentIndex(0)
- {
- }
-
- bool first() {
- if (mRangeList.size() > 0) {
- mIt = ScCellIterator(mpDoc, *mRangeList[0]);
- return mIt.first();
- } else {
- return false;
- }
- }
-
- const ScAddress& GetPos() const {
- return mIt.GetPos();
- }
-
- bool next() {
- if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
- return false;
- }
-
- if (mIt.next()) {
- return true;
- } else if (++nCurrentIndex < mRangeList.size()) {
- mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
- mIt.first();
- return true;
- } else {
- return false;
- }
- }
-};
-
-UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack,
const formula::FormulaToken* pToken, ScDocument* pDoc) {
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< RAUSItem >& rStack,
const formula::FormulaToken* pToken, ScDocument* pDoc) {
const OpCode aOpCode = pToken->GetOpCode();
auto nOpCode = static_cast<std::underlying_type<const
OpCode>::type>(aOpCode);
@@ -147,7 +98,7 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
StackItem >& rStack, const
if (rStack.size() == 0) {
SAL_WARN("sc.units", "Single item opcode failed (no stack items,
or range used)");
return { UnitsStatus::UNITS_INVALID, boost::none };
- } else if (rStack.top().type != StackItemType::UNITS) {
+ } else if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
@@ -183,13 +134,13 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
StackItem >& rStack, const
return { UnitsStatus::UNITS_INVALID, boost::none };
}
- if (rStack.top().type != StackItemType::UNITS) {
+ if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
UtUnit pSecondUnit = boost::get<UtUnit>(rStack.top().item);
rStack.pop();
- if (rStack.top().type != StackItemType::UNITS) {
+ if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
UtUnit pFirstUnit = boost::get<UtUnit>(rStack.top().item);
@@ -240,12 +191,12 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
StackItem >& rStack, const
for ( ; nParams > 0; nParams--) {
switch (rStack.top().type) {
- case StackItemType::UNITS:
+ case RAUSItemType::UNITS:
{
aUnitsStack.push(boost::get< UtUnit >(rStack.top().item));
break;
}
- case StackItemType::RANGE:
+ case RAUSItemType::RANGE:
{
aRangeList.Append(boost::get< ScRange >(rStack.top().item));
break;
@@ -545,7 +496,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
pArray->Dump();
#endif
- stack< StackItem > aStack;
+ stack< RAUSItem > aStack;
for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken =
pArray->NextRPN()) {
switch (pToken->GetType()) {
@@ -564,14 +515,14 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
return false;
}
- aStack.push( { StackItemType::UNITS, aUnit } );
+ aStack.push( { RAUSItemType::UNITS, aUnit } );
break;
}
case formula::svDoubleRef:
{
ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
ScRange aRange = pDoubleRef->toAbs(rFormulaAddress);
- aStack.push( { StackItemType::RANGE, aRange } );
+ aStack.push( { RAUSItemType::RANGE, aRange } );
break;
}
@@ -588,7 +539,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
case UnitsStatus::UNITS_VALID:
assert(aResult.units); // ensure that we have the optional unit
assert(aResult.units->isValid());
- aStack.push( { StackItemType::UNITS, aResult.units.get() } );
+ aStack.push( { RAUSItemType::UNITS, aResult.units.get() } );
break;
}
@@ -603,7 +554,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
UtUnit::createUnit("", aScale, mpUnitSystem); // Get the
dimensionless unit 1
aScale = aScale*(pToken->GetDouble());
- aStack.push( { StackItemType::UNITS, aScale } );
+ aStack.push( { RAUSItemType::UNITS, aScale } );
break;
}
@@ -619,7 +570,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
if (aStack.size() != 1) {
SAL_WARN("sc.units", "Wrong number of units on stack, should be 1,
actual number: " << aStack.size());
return false;
- } else if (aStack.top().type != StackItemType::UNITS) {
+ } else if (aStack.top().type != RAUSItemType::UNITS) {
SAL_WARN("sc.units", "End of verification: item on stack does not
contain units");
return false;
}
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index 6abffdd..379a6a7 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -13,7 +13,6 @@
#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
-#include <boost/variant.hpp>
#include <boost/weak_ptr.hpp>
#include <formula/opcode.hxx>
@@ -25,6 +24,7 @@
#include "rangelst.hxx"
#include "units.hxx"
+#include "raustack.hxx"
#include "utunit.hxx"
#include <stack>
@@ -42,16 +42,6 @@ namespace test {
class UnitsTest;
}
-enum class StackItemType {
- UNITS,
- RANGE
-};
-
-struct StackItem {
- StackItemType type;
- boost::variant< ScRange, UtUnit > item;
-};
-
enum class UnitsStatus {
UNITS_VALID,
UNITS_UNKNOWN,
@@ -103,7 +93,7 @@ public:
const OUString& rsOldUnit)
SAL_OVERRIDE;
private:
- UnitsResult getOutputUnitsForOpCode(std::stack< StackItem >& rStack, const
formula::FormulaToken* pToken, ScDocument* pDoc);
+ UnitsResult getOutputUnitsForOpCode(std::stack< RAUSItem >& rStack, const
formula::FormulaToken* pToken, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit bf0d568a89e4b40b15bcf71d94c7082ba6357d9b
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Wed Apr 1 14:54:20 2015 +0200
Check output unit against header definition during verification.
Change-Id: I98a706d80eb442d274fc111fb6c22e43d79fb9ff
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index fbd0c2d..99e03af 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -616,7 +616,23 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
}
}
- // TODO: only fail if actual parsing fails?
+ if (aStack.size() != 1) {
+ SAL_WARN("sc.units", "Wrong number of units on stack, should be 1,
actual number: " << aStack.size());
+ return false;
+ } else if (aStack.top().type != StackItemType::UNITS) {
+ SAL_WARN("sc.units", "End of verification: item on stack does not
contain units");
+ return false;
+ }
+
+ OUString sUnitString;
+ ScAddress aAddress;
+
+ UtUnit aHeaderUnit = findHeaderUnitForCell(rFormulaAddress, pDoc,
sUnitString, aAddress);
+ UtUnit aResultUnit = boost::get< UtUnit>(aStack.top().item);
+
+ if (aHeaderUnit.isValid() && aHeaderUnit != aResultUnit) {
+ return false;
+ }
return true;
}
commit 6b23a4f2a1e9f95894dd704cd104740e4cbfd9e0
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Wed Apr 1 13:48:01 2015 +0200
Use correct stack for multiparam opcodes.
Also add an appropriate unit test.
Change-Id: Ie2e9cce74563dea80b33f5e8238ba6ad1aae1b49
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index ac1bfa5..7b5fd25 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -291,7 +291,6 @@ void UnitsTest::testUnitVerification() {
// SUM("cm"&"kg") - multiple ranges
address.IncRow();
- // TODO: by default / when testing: separator is...?
mpDoc->SetFormula(address, "=SUM(A1:A3,B1:B3)");
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
@@ -325,6 +324,23 @@ void UnitsTest::testUnitVerification() {
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // Test that multiple arguments of mixed types work too
+ // Adding multiple cells from one column is ok
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1,A2:A3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // But mixing the columns fails because of mixed units
+ // (This test is primarily to ensure that we can handle arbitrary numbers
+ // of arguments.)
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1,A2:A3,B1:B3,C1,C2,C3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
// Do a quick sanity check for all the other supported functions
// The following all expect identically united inputs, and should
// preserve that unit:
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index d49799f..fbd0c2d 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -274,7 +274,7 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
StackItem >& rStack, const
return { UnitsStatus::UNITS_INVALID, boost::none };
}
}
- rStack.pop();
+ aUnitsStack.pop();
}
if (aIt.first()) {
commit d73c16d4c55ad6c4dba2e4ea475cfd356cf73f85
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Wed Apr 1 13:21:35 2015 +0200
Refactor and rewrite range and multiparam opcode handling.
Change-Id: Ic52bfb7daae44ea8f8af710fa70f1f30150fc274
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 6057a0e..d49799f 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -85,26 +85,76 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack,
const OpCode& rOpCode) {
- UtUnit pOut;
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
- auto nOpCode = static_cast<std::underlying_type<const
OpCode>::type>(rOpCode);
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+ {
+ }
+ bool first() {
+ if (mRangeList.size() > 0) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[0]);
+ return mIt.first();
+ } else {
+ return false;
+ }
+ }
+
+ const ScAddress& GetPos() const {
+ return mIt.GetPos();
+ }
+
+ bool next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ return true;
+ } else {
+ return false;
+ }
+ }
+};
+
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack,
const formula::FormulaToken* pToken, ScDocument* pDoc) {
+ const OpCode aOpCode = pToken->GetOpCode();
+ auto nOpCode = static_cast<std::underlying_type<const
OpCode>::type>(aOpCode);
+
+ UtUnit pOut;
// TODO: sc/source/core/tool/parclass.cxx has a mapping of opcodes to
possible operands, which we
// should probably be using in practice.
if (nOpCode >= SC_OPCODE_START_UN_OP &&
nOpCode < SC_OPCODE_STOP_UN_OP) {
- if (!(rUnitStack.size() >= 1)) {
- SAL_WARN("sc.units", "no units on stack for unary operation");
+ if (rStack.size() == 0) {
+ SAL_WARN("sc.units", "Single item opcode failed (no stack items,
or range used)");
return { UnitsStatus::UNITS_INVALID, boost::none };
+ } else if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
- UtUnit pUnit = rUnitStack.top();
- rUnitStack.pop();
+ UtUnit pUnit =
boost::get<UtUnit>(rStack.top().item);//rStack.top().item.get< UtUnit >();
+ rStack.pop();
- switch (rOpCode) {
+ switch (aOpCode) {
case ocNot:
if (!pUnit.isDimensionless()) {
return { UnitsStatus::UNITS_INVALID, boost::none };
@@ -128,19 +178,24 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
UtUnit >& rUnitStack, cons
} else if (nOpCode >= SC_OPCODE_START_BIN_OP &&
nOpCode < SC_OPCODE_STOP_BIN_OP) {
- if (!(rUnitStack.size() >= 2)) {
- SAL_WARN("sc.units", "less than two units on stack when attempting
binary operation");
- // TODO: what should we be telling the user in this case? Can this
even happen (i.e.
- // should we just be asserting here?)
+ if (rStack.size() < 2) {
+ SAL_WARN("sc.units", "less than two items on stack when attempting
binary operation");
return { UnitsStatus::UNITS_INVALID, boost::none };
}
- UtUnit pSecondUnit = rUnitStack.top();
- rUnitStack.pop();
- UtUnit pFirstUnit = rUnitStack.top();
- rUnitStack.pop();
+ if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ UtUnit pSecondUnit = boost::get<UtUnit>(rStack.top().item);
+ rStack.pop();
+
+ if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ UtUnit pFirstUnit = boost::get<UtUnit>(rStack.top().item);
+ rStack.pop();
- switch (rOpCode) {
+ switch (aOpCode) {
case ocAdd:
// Adding and subtracting both require the same units on both sides
// hence we can just fall through / use the same logic.
@@ -164,90 +219,111 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack<
UtUnit >& rUnitStack, cons
SAL_INFO("sc.units", "unit verification not supported for opcode:
" << nOpCode);
assert(false);
}
+ } else if (nOpCode >= SC_OPCODE_START_2_PAR &&
+ nOpCode < SC_OPCODE_STOP_2_PAR) {
+ sal_uInt8 nParams = pToken->GetByte();
- } else {
- SAL_INFO("sc.units", "unit verification not supported for opcode: " <<
nOpCode);
- return { UnitsStatus::UNITS_UNKNOWN, boost::none };
- }
- // TODO: else if unary, or no params, or ...
- // TODO: implement further sensible opcode handling
- return { UnitsStatus::UNITS_VALID, pOut };
-}
-
-class RangeListIterator {
-private:
- const ScRangeList mRangeList;
- ScDocument* mpDoc;
-
- ScCellIterator mIt;
- size_t nCurrentIndex;
+ assert(nParams <= rStack.size());
-public:
- RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
- :
- mRangeList(rRangeList),
- mpDoc(pDoc),
- mIt(pDoc, ScRange()),
- nCurrentIndex(0)
- {
- if (rRangeList.size() > 0) {
- mIt = ScCellIterator(pDoc, *rRangeList[0]);
- mIt.first();
+ // If there are no input parameters then the output unit is undefined.
+ // (In practice this would probably be nonsensical, but isn't a unit
+ // error per-se.)
+ if (nParams == 0) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
- }
- const ScAddress& GetPos() const {
- return mIt.GetPos();
- }
+ // This is still quite an ugly solution, even better would maybe be to
have
+ // a StackUnitIterator which iterates all the appropriate units given
a number of stack items
+ // to iterate over?
+ ScRangeList aRangeList;
+ stack< UtUnit > aUnitsStack;
+
+ for ( ; nParams > 0; nParams--) {
+ switch (rStack.top().type) {
+ case StackItemType::UNITS:
+ {
+ aUnitsStack.push(boost::get< UtUnit >(rStack.top().item));
+ break;
+ }
+ case StackItemType::RANGE:
+ {
+ aRangeList.Append(boost::get< ScRange >(rStack.top().item));
+ break;
+ }
+ }
- bool next() {
- if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
- return false;
+ rStack.pop();
}
- if (mIt.next()) {
- return true;
- } else if (++nCurrentIndex < mRangeList.size()) {
- mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
- mIt.first();
- return true;
- } else {
- return false;
- }
- }
-};
+ RangeListIterator aIt(pDoc, aRangeList);
-UnitsResult UnitsImpl::getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode,
const ScRangeList& rRangeList, ScDocument* pDoc) {
- RangeListIterator aIt(pDoc, rRangeList);
+ switch (aOpCode) {
+ case ocSum:
+ case ocMin:
+ case ocMax:
+ case ocAverage:
+ {
+ boost::optional< UtUnit > aFirstUnit;
+
+ while (aUnitsStack.size() > 0) {
+ if (!aFirstUnit) {
+ aFirstUnit = aUnitsStack.top();
+ } else {
+ UtUnit aCurrentUnit(aUnitsStack.top());
+ if (aFirstUnit.get() != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+ rStack.pop();
+ }
- switch (rOpCode) {
- case ocSum:
- case ocMin:
- case ocMax:
- case ocAverage:
- {
- UtUnit aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ if (aIt.first()) {
+ do {
+ if (!aFirstUnit) {
+ aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ } else {
+ UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(),
pDoc);
+ if (aFirstUnit.get() != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+ } while (aIt.next());
- while (aIt.next()) {
- UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(), pDoc);
- if (aFirstUnit != aCurrentUnit) {
- return { UnitsStatus::UNITS_INVALID, boost::none };
}
+
+ return { UnitsStatus::UNITS_VALID, aFirstUnit };
}
+ case ocProduct:
+ {
+ // To avoid having to detect when we get the first unit (i.e. to
avoid
+ // the play with the optinal< UtUnit > as above), we just start
with
+ // the dimensionless Unit 1 and multiply from there.
+ // This can also be avoided if we implement a combined Unit and
Range
+ // Iterator (see above for more info).
+ UtUnit aUnit;
+ UtUnit::createUnit("", aUnit, mpUnitSystem);
+
+ while (aUnitsStack.size() > 0) {
+ aUnit *= aUnitsStack.top();
+ aUnitsStack.pop();
+ }
- return { UnitsStatus::UNITS_VALID, aFirstUnit };
- }
- case ocProduct:
- {
- UtUnit aUnit = getUnitForCell(aIt.GetPos(), pDoc);
- while (aIt.next()) {
- aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ if (aIt.first()) {
+ do {
+ aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ } while (aIt.next());
+ }
+
+ return { UnitsStatus::UNITS_VALID, aUnit };
}
- return { UnitsStatus::UNITS_VALID, aUnit };
- }
- default:
+ default:
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ } else {
+ SAL_INFO("sc.units", "unit verification not supported for opcode: " <<
nOpCode);
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
+ return { UnitsStatus::UNITS_VALID, pOut };
}
OUString UnitsImpl::extractUnitStringFromFormat(const OUString& rFormatString)
{
@@ -469,62 +545,42 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
pArray->Dump();
#endif
- stack< UtUnit > aUnitStack;
+ stack< StackItem > aStack;
for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken =
pArray->NextRPN()) {
switch (pToken->GetType()) {
case formula::svSingleRef:
{
- UtUnit pUnit(getUnitForRef(pToken, rFormulaAddress, pDoc));
+ UtUnit aUnit(getUnitForRef(pToken, rFormulaAddress, pDoc));
- if (!pUnit.isValid()) {
+ if (!aUnit.isValid()) {
SAL_INFO("sc.units", "no unit returned for scSingleRef,
ut_status: " << getUTStatus());
// This only happens in case of parsing (or system) errors.
// However maybe we should be returning "unverified" for
// unparseable formulas?
// (or even have a "can't be verified" state too?)
- // see below for more.
+ // see below for more.x
return false;
}
- aUnitStack.push(pUnit);
+ aStack.push( { StackItemType::UNITS, aUnit } );
break;
}
case formula::svDoubleRef:
{
ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
- ScRangeList aRangeList(pDoubleRef->toAbs(rFormulaAddress));
-
- // Functions operating on DoubleRefs seem to have a variable
number of input
- // arguments (which will all preceed the opcode in the
TokenArray), hence
- // we read all the ranges in first before operating on them.
- // The appropriate handling of the opcode depends on the specific
opcode
- // (i.e. identical units needed for sum, but not for
multiplication), hence
- // we need to handle that opcode here as opposed to as part of the
usual flow
- // (where we would simply push the units onto the stack).
- pToken = pArray->NextRPN();
- while (pToken->GetType() == formula::svDoubleRef) {
- pDoubleRef = pToken->GetDoubleRef();
- aRangeList.Append(pDoubleRef->toAbs(rFormulaAddress));
- pToken = pArray->NextRPN();
- }
-
- if (!pToken) {
- // It's possible to have DoubleRef as the last token, e.g the
simplest
- // example being "=A1:B1" - in this case the formula can't be
evaluated
- // anyways, so giving up on unit verification is the most
sensible option.
- // (I.e. the formula ends up spewing out #VALUE to the
end-user - there's
- // no point in doing any verification here.)
- return true;
- }
-
- UnitsResult
aResult(getOutputUnitForDoubleRefOpcode(pToken->GetOpCode(), aRangeList, pDoc));
+ ScRange aRange = pDoubleRef->toAbs(rFormulaAddress);
+ aStack.push( { StackItemType::RANGE, aRange } );
+ break;
+ }
+ case formula::svByte:
+ {
+ UnitsResult aResult = getOutputUnitsForOpCode(aStack, pToken,
pDoc);
switch (aResult.status) {
case UnitsStatus::UNITS_INVALID:
- SAL_INFO("sc.units", "svDoubleRef opcode unit error detected");
return false;
case UnitsStatus::UNITS_UNKNOWN:
// Unsupported hence we stop processing.
@@ -532,26 +588,12 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
case UnitsStatus::UNITS_VALID:
assert(aResult.units); // ensure that we have the optional unit
assert(aResult.units->isValid());
- aUnitStack.push(aResult.units.get());
+ aStack.push( { StackItemType::UNITS, aResult.units.get() } );
break;
}
break;
}
- case formula::svByte:
- {
- UtUnit pOut = getOutputUnitsForOpCode(aUnitStack,
pToken->GetOpCode());
-
- // A null unit indicates either invalid units and/or other
erronous input
- // i.e. is an indication that getOutputUnitsForOpCode failed.
- if (pOut.isValid()) {
- aUnitStack.push(pOut);
- } else {
- return false;
- }
-
- break;
- }
// As far as I can tell this is only used for an actual numerical value
// in which case we just want to use it as scaling factor
// (for example [m] + [cm]/100 would be a valid operation)
@@ -561,7 +603,8 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
UtUnit::createUnit("", aScale, mpUnitSystem); // Get the
dimensionless unit 1
aScale = aScale*(pToken->GetDouble());
- aUnitStack.push(aScale);
+ aStack.push( { StackItemType::UNITS, aScale } );
+
break;
}
default:
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index aacf9bb..6abffdd 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -13,6 +13,7 @@
#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
+#include <boost/variant.hpp>
#include <boost/weak_ptr.hpp>
#include <formula/opcode.hxx>
@@ -41,6 +42,16 @@ namespace test {
class UnitsTest;
}
+enum class StackItemType {
+ UNITS,
+ RANGE
+};
+
+struct StackItem {
+ StackItemType type;
+ boost::variant< ScRange, UtUnit > item;
+};
+
enum class UnitsStatus {
UNITS_VALID,
UNITS_UNKNOWN,
@@ -92,8 +103,7 @@ public:
const OUString& rsOldUnit)
SAL_OVERRIDE;
private:
- UnitsResult getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack,
const OpCode& rOpCode);
- UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const
ScRangeList& rRange, ScDocument* pDoc);
+ UnitsResult getOutputUnitsForOpCode(std::stack< StackItem >& rStack, const
formula::FormulaToken* pToken, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit 8d392e360b39a8bba1310fbcdf0e23366ed37812
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 19:57:04 2015 +0200
Use UnitsResult for processing of opcodes too.
Change-Id: I7bbbf4ff0bcbd01d70fd929cf62537ceec364c83
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 3a181b5..6057a0e 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -85,7 +85,7 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const
OpCode& rOpCode) {
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack,
const OpCode& rOpCode) {
UtUnit pOut;
auto nOpCode = static_cast<std::underlying_type<const
OpCode>::type>(rOpCode);
@@ -98,7 +98,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >&
rUnitStack, const OpC
if (!(rUnitStack.size() >= 1)) {
SAL_WARN("sc.units", "no units on stack for unary operation");
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
UtUnit pUnit = rUnitStack.top();
@@ -107,7 +107,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >&
rUnitStack, const OpC
switch (rOpCode) {
case ocNot:
if (!pUnit.isDimensionless()) {
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
// We just keep the same unit (in this case no unit) so can
// fall through.
@@ -132,7 +132,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >&
rUnitStack, const OpC
SAL_WARN("sc.units", "less than two units on stack when attempting
binary operation");
// TODO: what should we be telling the user in this case? Can this
even happen (i.e.
// should we just be asserting here?)
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
UtUnit pSecondUnit = rUnitStack.top();
@@ -150,6 +150,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >&
rUnitStack, const OpC
pOut = pFirstUnit;
SAL_INFO("sc.units", "verified equality for unit " <<
pFirstUnit);
} else {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
// TODO: notify/link UI.
}
break;
@@ -166,11 +167,11 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit
>& rUnitStack, const OpC
} else {
SAL_INFO("sc.units", "unit verification not supported for opcode: " <<
nOpCode);
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
// TODO: else if unary, or no params, or ...
// TODO: implement further sensible opcode handling
-
- return pOut;
+ return { UnitsStatus::UNITS_VALID, pOut };
}
class RangeListIterator {
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index d6d001c..aacf9bb 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -92,7 +92,7 @@ public:
const OUString& rsOldUnit)
SAL_OVERRIDE;
private:
- UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const
OpCode& rOpCode);
+ UnitsResult getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack,
const OpCode& rOpCode);
UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const
ScRangeList& rRange, ScDocument* pDoc);
/**
commit 72d769219cdff0119ce9b6cc4dc67e8334f9e998
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 11:12:42 2015 +0200
Implement verification of sum/product and other opcodes for ranges.
Change-Id: I8d0d4cf46bddd585e633b5ee297b6bd7e0a2cf3b
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index af5fff5..ac1bfa5 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -79,6 +79,11 @@ void UnitsTest::setUp() {
mpDoc = &m_xDocShRef->GetDocument();
mpUnitsImpl = UnitsImpl::GetUnits();
+
+ // Set separators for formulae - for now just argument separators (no
matrix separators)
+ ScFormulaOptions aOptions;
+ aOptions.SetFormulaSepArg(",");
+ m_xDocShRef->SetFormulaOptions(aOptions);
}
void UnitsTest::tearDown() {
@@ -183,11 +188,19 @@ void UnitsTest::testUnitVerification() {
mpDoc->SetNumberFormat(address, nKeyS);
mpDoc->SetValue(address, 3);
- // 4th column: 5cm/s
+ // 4th column: 5cm/s, 10cm/s, 15cm/s
address = ScAddress(3, 0, 0);
mpDoc->SetNumberFormat(address, nKeyCM_S);
mpDoc->SetValue(address, 5);
+ address.IncRow();
+ mpDoc->SetNumberFormat(address, nKeyCM_S);
+ mpDoc->SetValue(address, 10);
+
+ address.IncRow();
+ mpDoc->SetNumberFormat(address, nKeyCM_S);
+ mpDoc->SetValue(address, 15);
+
// 5th column: 1m
address = ScAddress(4, 0, 0);
mpDoc->SetNumberFormat(address, nKeyM);
@@ -256,11 +269,81 @@ void UnitsTest::testUnitVerification() {
// And scaling doesn't help when adding incompatible units (kg + 100*m)
address = ScAddress(0, 13, 0);
- mpDoc->SetFormula(address, "=B1+100E1");
+ mpDoc->SetFormula(address, "=B1+100*E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // Test Ranges:
+ // SUM("kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm"&"kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm"&"kg") - multiple ranges
+ address.IncRow();
+ // TODO: by default / when testing: separator is...?
+ mpDoc->SetFormula(address, "=SUM(A1:A3,B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm")+SUM("kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:A3)+SUM(B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm")/SUM("s")+SUM("cm/s")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:A3)/SUM(C1:C3)+SUM(D1:D3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // PRODUCT("cm/","s")+"cm"
+ address.IncRow();
+ mpDoc->SetFormula(address, "=PRODUCT(C1:D1)+A1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // PRODUCT("cm/","s")+"kg"
+ address.IncRow();
+ mpDoc->SetFormula(address, "=PRODUCT(C1:D1)+B1");
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // Do a quick sanity check for all the other supported functions
+ // The following all expect identically united inputs, and should
+ // preserve that unit:
+ std::vector<OUString> aPreservingFuncs{ "SUM", "AVERAGE", "MIN", "MAX" };
+ for (const OUString& aFunc: aPreservingFuncs) {
+ // FOO(cm) + cm
+ address.IncRow();
+ mpDoc->SetFormula(address, "=" + aFunc + "(A1:A2)+A3");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // FOO(cm) + kg
+ address.IncRow();
+ mpDoc->SetFormula(address, "=" + aFunc + "(A1:A2)+B3");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ }
}
void UnitsTest::testUnitFromFormatStringExtraction() {
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 6d280b4..3a181b5 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -11,6 +11,7 @@
#include "util.hxx"
+#include "dociter.hxx"
#include "document.hxx"
#include "refdata.hxx"
#include "stringutil.hxx"
@@ -172,6 +173,82 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >&
rUnitStack, const OpC
return pOut;
}
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
+
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+ {
+ if (rRangeList.size() > 0) {
+ mIt = ScCellIterator(pDoc, *rRangeList[0]);
+ mIt.first();
+ }
+ }
+
+ const ScAddress& GetPos() const {
+ return mIt.GetPos();
+ }
+
+ bool next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ return true;
+ } else {
+ return false;
+ }
+ }
+};
+
+UnitsResult UnitsImpl::getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode,
const ScRangeList& rRangeList, ScDocument* pDoc) {
+ RangeListIterator aIt(pDoc, rRangeList);
+
+ switch (rOpCode) {
+ case ocSum:
+ case ocMin:
+ case ocMax:
+ case ocAverage:
+ {
+ UtUnit aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+
+ while (aIt.next()) {
+ UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ if (aFirstUnit != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+
+ return { UnitsStatus::UNITS_VALID, aFirstUnit };
+ }
+ case ocProduct:
+ {
+ UtUnit aUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ while (aIt.next()) {
+ aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ }
+ return { UnitsStatus::UNITS_VALID, aUnit };
+ }
+ default:
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+}
+
OUString UnitsImpl::extractUnitStringFromFormat(const OUString& rFormatString)
{
// TODO: decide what we do for different subformats? Simplest solution
// would be to not allow unit storage for multiple subformats.
@@ -413,6 +490,53 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
aUnitStack.push(pUnit);
break;
}
+ case formula::svDoubleRef:
+ {
+ ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
+ ScRangeList aRangeList(pDoubleRef->toAbs(rFormulaAddress));
+
+ // Functions operating on DoubleRefs seem to have a variable
number of input
+ // arguments (which will all preceed the opcode in the
TokenArray), hence
+ // we read all the ranges in first before operating on them.
+ // The appropriate handling of the opcode depends on the specific
opcode
+ // (i.e. identical units needed for sum, but not for
multiplication), hence
+ // we need to handle that opcode here as opposed to as part of the
usual flow
+ // (where we would simply push the units onto the stack).
+ pToken = pArray->NextRPN();
+ while (pToken->GetType() == formula::svDoubleRef) {
+ pDoubleRef = pToken->GetDoubleRef();
+ aRangeList.Append(pDoubleRef->toAbs(rFormulaAddress));
+ pToken = pArray->NextRPN();
+ }
+
+ if (!pToken) {
+ // It's possible to have DoubleRef as the last token, e.g the
simplest
+ // example being "=A1:B1" - in this case the formula can't be
evaluated
+ // anyways, so giving up on unit verification is the most
sensible option.
+ // (I.e. the formula ends up spewing out #VALUE to the
end-user - there's
+ // no point in doing any verification here.)
+ return true;
+ }
+
+ UnitsResult
aResult(getOutputUnitForDoubleRefOpcode(pToken->GetOpCode(), aRangeList, pDoc));
+
+
+ switch (aResult.status) {
+ case UnitsStatus::UNITS_INVALID:
+ SAL_INFO("sc.units", "svDoubleRef opcode unit error detected");
+ return false;
+ case UnitsStatus::UNITS_UNKNOWN:
+ // Unsupported hence we stop processing.
+ return true;
+ case UnitsStatus::UNITS_VALID:
+ assert(aResult.units); // ensure that we have the optional unit
+ assert(aResult.units->isValid());
+ aUnitStack.push(aResult.units.get());
+ break;
+ }
+
+ break;
+ }
case formula::svByte:
{
UtUnit pOut = getOutputUnitsForOpCode(aUnitStack,
pToken->GetOpCode());
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index e1427ed..d6d001c 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -10,6 +10,7 @@
#ifndef INCLUDED_SC_SOURCE_CORE_UNITS_UNITSIMPL_HXX
#define INCLUDED_SC_SOURCE_CORE_UNITS_UNITSIMPL_HXX
+#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
@@ -20,7 +21,9 @@
#include <udunits2.h>
-#include <units.hxx>
+#include "rangelst.hxx"
+#include "units.hxx"
+
#include "utunit.hxx"
#include <stack>
@@ -38,6 +41,22 @@ namespace test {
class UnitsTest;
}
+enum class UnitsStatus {
+ UNITS_VALID,
+ UNITS_UNKNOWN,
+ UNITS_INVALID
+};
+
+/**
+ * The result for a given units operation.
+ * If UNITS_VALID then the resulting unit is also included,
+ * otherwise units is empty.
+ */
+struct UnitsResult {
+ UnitsStatus status;
+ boost::optional<UtUnit> units;
+};
+
class UnitsImpl: public Units {
friend class test::UnitsTest;
@@ -74,6 +93,7 @@ public:
private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const
OpCode& rOpCode);
+ UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const
ScRangeList& rRange, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit b921c4964db4cb0dfae556bcfa40aaec3b86abf0
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 09:50:45 2015 +0200
Move declarations to more sensible place.
Change-Id: Ieb60b2a263a4d700f366b0aae7128e5a66ac4205
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index 91730f7..e1427ed 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -74,8 +74,6 @@ public:
private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const
OpCode& rOpCode);
- OUString extractUnitStringFromFormat(const OUString& rFormatString);
- OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument*
pDoc);
/**
* Find and extract a Unit in the standard header notation,
@@ -101,6 +99,9 @@ private:
bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
+ OUString extractUnitStringFromFormat(const OUString& rFormatString);
+ OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument*
pDoc);
+
/**
* Retrieve the units for a given cell. This probes based on the usual
rules
* for cell annotation/column header.
commit 7f87b021220584e8080a06a3875b18b83f28f5fe
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 09:50:05 2015 +0200
Implement UtUnit::operator*=
Change-Id: I2733154c3d7c42f3af273a0267158e8fa5957c23
diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx
index 24225dd..c137f67 100644
--- a/sc/source/core/units/utunit.hxx
+++ b/sc/source/core/units/utunit.hxx
@@ -90,6 +90,11 @@ public:
return UtUnit(ut_multiply(this->get(), rUnit.get()));
}
+ UtUnit& operator*=(const UtUnit& rUnit) {
+ reset(ut_multiply(this->get(), rUnit.get()));
+ return *this;
+ }
+
/**
* Multiply a unit by a given constant.
* This scales the unit by the inverse of that constant,
commit 34898954b9b08708bb228e8065bf4c344661975a
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 09:49:27 2015 +0200
Split getUnitForRef into mapping and retrieval.
We will need getUnitForCell when verifying formulas operating on ranges
(where we need to iterate over cells in a range, and retrieve those
cells units - as opposed to retrieving the unit for a reference in a
token array).
Change-Id: I6652057bcb3ee42becb4c5425b06ce544cd400a6
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 9492dd1..6d280b4 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -314,19 +314,8 @@ bool UnitsImpl::extractUnitFromHeaderString(const
OUString& rsHeader, UtUnit& aU
return false;
}
-UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress&
rFormulaAddress,
- ScDocument* pDoc) {
- assert(pToken->GetType() == formula::svSingleRef);
-
- ScSingleRefData* pRef = pToken->GetSingleRef();
- assert(pRef);
-
- // Addresses can/will be relative to the formula, for extracting
- // units however we will need to get the absolute address (i.e.
- // by adding the current address to the relative formula address).
- const ScAddress aInputAddress = pRef->toAbs( rFormulaAddress );
-
- OUString sUnitString = extractUnitStringForCell(aInputAddress, pDoc);
+UtUnit UnitsImpl::getUnitForCell(const ScAddress& rCellAddress, ScDocument*
pDoc) {
+ OUString sUnitString = extractUnitStringForCell(rCellAddress, pDoc);
UtUnit aUnit;
if (sUnitString.getLength() > 0 &&
@@ -336,15 +325,31 @@ UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken,
const ScAddress& rFormulaA
OUString aHeaderUnitString; // Unused -- passed by reference below
ScAddress aHeaderAddress; // Unused too
- UtUnit aHeaderUnit = findHeaderUnitForCell(aInputAddress, pDoc,
aHeaderUnitString, aHeaderAddress);
+ UtUnit aHeaderUnit = findHeaderUnitForCell(rCellAddress, pDoc,
aHeaderUnitString, aHeaderAddress);
if (aHeaderUnit.isValid())
return aHeaderUnit;
- SAL_INFO("sc.units", "no unit obtained for token at cell " <<
aInputAddress.GetColRowString());
+ SAL_INFO("sc.units", "no unit obtained for token at cell " <<
rCellAddress.GetColRowString());
// We return the dimensionless unit 1 if we don't find any other data
suggesting a unit.
UtUnit::createUnit("", aUnit, mpUnitSystem);
return aUnit;
+
+}
+
+UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress&
rFormulaAddress,
+ ScDocument* pDoc) {
+ assert(pToken->GetType() == formula::svSingleRef);
+
+ ScSingleRefData* pRef = pToken->GetSingleRef();
+ assert(pRef);
+
+ // Addresses can/will be relative to the formula, for extracting
+ // units however we will need to get the absolute address (i.e.
+ // by adding the current address to the relative formula address).
+ const ScAddress aCellAddress = pRef->toAbs( rFormulaAddress );
+
+ return getUnitForCell(aCellAddress, pDoc);
}
UtUnit UnitsImpl::findHeaderUnitForCell(const ScAddress& rCellAddress,
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index 1e8ac8f..91730f7 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -101,6 +101,12 @@ private:
bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
+ /**
+ * Retrieve the units for a given cell. This probes based on the usual
rules
+ * for cell annotation/column header.
+ * Retrieving units for a formula cell is not yet supported.
+ */
+ UtUnit getUnitForCell(const ScAddress& rCellAddress, ScDocument* pDoc);
UtUnit getUnitForRef(formula::FormulaToken* pToken,
const ScAddress& rFormulaAddress,
ScDocument* pDoc);
commit 39751bf7916cf6d397d49fcab95c778a754e26af
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Mon Mar 30 09:46:36 2015 +0200
Typo in comment
Change-Id: Ic6755b950f6b26546072afa82a9ad68caf2becc8
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 06cd4f3..9492dd1 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -257,7 +257,7 @@ bool UnitsImpl::findUnitInStandardHeader(const OUString&
rsHeader, UtUnit& aUnit
bool UnitsImpl::findFreestandingUnitInHeader(const OUString& rsHeader, UtUnit&
aUnit, OUString& sUnitString) {
- // We just split the string and test whether each token is either a valid
unit in it's own right,
+ // We just split the string and test whether each token is either a valid
unit in its own right,
// or is an operator that could glue together multiple units (i.e.
multiplication/division).
// This is sufficient for when there are spaces between elements composing
the unit, and none
// of the individual elements starts or begins with an operator.
commit 5c941ff7c510b7ec167a3ac69a8e09f3ebc5b72f
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Sat Mar 21 06:47:24 2015 +0100
Handle constants in formulas for verification too.
Change-Id: Ib1b07e6dabf961726b3411b5ff00c67862d95d03
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index a0d5fa1..af5fff5 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -125,7 +125,7 @@ void UnitsTest::testUnitVerification() {
mpDoc->EnsureTable(0);
SvNumberFormatter* pFormatter = mpDoc->GetFormatTable();
- sal_uInt32 nKeyCM, nKeyKG, nKeyS, nKeyCM_S;
+ sal_uInt32 nKeyCM, nKeyM, nKeyKG, nKeyS, nKeyCM_S;
// Used to return position of error in input string for PutEntry
// -- not needed here.
@@ -133,6 +133,8 @@ void UnitsTest::testUnitVerification() {
short nType = css::util::NumberFormat::DEFINED;
+ OUString sM = "#\"m\"";
+ pFormatter->PutEntry(sM, nCheckPos, nType, nKeyM);
OUString sCM = "#\"cm\"";
pFormatter->PutEntry(sCM, nCheckPos, nType, nKeyCM);
OUString sKG = "#\"kg\"";
@@ -186,6 +188,11 @@ void UnitsTest::testUnitVerification() {
mpDoc->SetNumberFormat(address, nKeyCM_S);
mpDoc->SetValue(address, 5);
+ // 5th column: 1m
+ address = ScAddress(4, 0, 0);
+ mpDoc->SetNumberFormat(address, nKeyM);
+ mpDoc->SetValue(address, 1);
+
ScFormulaCell* pCell;
ScTokenArray* pTokens;
@@ -223,6 +230,37 @@ void UnitsTest::testUnitVerification() {
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // Test that addition of scaled units works (cm + 100*m)
+ address = ScAddress(0, 10, 0);
+ mpDoc->SetFormula(address, "=A1+100*E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // 10cm + 100*1m = 110cm
+ CPPUNIT_ASSERT(mpDoc->GetValue(address) == 110);
+
+ // But addition of them unscaled fails (cm + m)
+ address = ScAddress(0, 11, 0);
+ mpDoc->SetFormula(address, "=A1+E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // As does wrong scaling (cm+m/50)
+ address = ScAddress(0, 12, 0);
+ mpDoc->SetFormula(address, "=A1+E1/50");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // And scaling doesn't help when adding incompatible units (kg + 100*m)
+ address = ScAddress(0, 13, 0);
+ mpDoc->SetFormula(address, "=B1+100E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
}
void UnitsTest::testUnitFromFormatStringExtraction() {
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 58f4d14..06cd4f3 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -422,6 +422,18 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
break;
}
+ // As far as I can tell this is only used for an actual numerical value
+ // in which case we just want to use it as scaling factor
+ // (for example [m] + [cm]/100 would be a valid operation)
+ case formula::svDouble:
+ {
+ UtUnit aScale;
+ UtUnit::createUnit("", aScale, mpUnitSystem); // Get the
dimensionless unit 1
+ aScale = aScale*(pToken->GetDouble());
+
+ aUnitStack.push(aScale);
+ break;
+ }
default:
// We can't parse any other types of tokens yet, so assume that
the formula
// was correct.
commit ac45fcb30d1033d3896169d6e1ba5e2a88eeaeba
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Sat Mar 21 06:46:15 2015 +0100
Refactor unit extraction to separate local/header unit extraction
Change-Id: I46eb09fcbb4d83fc58140cda7fada2bf287b0a65
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 5c9b28c..58f4d14 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -208,7 +208,10 @@ OUString UnitsImpl::extractUnitStringForCell(const
ScAddress& rAddress, ScDocume
return extractUnitStringFromFormat(rFormatString);
}
-bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit&
aUnit, OUString& sUnitString) {
+bool UnitsImpl::findUnitInStandardHeader(const OUString& rsHeader, UtUnit&
aUnit, OUString& sUnitString) {
+ // TODO: we should do a sanity check that there's only one such unit
though (and fail if there are multiple).
+ // Since otherwise there's no way for us to know which unit is the
intended one, hence we need to get
+ // the user to deconfuse us by correcting their header to only
contain the intended unit.
com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>
xContext = comphelper::getProcessComponentContext();
uno::Reference<lang::XMultiServiceFactory>
xFactory(xContext->getServiceManager(), uno::UNO_QUERY_THROW);
@@ -222,18 +225,14 @@ bool UnitsImpl::extractUnitFromHeaderString(const
OUString& rString, UtUnit& aUn
aOptions.algorithmType = util::SearchAlgorithms_REGEXP ;
aOptions.searchFlag = util::SearchFlags::ALL_IGNORE_CASE;
- // 1. We try to check for units contained within square brackets
- // TODO: we should do a sanity check that there's only one such unit
though (and fail if there are multiple).
- // Since otherwise there's no way for us to know which unit is the
intended one, hence we need to get
- // the user to deconfuse us by correcting their header to only
contain the intended unit.
aOptions.searchString = "\\[([^\\]]+)\\]"; // Grab the contents between [
and ].
xSearch->setOptions( aOptions );
util::SearchResult aResult;
- sal_Int32 nStartPosition = rString.getLength();
+ sal_Int32 nStartPosition = rsHeader.getLength();
while (nStartPosition) {
// Search from the back since units are more likely to be at the end
of the header.
- aResult = xSearch->searchBackward(rString, nStartPosition, 0);
+ aResult = xSearch->searchBackward(rsHeader, nStartPosition, 0);
// We have either 0 items (no match), or 2 (matched string + the group
within)
if (aResult.subRegExpressions != 2) {
@@ -243,7 +242,7 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString&
rString, UtUnit& aUn
// i.e. startOffset is the last character of the intended
substring, endOffset the first character.
// We specifically grab the offsets for the first actual regex
group, which are stored in [1], the indexes
// at [0] represent the whole matched string (i.e. including
square brackets).
- sUnitString = rString.copy(aResult.endOffset[1],
aResult.startOffset[1] - aResult.endOffset[1]);
+ sUnitString = rsHeader.copy(aResult.endOffset[1],
aResult.startOffset[1] - aResult.endOffset[1]);
if (UtUnit::createUnit(sUnitString, aUnit, mpUnitSystem)) {
return true;
@@ -252,17 +251,26 @@ bool UnitsImpl::extractUnitFromHeaderString(const
OUString& rString, UtUnit& aUn
nStartPosition = aResult.endOffset[0];
}
}
+ sUnitString.clear();
+ return false;
+}
+
- // 2. We try to check for any free-standing units by splitting the string
and testing each split
- const sal_Int32 nTokenCount = comphelper::string::getTokenCount(rString, '
');
+bool UnitsImpl::findFreestandingUnitInHeader(const OUString& rsHeader, UtUnit&
aUnit, OUString& sUnitString) {
+ // We just split the string and test whether each token is either a valid
unit in it's own right,
+ // or is an operator that could glue together multiple units (i.e.
multiplication/division).
+ // This is sufficient for when there are spaces between elements composing
the unit, and none
+ // of the individual elements starts or begins with an operator.
// There's an inherent limit to how well we can cope with various spacing
issues here without
// a ton of computational complexity.
- // E.g. by parsing in this way we might end up with unmatched parentheses
which udunits won't like.
- // for now operators have to be completely freestanding i.e. "cm / kg" or
completely within a valid unit string e.g. "cm/kg"
+ // E.g. by parsing in this way we might end up with unmatched parentheses
which udunits won't like
+ // (thus rejecting the unit) etc.
+
+ const sal_Int32 nTokenCount = comphelper::string::getTokenCount(rsHeader,
' ');
const OUString sOperators = "/*"; // valid
sUnitString.clear();
for (sal_Int32 nToken = 0; nToken < nTokenCount; nToken++) {
- OUString sToken = rString.getToken(nToken, ' ');
+ OUString sToken = rsHeader.getToken(nToken, ' ');
UtUnit aTestUnit;
if (UtUnit::createUnit(sToken, aTestUnit, mpUnitSystem) ||
// Only test for a separator character if we have already got
something in our string, as
@@ -285,6 +293,20 @@ bool UnitsImpl::extractUnitFromHeaderString(const
OUString& rString, UtUnit& aUn
if (sUnitString.getLength() && UtUnit::createUnit(sUnitString, aUnit,
mpUnitSystem)) {
return true;
}
+ sUnitString.clear();
+ return false;
+}
+
+bool UnitsImpl::extractUnitFromHeaderString(const OUString& rsHeader, UtUnit&
aUnit, OUString& sUnitString) {
+ // 1. Ideally we have units in a 'standard' format, i.e. enclose in square
brackets:
+ if (findUnitInStandardHeader(rsHeader, aUnit, sUnitString)) {
+ return true;
+ }
+
+ // 2. But if not we check for free-standing units
+ if (findFreestandingUnitInHeader(rsHeader, aUnit, sUnitString)) {
+ return true;
+ }
// 3. Give up
aUnit = UtUnit(); // assign invalid
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index 5616379..1e8ac8f 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -76,7 +76,31 @@ private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const
OpCode& rOpCode);
OUString extractUnitStringFromFormat(const OUString& rFormatString);
OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument*
pDoc);
- bool extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit,
OUString& sUnitString);
+
+ /**
+ * Find and extract a Unit in the standard header notation,
+ * i.e. a unit enclose within square brackets (e.g. "length [cm]".
+ *
+ * @return true if such a unit is found.
+ */
+ bool findUnitInStandardHeader(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
+ /**
+ * Find and extract a freestanding Unit from a header string.
+ * This includes strings such as "speed m/s", "speed m / s",
+ * "speed (m/s)" etc.
+ * This is (for now) only a fallback for the case that the user hasn't
defined
+ * units in the standard notation, and can only handle a limited number
+ * of ways in which units could be written in a string.
+ * It may turn out that in practice this method should be extended to
support
+ * more permutations of the same unit, but this should at least cover the
most
+ * obvious cases.
+ *
+ * @ return true if a unit is found.
+ */
+ bool findFreestandingUnitInHeader(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
+
+ bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit,
OUString& sUnitString);
+
UtUnit getUnitForRef(formula::FormulaToken* pToken,
const ScAddress& rFormulaAddress,
ScDocument* pDoc);
commit 433f531821f280dbe66dbdf457442651a8fe5cd3
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Sat Mar 21 06:42:48 2015 +0100
Implement UtUnit::operator* for constants
Change-Id: I9c44f0a4d0b78c3eb3262bd55d8304b7b07460e2
diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx
index 058891a..24225dd 100644
--- a/sc/source/core/units/utunit.hxx
+++ b/sc/source/core/units/utunit.hxx
@@ -90,6 +90,19 @@ public:
return UtUnit(ut_multiply(this->get(), rUnit.get()));
}
+ /**
+ * Multiply a unit by a given constant.
+ * This scales the unit by the inverse of that constant,
+ * e.g. 100*m is equivalent to cm, hence
+ * the unit needs to be scaled by 1/100.
+ * (This is implemented in this way so that we can simply
+ * multiply units by any constants present in a formula
+ * in a natural way.)
+ */
+ UtUnit operator*(const double nMultiplier) {
+ return UtUnit(ut_scale(1/nMultiplier, this->get()));
+ }
+
UtUnit operator/(const UtUnit& rUnit) {
// the parameter is the right hand side value in the operation,
// i.e. we are working with this / rUnit.
commit 0ec56880b73b39fe0e6df2c18e28c5f639bed4e5
Author: Andrzej Hunt <andrzej.h...@collabora.com>
Date: Wed Mar 18 12:28:33 2015 +0000
Turn while into for for clarity.
Change-Id: I6e55441c900b64af014fe902c9c8bf928aa7e31a
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 427aec3..5c9b28c 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -366,9 +366,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
stack< UtUnit > aUnitStack;
- FormulaToken* pToken = pArray->FirstRPN();
-
- while (pToken != 0) {
+ for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken =
pArray->NextRPN()) {
switch (pToken->GetType()) {
case formula::svSingleRef:
{
@@ -409,8 +407,6 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const
ScAddress& rFormulaAdd
SAL_WARN("sc.units", "Unrecognised token type " <<
pToken->GetType());
return true;
}
-
- pToken = pArray->NextRPN();
}
// TODO: only fail if actual parsing fails?
commit c56b8eaaa7a2726fee6130cd50af9ddac7879051
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Mon Mar 16 07:45:50 2015 +0000
Add missing include.
Change-Id: I596645b718e12fab46e5fbb7d5ccdc0af251c107
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index e05ac67..f0d7fa8 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -46,6 +46,7 @@
#include "scopetools.hxx"
#include "editutil.hxx"
#include "sharedformula.hxx"
+#include "units.hxx"
#include <listenercontext.hxx>
#include <com/sun/star/i18n/LocaleDataItem.hpp>
commit 44ce335441fd2342f3a4f3456ded2058e89ec6c9
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Mon Mar 16 07:45:23 2015 +0000
Show unit conversion infobar as appropriate.
Change-Id: I882a67167ed5ef60411ccee546df89a2793b36fa
diff --git a/sc/inc/sc.hrc b/sc/inc/sc.hrc
index bcaf7f9..c85bdd7 100644
--- a/sc/inc/sc.hrc
+++ b/sc/inc/sc.hrc
@@ -1000,8 +1000,8 @@
#define STR_UNITS_ERRORINCELL (STR_START + 450)
#define BT_UNITS_EDIT_CELL (STR_START + 451)
-#define STR_UNITS_CONV_REQUIRED (STR_START + 455)
-#define BT_UNITS_CONV_THISCELL (STR_START + 456)
+#define STR_UNITS_CONVERSION_RECOMMENDED (STR_START + 455)
+#define BT_UNITS_CONVERT_THIS_CELL (STR_START + 456)
#define BT_UNITS_CONV_ALL (STR_START + 457)
#define STR_END (BT_UNITS_CONV_ALL)
diff --git a/sc/source/ui/inc/viewfunc.hxx b/sc/source/ui/inc/viewfunc.hxx
index ed92f8a..20d6872 100644
--- a/sc/source/ui/inc/viewfunc.hxx
+++ b/sc/source/ui/inc/viewfunc.hxx
@@ -50,6 +50,8 @@ class SvxHyperlinkItem;
class ScTransferObj;
class ScTableProtection;
+struct UnitConversionPushButton;
+
namespace editeng { class SvxBorderLine; }
namespace sc {
@@ -373,6 +375,13 @@ private:
void NotifyUnitErrorInFormula( const ScAddress& rAddress,
ScDocument* pDoc );
DECL_LINK( EditUnitErrorFormulaHandler, PushButton* );
+
+ void NotifyUnitConversionRecommended( const ScAddress&
rCellAddress,
+ ScDocument* pDoc,
+ const OUString&
sHeaderUnit,
+ const ScAddress&
rHeaderAddress,
+ const OUString& sCellUnit
);
+ DECL_LINK( UnitConversionRecommendedHandler, UnitConversionPushButton* );
};
#endif
diff --git a/sc/source/ui/src/units.src b/sc/source/ui/src/units.src
index 6165af8..be4f970 100644
--- a/sc/source/ui/src/units.src
+++ b/sc/source/ui/src/units.src
@@ -31,16 +31,16 @@ PushButton BT_UNITS_EDIT_CELL
Text[ en-US ] = "Edit Formula";
};
-String STR_UNITS_CONV_REQUIRED
+String STR_UNITS_CONVERSION_RECOMMENDED
{
- Text [ en-US ] = "You entered data in $1, would you like to convert to
$2?" ;
+ Text [ en-US ] = "You entered data in [$1] in cell $2, where [$3] (defined
at cell $4) would be expected." ;
};
-PushButton BT_UNITS_CONV_THISCELL
+PushButton BT_UNITS_CONVERT_THIS_CELL
{
Pos = MAP_APPFONT( 0 , 0 );
Size = MAP_APPFONT( 70 , 0 );
- Text [ en-US ] = "Convert (only this cell)" ;
+ Text [ en-US ] = "Convert to [$1]" ;
};
PushButton BT_UNITS_CONV_ALL
diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx
index 0ac6fff..595c5a9 100644
--- a/sc/source/ui/view/viewfunc.cxx
+++ b/sc/source/ui/view/viewfunc.cxx
@@ -564,19 +564,35 @@ void ScViewFunc::EnterData( SCCOL nCol, SCROW nRow, SCTAB
nTab,
}
}
}
- else
+ else // ( !bFormula )
{
ScMarkData::iterator itr = rMark.begin(), itrEnd = rMark.end();
for ( ; itr != itrEnd; ++itr )
{
bool bNumFmtSet = false;
- rFunc.SetNormalString( bNumFmtSet, ScAddress( nCol, nRow, *itr ),
rString, false );
+ const ScAddress aAddress( nCol, nRow, *itr );
+ rFunc.SetNormalString( bNumFmtSet, aAddress, rString, false );
if (bNumFmtSet)
{
/* FIXME: if set on any sheet results in changed only on
* sheet nTab for TestFormatArea() and DoAutoAttributes() */
bNumFmtChanged = true;
}
+
+#ifdef ENABLE_CALC_UNITVERIFICATION
+ boost::shared_ptr< sc::units::Units > pUnits =
sc::units::Units::GetUnits();
+
+ OUString sHeaderUnit, sCellUnit;
+ ScAddress aHeaderAddress;
+
+ if ( pUnits->isCellConversionRecommended( aAddress, pDoc,
sHeaderUnit, aHeaderAddress, sCellUnit ) ) {
+ NotifyUnitConversionRecommended( aAddress, pDoc, sHeaderUnit,
aHeaderAddress, sCellUnit );
+ } else {
+ SfxViewFrame* pViewFrame =
GetViewData().GetViewShell()->GetFrame();
+ OUString sAddress = aAddress.Format( SCA_BITS, pDoc );
+ pViewFrame->RemoveInfoBar( sAddress );
+ }
+#endif
}
}
@@ -2872,4 +2888,106 @@ IMPL_LINK( ScViewFunc, EditUnitErrorFormulaHandler,
PushButton*, pButton )
return 0;
}
+/*
+ * PushButton wrapper used to persist cell conversion data between data
+ * entry and whenever the user chooses to activate conversion.
+ * This is required as there doesn't seem to be any other way to pass
+ * data to the Handler. This is the easiest way of passing the data (we can't
+ * wrap the infobar itself as it is constructed by Viewframe's AppendInfoBar,
+ * and hidden elements seem even more hacky).
+ *
+ * Forward declaration in viewfunc.hxx
+ */
+struct UnitConversionPushButton: public PushButton
+{
+ const ScAddress aCellAddress;
+ ScDocument* mpDoc;
+ const OUString sHeaderUnit;
+ const OUString sCellUnit;
+
+ UnitConversionPushButton( vcl::Window* pParent,
+ const ResId& rResId,
+ const ScAddress& rCellAddress,
+ ScDocument* pDoc,
+ const OUString& rsHeaderUnit,
+ const OUString& rsCellUnit ):
+ PushButton( pParent, rResId ),
+ aCellAddress( rCellAddress ),
+ mpDoc( pDoc ),
+ sHeaderUnit( rsHeaderUnit ),
+ sCellUnit( rsCellUnit )
+ {}
+};
+
+void ScViewFunc::NotifyUnitConversionRecommended( const ScAddress&
rCellAddress,
+ ScDocument* pDoc,
+ const OUString& rsHeaderUnit,
+ const ScAddress&
rHeaderAddress,
+ const OUString& rsCellUnit ) {
+ SfxViewFrame* pViewFrame = GetViewData().GetViewShell()->GetFrame();
+
+ // As with NotifyUnitErrorInFormula we use the cell address as the infobar
id.
+ // See NotifyUnitErrorInFormula for full details on why. It's impossible
for both
+ // infobars to be valid for a single cell anyways (as the
UnitErrorInFormula infobar
+ // can only appear for Formulas, whereas this one can only appear for
values), hence
+ // using the same id format is not an issue.
+ OUString sTitle = SC_RESSTR( STR_UNITS_CONVERSION_RECOMMENDED );
+ sTitle = sTitle.replaceAll( "$1", rsCellUnit );
+ sTitle = sTitle.replaceAll( "$2", rCellAddress.GetColRowString() );
+ sTitle = sTitle.replaceAll( "$3", rsHeaderUnit );
+ sTitle = sTitle.replaceAll( "$4", rHeaderAddress.GetColRowString() );
+
+ OUString sCellAddress = rCellAddress.Format( SCA_BITS, pDoc );
+ SfxInfoBarWindow* pInfoBar = pViewFrame->AppendInfoBar( sCellAddress,
sTitle );
+
+ assert( pInfoBar );
+
+ UnitConversionPushButton* pButtonConvertCell = new
UnitConversionPushButton( &pViewFrame->GetWindow(),
+
ScResId(BT_UNITS_CONVERT_THIS_CELL),
+
rCellAddress,
+
pDoc,
+
rsHeaderUnit,
+
rsCellUnit );
+ pButtonConvertCell->SetClickHdl( LINK( this, ScViewFunc,
UnitConversionRecommendedHandler ) );
+
+ OUString sConvertText = pButtonConvertCell->GetText();
+ sConvertText = sConvertText.replaceAll( "$1", rsHeaderUnit );
+ pButtonConvertCell->SetText( sConvertText );
+
+ pInfoBar->addButton( pButtonConvertCell );
+}
+
+IMPL_LINK( ScViewFunc, UnitConversionRecommendedHandler,
UnitConversionPushButton*, pButton )
+{
+ // Do conversion first, and only then remove the infobar as we need data
from the infobar
+ // (specifically from the pushbutton) to do the conversion.
+#ifdef ENABLE_CALC_UNITVERIFICATION
+ boost::shared_ptr< sc::units::Units > pUnits =
sc::units::Units::GetUnits();
+
+ pUnits->convertCellToHeaderUnit( pButton->aCellAddress,
+ pButton->mpDoc,
+ pButton->sHeaderUnit,
+ pButton->sCellUnit );
+#endif
+
+ OUString sAddress;
+ {
+ // keep pInfoBar within this scope only as we'll be deleting it just
below (using RemoveInfoBar)
+ SfxInfoBarWindow* pInfoBar = dynamic_cast< SfxInfoBarWindow* >(
pButton->GetParent() );
+ sAddress = pInfoBar->getId();
+ }
+ SfxViewFrame* pViewFrame = GetViewData().GetViewShell()->GetFrame();
+ pViewFrame->RemoveInfoBar( sAddress );
+
+
+ // We make sure we then scroll to the desired cell to make the change
clear.
+ SfxStringItem aPosition( SID_CURRENTCELL, sAddress );
+ SfxBoolItem aUnmark( FN_PARAM_1, true ); // Removes existing selection if
present.
+ GetViewData().GetDispatcher().Execute( SID_CURRENTCELL,
+ SfxCallMode::SYNCHRON |
SfxCallMode::RECORD,
+ &aPosition, &aUnmark, 0L );
+
+ return 0;
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit fd3e1f1f05f6a2d27ad2dbbfb3f3633312e78bb6
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sun Mar 15 20:37:00 2015 +0000
Implement convertCellToHeaderUnit.
Change-Id: Ia62705ea45f116e9204375cdf5658fb06b76315f
diff --git a/sc/inc/units.hxx b/sc/inc/units.hxx
index ecd2946..9837d36 100644
--- a/sc/inc/units.hxx
+++ b/sc/inc/units.hxx
@@ -57,6 +57,25 @@ public:
ScAddress& rHeaderCellAddress,
OUString& rCellUnit) = 0;
+ /**
+ * Convert a cell with local unit annotation to store data in the units
represented by
+ * its (column-level) header.
+ *
+ * This method should only be used with the data provided by
isCellConversionRecommended.
+ * It will remove the cell's local unit annotation, and not add any of
it's own annotation.
+ * You should use the (yet to be implemented) convertCells() method for
more generic
+ * unit conversion.
+ *
+ * Returns true for successful conversion, false if an error occured (e.g.
if the data
+ * has been modified in the meantime, i.e. that the units no longer
correspond
+ * to the expected annotation or header).
+ */
+ virtual bool convertCellToHeaderUnit(const ScAddress& rCellAddress,
+ ScDocument* pDoc,
+ const OUString& rsNewUnit,
+ const OUString& rsOldUnit) = 0;
+
+
virtual ~Units() {}
};
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index c657981..a0d5fa1 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -47,7 +47,7 @@ public:
void testUnitFromHeaderExtraction();
- void testCellConversionRequired();
+ void testCellConversion();
CPPUNIT_TEST_SUITE(UnitsTest);
@@ -58,7 +58,7 @@ public:
CPPUNIT_TEST(testUnitValueStringSplitting);
CPPUNIT_TEST(testUnitFromHeaderExtraction);
- CPPUNIT_TEST(testCellConversionRequired);
+ CPPUNIT_TEST(testCellConversion);
CPPUNIT_TEST_SUITE_END();
@@ -320,7 +320,9 @@ void UnitsTest::testUnitFromHeaderExtraction() {
// CPPUNIT_ASSERT(aUnit == aTestUnit);
}
-void UnitsTest::testCellConversionRequired() {
+void UnitsTest::testCellConversion() {
+ // We test both isCellConversionRecommended, and convertCellToHeaderUnit
+ // since their arguments are essentially shared / dependent.
mpDoc->EnsureTable(0);
// Set up a column with a normal header and a few data values
@@ -358,13 +360,18 @@ void UnitsTest::testCellConversionRequired() {
// First united cell: "cm" (convertible to "m")
address.IncRow();
mpDoc->SetNumberFormat(address, nKeyCM);
- mpDoc->SetValue(address, 10);
+ mpDoc->SetValue(address, 100);
CPPUNIT_ASSERT(mpUnitsImpl->isCellConversionRecommended(address, mpDoc,
sHeaderUnit, aHeaderAddress, sCellUnit));
CPPUNIT_ASSERT(sHeaderUnit == "m");
CPPUNIT_ASSERT(sCellUnit == "cm");
CPPUNIT_ASSERT(aHeaderAddress == ScAddress(20, 0, 0));
+ // Test conversion (From cm to m)
+ CPPUNIT_ASSERT(mpUnitsImpl->convertCellToHeaderUnit(address, mpDoc,
sHeaderUnit, sCellUnit));
+ CPPUNIT_ASSERT(mpDoc->GetValue(address) == 1);
+ CPPUNIT_ASSERT(mpDoc->GetNumberFormat(address) == 0);
+
// Second united cell: "kg" (not convertible to "m")
address.IncRow();
mpDoc->SetNumberFormat(address, nKeyKG);
@@ -374,6 +381,10 @@ void UnitsTest::testCellConversionRequired() {
CPPUNIT_ASSERT(sHeaderUnit.isEmpty());
CPPUNIT_ASSERT(sCellUnit.isEmpty());
CPPUNIT_ASSERT(!aHeaderAddress.IsValid());
+
+ // We specifically don't test conversion with invalid units since that
would be nonsensical
+ // (i.e. would require us passing in made up arguments, where it's
specifically necessary
+ // to pass in the output of isCellConversionRecommended).
}
CPPUNIT_TEST_SUITE_REGISTRATION(UnitsTest);
diff --git a/sc/source/core/units/unitsimpl.cxx
b/sc/source/core/units/unitsimpl.cxx
index 50ba25a..427aec3 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -479,4 +479,45 @@ bool UnitsImpl::isCellConversionRecommended(const
ScAddress& rCellAddress,
return false;
}
+bool UnitsImpl::convertCellToHeaderUnit(const ScAddress& rCellAddress,
+ ScDocument* pDoc,
+ const OUString& rsNewUnit,
+ const OUString& rsOldUnit) {
+ assert(rCellAddress.IsValid());
+
+ OUString sCellUnit = extractUnitStringForCell(rCellAddress, pDoc);
+ UtUnit aOldUnit;
+ UtUnit::createUnit(sCellUnit, aOldUnit, mpUnitSystem);
+
+ OUString sHeaderUnitFound;
+ ScAddress aHeaderAddress; // Unused, but passed by reference
+ UtUnit aNewUnit = findHeaderUnitForCell(rCellAddress, pDoc,
sHeaderUnitFound, aHeaderAddress);
+
+ // We test that we still have all data in the same format as expected.
+ // This is maybe a tad defensive, but this call is most likely to be
delayed
+ // relative to isCellConversionRecommended (e.g. if the user is asked for
+ // confirmation that conversion is desired), hence it's entirely feasible
+ // for data to be changed in the document but this action to be still
+ // called afterwards (especially for non-modal interactions, e.g.
+ // with an infobar which can remain open whilst the document is edited).
+ if ((sCellUnit == rsOldUnit) &&
+ (sHeaderUnitFound == rsNewUnit) &&
+ (pDoc->GetCellType(rCellAddress) == CELLTYPE_VALUE)) {
+ assert(aOldUnit.areConvertibleTo(aNewUnit));
+ double nOldValue = pDoc->GetValue(rCellAddress);
+ double nNewValue = aOldUnit.convertValueTo(nOldValue, aNewUnit);
+
+ pDoc->SetValue(rCellAddress, nNewValue);
+ pDoc->SetNumberFormat(rCellAddress, 0); // 0 == no number format?
+
+ return true;
+ }
+
+ // In an ideal scenario the UI is written such that we never reach this
point,
+ // however that is likely to be hard to achieve, hence we still allow
+ // for this case (see above for more information).
+ SAL_INFO("sc.units", "Unit conversion cancelled: units changed in
meantime.");
+ return false;
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/units/unitsimpl.hxx
b/sc/source/core/units/unitsimpl.hxx
index eec34e2..5616379 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -67,6 +67,10 @@ public:
OUString& rHeaderUnit,
ScAddress& rHeaderCellAddress,
OUString& rCellUnit) SAL_OVERRIDE;
+ virtual bool convertCellToHeaderUnit(const ScAddress& rCellAddress,
+ ScDocument* pDoc,
+ const OUString& rsNewUnit,
+ const OUString& rsOldUnit)
SAL_OVERRIDE;
private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const
OpCode& rOpCode);
commit 72adfb711aca3a311a506304f04260d6d54ac37d
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sun Mar 15 19:59:16 2015 +0000
Implement UtUnit::convertValueTo
Change-Id: Id4469c7f03b3d73a4d2f7c4582602fed64b47df7
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index 32d48d7..c657981 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -110,6 +110,14 @@ void UnitsTest::testUTUnit() {
CPPUNIT_ASSERT(aM_S*aS == aM);
CPPUNIT_ASSERT(aM/aS == aM_S);
+
+ // Do some simple conversion testing
+ UtUnit aCM;
+ UtUnit::createUnit("cm", aCM, mpUnitsImpl->mpUnitSystem);
+ CPPUNIT_ASSERT(aCM.areConvertibleTo(aM));
+ CPPUNIT_ASSERT(!aCM.areConvertibleTo(aS));
+ CPPUNIT_ASSERT(aCM.convertValueTo(0.0, aM) == 0.0); // 0 converts to 0
+ CPPUNIT_ASSERT(aCM.convertValueTo(100.0, aM) == 1.0);
}
void UnitsTest::testUnitVerification() {
diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx
index 557a5fb..058891a 100644
--- a/sc/source/core/units/utunit.hxx
+++ b/sc/source/core/units/utunit.hxx
@@ -99,6 +99,21 @@ public:
bool areConvertibleTo(const UtUnit& rUnit) {
return ut_are_convertible(this->get(), rUnit.get());
}
+
+ double convertValueTo(double nOriginalValue, const UtUnit& rUnit) {
+ // We could write our own cv_converter wrapper too, but that
+ // seems unnecessary given the limited selection of
+ // operations (convert float/double, either individually
+ // or as an array) -- of which we only need a subset anyway
+ // (ie. only for doubles).
+ cv_converter* pConverter = ut_get_converter(this->get(), rUnit.get());
+ assert(pConverter);
+
+ float nConvertedValue = cv_convert_double(pConverter, nOriginalValue);
+
+ cv_free(pConverter);
+ return nConvertedValue;
+ }
};
template< typename charT, typename traits >
commit ae4f758b813ec1b02bd1ba7d5edb7f4c02444727
Author: Andrzej Hunt <andr...@ahunt.org>
Date: Sun Mar 15 19:44:13 2015 +0000
Implement isCellConversionRecommended
Change-Id: I76f8c3c78db66af1087380484ac60da7ec78773d
diff --git a/sc/inc/units.hxx b/sc/inc/units.hxx
index 9561b40..ecd2946 100644
--- a/sc/inc/units.hxx
+++ b/sc/inc/units.hxx
@@ -39,6 +39,24 @@ public:
*/
virtual bool splitUnitsFromInputString(const OUString& rInput, OUString&
rValue, OUString& rUnit) = 0;
+ /**
+ * Check whether a cell should have it's data converted to another unit for
+ * sensible unit verification.
+ *
+ * Returns true if the cell has a local unit annotation (e.g. contains
"10cm",
+ * but split into value (10) and unit format ('#"cm"')), but the column
header
+ * defines a different unit (e.g. "length [m]).
+ * The data returned can then be used by convertCellToHeaderUnit if the
user
+ * desires conversion in this case.
+ *
+ * If returning false, the header and cell units will be empty and the
address invalid.
+ */
+ virtual bool isCellConversionRecommended(const ScAddress& rCellAddress,
+ ScDocument* pDoc,
+ OUString& rHeaderUnit,
+ ScAddress& rHeaderCellAddress,
+ OUString& rCellUnit) = 0;
+
virtual ~Units() {}
};
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index 4ceb123..32d48d7 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -47,6 +47,8 @@ public:
void testUnitFromHeaderExtraction();
+ void testCellConversionRequired();
+
CPPUNIT_TEST_SUITE(UnitsTest);
CPPUNIT_TEST(testUTUnit);
@@ -56,6 +58,8 @@ public:
CPPUNIT_TEST(testUnitValueStringSplitting);
CPPUNIT_TEST(testUnitFromHeaderExtraction);
+ CPPUNIT_TEST(testCellConversionRequired);
+
CPPUNIT_TEST_SUITE_END();
private:
@@ -308,6 +312,62 @@ void UnitsTest::testUnitFromHeaderExtraction() {
// CPPUNIT_ASSERT(aUnit == aTestUnit);
}
+void UnitsTest::testCellConversionRequired() {
+ mpDoc->EnsureTable(0);
+
+ // Set up a column with a normal header and a few data values
+ ScAddress address(20, 0, 0);
+ mpDoc->SetString(address, "length [m]");
+
+ address.IncRow();
+ mpDoc->SetValue(address, 1);
+ address.IncRow();
+ mpDoc->SetValue(address, 2);
+ address.IncRow();
+ mpDoc->SetValue(address, 3);
+
+ ScAddress aHeaderAddress;
+ OUString sHeaderUnit, sCellUnit;
+
... etc. - the rest is truncated
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
