Rebased ref, commits from common ancestor: commit 50620c88049227f19ba3a3e95211f461d8be0061 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 536af7afbf7c083aa4d0762211479c2d69f86f67 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 70fbafb199f0b7c12fc50af6ce67a4579490caab 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 a60d5c2d6a848d1a275825248f62dddad53496ab 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 f8cea94743cf4861748e0cd8e1149fc16f262811 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 57195bbc475150349800c712ff4f41313911c338 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 3b8832825117abb3de40de10af5ccd49cd13c364 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 c42de9628cf7cf2d1d856fdc8b613859e0e85b4a 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 c5dd682e6f669d1328184f7c1e2d172db7bc6987 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 216b294a808d4c4e6d150f6c3db47534102be911 Author: Andrzej Hunt <andrzej.h...@collabora.com> Date: Mon Mar 30 11:12:42 2015 +0200 Verification of sum/product + further 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 16323959fdf142af3abcb7e84654de337b085405 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 5c0d8fde5310c768e4ec72d21d4773486c60f6a3 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 d676671f8ca9cef9d2e2729de84522129d0b6770 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 200c3193c261bc95163e532a7092a147a1afae95 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 fe56a052a1c90c356bf294590f741a5093b16ddd 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 c6c612f73edf5b76da905bdcc3757926a7d199e2 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 018929a214d888d56d8c91b2652aaabece75a71d 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 1dbb866abbcd0a33723e7992984c8603dc7cf441 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 b27e16833f7dfdb6db824dcab08dd2c81218dd25 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 7fd3a4e9e25bcd1ab467c8d457137d1a43f626c1 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 603d1b12e8867e2d28f8c1d3a6ce970ec815cfa6 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 ef02d3c1a66d20ab8a9960d8d55d715d81f6dad9 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 1902c4db55d718b26344866a172acb60a376c98c 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; + + // Test that we don't expect conversion for an non-united value cell ... etc. - the rest is truncated _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits