sc/inc/sc.hrc | 4 - sc/inc/units.hxx | 37 +++++++++++ sc/qa/unit/units.cxx | 101 ++++++++++++++++++++++++++++-- sc/source/core/data/column3.cxx | 1 sc/source/core/units/unitsimpl.cxx | 118 ++++++++++++++++++++++++++++++----- sc/source/core/units/unitsimpl.hxx | 22 ++++++ sc/source/core/units/utunit.hxx | 23 ++++++ sc/source/ui/inc/viewfunc.hxx | 9 ++ sc/source/ui/src/units.src | 8 +- sc/source/ui/view/viewfunc.cxx | 122 ++++++++++++++++++++++++++++++++++++- 10 files changed, 411 insertions(+), 34 deletions(-)
New commits: commit 3d422829e56c46c81a389e5be63be9336941257c 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 42e9db8..4a2e38a 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 343b138220fc95cdc748898dfe2fdff5c79db3f9 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 b484ae6..a7de1dd 100644 --- a/sc/inc/sc.hrc +++ b/sc/inc/sc.hrc @@ -994,8 +994,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 ec7598a..50353f3 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 { @@ -371,6 +373,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 3073fe6..d0a2108 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 1cdfc67ab9a6df577a83064d0c8c29f232412eb2 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 05f8128864bf110506715ff7c195f386e2ed16f5 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 48088e0d37dd85cf57ba1e6cdb299e05fc00d135 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 + CPPUNIT_ASSERT(!mpUnitsImpl->isCellConversionRecommended(address, mpDoc, sHeaderUnit, aHeaderAddress, sCellUnit)); + CPPUNIT_ASSERT(sHeaderUnit.isEmpty()); + CPPUNIT_ASSERT(sCellUnit.isEmpty()); + CPPUNIT_ASSERT(!aHeaderAddress.IsValid()); + + // And now set up cells with local units + SvNumberFormatter* pFormatter = mpDoc->GetFormatTable(); + sal_uInt32 nKeyCM, nKeyKG; + + sal_Int32 nCheckPos; // Passed by reference - unused + short nType = css::util::NumberFormat::DEFINED; + + OUString sCM = "#\"cm\""; + pFormatter->PutEntry(sCM, nCheckPos, nType, nKeyCM); + OUString sKG = "#\"kg\""; + pFormatter->PutEntry(sKG, nCheckPos, nType, nKeyKG); + + // First united cell: "cm" (convertible to "m") + address.IncRow(); + mpDoc->SetNumberFormat(address, nKeyCM); + mpDoc->SetValue(address, 10); + + CPPUNIT_ASSERT(mpUnitsImpl->isCellConversionRecommended(address, mpDoc, sHeaderUnit, aHeaderAddress, sCellUnit)); + CPPUNIT_ASSERT(sHeaderUnit == "m"); + CPPUNIT_ASSERT(sCellUnit == "cm"); + CPPUNIT_ASSERT(aHeaderAddress == ScAddress(20, 0, 0)); + + // Second united cell: "kg" (not convertible to "m") + address.IncRow(); + mpDoc->SetNumberFormat(address, nKeyKG); + mpDoc->SetValue(address, 50); + + CPPUNIT_ASSERT(!mpUnitsImpl->isCellConversionRecommended(address, mpDoc, sHeaderUnit, aHeaderAddress, sCellUnit)); + CPPUNIT_ASSERT(sHeaderUnit.isEmpty()); + CPPUNIT_ASSERT(sCellUnit.isEmpty()); + CPPUNIT_ASSERT(!aHeaderAddress.IsValid()); +} + CPPUNIT_TEST_SUITE_REGISTRATION(UnitsTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx index 40228e4..50ba25a 100644 --- a/sc/source/core/units/unitsimpl.cxx +++ b/sc/source/core/units/unitsimpl.cxx @@ -454,4 +454,29 @@ bool UnitsImpl::splitUnitsFromInputString(const OUString& rInput, OUString& rVal } } +bool UnitsImpl::isCellConversionRecommended(const ScAddress& rCellAddress, + ScDocument* pDoc, + OUString& rsHeaderUnit, + ScAddress& rHeaderCellAddress, + OUString& rsCellUnit) { + assert(rCellAddress.IsValid()); + + UtUnit aCellUnit; + rsCellUnit = extractUnitStringForCell(rCellAddress, pDoc); + + if (!rsCellUnit.isEmpty() && UtUnit::createUnit(rsCellUnit, aCellUnit, mpUnitSystem)) { + UtUnit aHeaderUnit = findHeaderUnitForCell(rCellAddress, pDoc, rsHeaderUnit, rHeaderCellAddress); + if (rHeaderCellAddress.IsValid()) { + if (aHeaderUnit.areConvertibleTo(aCellUnit)) { + return true; + } + } + } + + rsHeaderUnit.clear(); + rHeaderCellAddress.SetInvalid(); + rsCellUnit.clear(); + 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 b9da24e..eec34e2 100644 --- a/sc/source/core/units/unitsimpl.hxx +++ b/sc/source/core/units/unitsimpl.hxx @@ -62,6 +62,11 @@ public: virtual bool verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAddress, ScDocument* pDoc) SAL_OVERRIDE; virtual bool splitUnitsFromInputString(const OUString& rInput, OUString& rValue, OUString& rUnit) SAL_OVERRIDE; + virtual bool isCellConversionRecommended(const ScAddress& rCellAddress, + ScDocument* pDoc, + OUString& rHeaderUnit, + ScAddress& rHeaderCellAddress, + OUString& rCellUnit) SAL_OVERRIDE; private: UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode); commit b6e8eb0449ac43c742cc7641c8561da412f8592a Author: Andrzej Hunt <andr...@ahunt.org> Date: Sun Mar 15 19:31:27 2015 +0000 Implement UtUnit::areConvertibleTo Change-Id: I7e8599a363e297c0c0a36c1f9cc68a5e234787b6 diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx index cbdd348..557a5fb 100644 --- a/sc/source/core/units/utunit.hxx +++ b/sc/source/core/units/utunit.hxx @@ -95,6 +95,10 @@ public: // i.e. we are working with this / rUnit. return UtUnit(ut_divide(this->get(), rUnit.get())); } + + bool areConvertibleTo(const UtUnit& rUnit) { + return ut_are_convertible(this->get(), rUnit.get()); + } }; template< typename charT, typename traits > commit b7eee6a6441bca9f8e89cf423a6c01d0fd8a217f Author: Andrzej Hunt <andr...@ahunt.org> Date: Sun Mar 15 14:16:41 2015 +0000 Refactor header finding into it's own method. We need this for e.g. verifying when a conversion of local units may be required. Change-Id: Ie1239b4142065546ea7543dad05240fc9a3e34f7 diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx index 2d3b8db..40228e4 100644 --- a/sc/source/core/units/unitsimpl.cxx +++ b/sc/source/core/units/unitsimpl.cxx @@ -312,11 +312,28 @@ UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaA return aUnit; } + OUString aHeaderUnitString; // Unused -- passed by reference below + ScAddress aHeaderAddress; // Unused too + UtUnit aHeaderUnit = findHeaderUnitForCell(aInputAddress, pDoc, aHeaderUnitString, aHeaderAddress); + if (aHeaderUnit.isValid()) + return aHeaderUnit; + + SAL_INFO("sc.units", "no unit obtained for token at cell " << aInputAddress.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::findHeaderUnitForCell(const ScAddress& rCellAddress, + ScDocument* pDoc, + OUString& rsHeaderUnitString, + ScAddress& rHeaderAddress) { // Scan UPwards from the current cell to find a header. This is since we could potentially // have two different sets of data sharing a column, hence finding the closest header is necessary. - ScAddress aAddress = aInputAddress; - while (aAddress.Row() > 0) { - aAddress.IncRow(-1); + rHeaderAddress = rCellAddress; + while (rHeaderAddress.Row() > 0) { + rHeaderAddress.IncRow(-1); // We specifically test for string cells as intervening data cells could have // differently defined units of their own. (However as these intervening cells @@ -336,12 +353,9 @@ UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaA return aUnit; } } - - SAL_INFO("sc.units", "no unit obtained for token at cell " << aInputAddress.GetColRowString()); - - // We return the dimensionless unit 1 if we don't find any other data suggesting a unit. - UtUnit::createUnit("", aUnit, mpUnitSystem); - return aUnit; + rHeaderAddress.SetInvalid(); + rsHeaderUnitString.clear(); + return UtUnit(); } // getUnitForRef: check format -> if not in format, use more complicated method? (Format overrides header definition) diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx index 33c2fb9..b9da24e 100644 --- a/sc/source/core/units/unitsimpl.hxx +++ b/sc/source/core/units/unitsimpl.hxx @@ -71,6 +71,17 @@ private: UtUnit getUnitForRef(formula::FormulaToken* pToken, const ScAddress& rFormulaAddress, ScDocument* pDoc); + + /** + * Return both the UtUnit and the String as we usually want the UtUnit + * (which is created from the String, and has to be created to ensure + * that there is a valid unit), but we might also need the original + * String (which can't necessarily be regenerated from the UtUnit). + */ + UtUnit findHeaderUnitForCell(const ScAddress& rCellAddress, + ScDocument* pDoc, + OUString& rsHeaderUnitString, + ScAddress& rHeaderAddress); }; }} // namespace sc::units commit 1ca0113593e9d5b1d97f44cedcef4ffff9af1aeb Author: Andrzej Hunt <andr...@ahunt.org> Date: Sun Mar 15 14:14:39 2015 +0000 Implement operator!= for UtUnit. Change-Id: Iafcc26411f518fc2b8741ec1cb9d5e9eda28a184 diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx index 97b6305..cbdd348 100644 --- a/sc/source/core/units/utunit.hxx +++ b/sc/source/core/units/utunit.hxx @@ -82,6 +82,10 @@ public: return ut_compare(this->get(), rUnit.get()) == 0; } + bool operator!=(const UtUnit& rUnit) { + return !operator==(rUnit); + } + UtUnit operator*(const UtUnit& rUnit) { return UtUnit(ut_multiply(this->get(), rUnit.get())); } commit 225b86c0015adf55147e1a78911da2dbe8ad8826 Author: Andrzej Hunt <andr...@ahunt.org> Date: Sun Mar 15 14:14:25 2015 +0000 Return original unit string for extractUnitFromHeaderString too. Change-Id: I060322d3a05be4c2c2a5ff8e482671dd9765d785 diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx index 97acef6..4ceb123 100644 --- a/sc/qa/unit/units.cxx +++ b/sc/qa/unit/units.cxx @@ -259,43 +259,51 @@ void UnitsTest::testUnitValueStringSplitting() { void UnitsTest::testUnitFromHeaderExtraction() { UtUnit aUnit; + OUString sUnitString; OUString sEmpty = ""; - CPPUNIT_ASSERT(!mpUnitsImpl->extractUnitFromHeaderString(sEmpty, aUnit)); + CPPUNIT_ASSERT(!mpUnitsImpl->extractUnitFromHeaderString(sEmpty, aUnit, sUnitString)); CPPUNIT_ASSERT(aUnit == UtUnit()); + CPPUNIT_ASSERT(sUnitString.isEmpty()); OUString sSimple = "bla bla [cm/s]"; - CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sSimple, aUnit)); + CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sSimple, aUnit, sUnitString)); // We need to test in Units (rather than testing Unit::getString()) as // any given unit can have multiple string representations (and utunits defaults to // representing e.g. cm as (I think) "0.01m"). UtUnit aTestUnit; CPPUNIT_ASSERT(UtUnit::createUnit("cm/s", aTestUnit, mpUnitsImpl->mpUnitSystem)); CPPUNIT_ASSERT(aUnit == aTestUnit); + CPPUNIT_ASSERT(sUnitString == "cm/s"); OUString sFreeStanding = "bla bla kg/h"; - CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStanding, aUnit)); + CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStanding, aUnit, sUnitString)); CPPUNIT_ASSERT(UtUnit::createUnit("kg/h", aTestUnit, mpUnitsImpl->mpUnitSystem)); CPPUNIT_ASSERT(aUnit == aTestUnit); + CPPUNIT_ASSERT(sUnitString == "kg/h"); OUString sFreeStandingWithSpaces = "bla bla m / s"; - CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStandingWithSpaces, aUnit)); + CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStandingWithSpaces, aUnit, sUnitString)); CPPUNIT_ASSERT(UtUnit::createUnit("m/s", aTestUnit, mpUnitsImpl->mpUnitSystem)); CPPUNIT_ASSERT(aUnit == aTestUnit); + CPPUNIT_ASSERT(sUnitString == "m/s"); OUString sOperatorSeparated = "bla bla / t/s"; - CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sOperatorSeparated, aUnit)); + CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sOperatorSeparated, aUnit, sUnitString)); CPPUNIT_ASSERT(UtUnit::createUnit("t/s", aTestUnit, mpUnitsImpl->mpUnitSystem)); CPPUNIT_ASSERT(aUnit == aTestUnit); + CPPUNIT_ASSERT(sUnitString == "t/s"); + OUString sRoundBrackets = "bla bla (t/h)"; - CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sRoundBrackets, aUnit)); + CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sRoundBrackets, aUnit, sUnitString)); CPPUNIT_ASSERT(UtUnit::createUnit("t/h", aTestUnit, mpUnitsImpl->mpUnitSystem)); CPPUNIT_ASSERT(aUnit == aTestUnit); + CPPUNIT_ASSERT(sUnitString == "(t/h)"); // This becomes more of a nightmare to support, so let's not bother for now. // OUString sFreeStandingMixedSpaces = "bla bla m /s* kg"; - // CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStanding, aUnit)); + // CPPUNIT_ASSERT(mpUnitsImpl->extractUnitFromHeaderString(sFreeStanding, aUnit, sUnitString)); // CPPUNIT_ASSERT(UtUnit::createUnit("m/s", aTestUnit, mpUnitsImpl->mpUnitSystem)); // CPPUNIT_ASSERT(aUnit == aTestUnit); } diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx index 1c5f1bf..2d3b8db 100644 --- a/sc/source/core/units/unitsimpl.cxx +++ b/sc/source/core/units/unitsimpl.cxx @@ -208,7 +208,7 @@ OUString UnitsImpl::extractUnitStringForCell(const ScAddress& rAddress, ScDocume return extractUnitStringFromFormat(rFormatString); } -bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit) { +bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit, OUString& sUnitString) { com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> xContext = comphelper::getProcessComponentContext(); uno::Reference<lang::XMultiServiceFactory> xFactory(xContext->getServiceManager(), uno::UNO_QUERY_THROW); @@ -243,7 +243,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). - OUString sUnitString = rString.copy(aResult.endOffset[1], aResult.startOffset[1] - aResult.endOffset[1]); + sUnitString = rString.copy(aResult.endOffset[1], aResult.startOffset[1] - aResult.endOffset[1]); if (UtUnit::createUnit(sUnitString, aUnit, mpUnitSystem)) { return true; @@ -260,7 +260,7 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn // 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" const OUString sOperators = "/*"; // valid - OUString sCurrent = ""; + sUnitString.clear(); for (sal_Int32 nToken = 0; nToken < nTokenCount; nToken++) { OUString sToken = rString.getToken(nToken, ' '); UtUnit aTestUnit; @@ -268,11 +268,11 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn // Only test for a separator character if we have already got something in our string, as // some of the operators could be used as separators from description to unit // (e.g. "a description / kg"). - ((sCurrent.getLength() > 0) && (sToken.getLength() == 1) && (sOperators.indexOf(sToken[0]) != -1))) { + ((sUnitString.getLength() > 0) && (sToken.getLength() == 1) && (sOperators.indexOf(sToken[0]) != -1))) { // we're repeatedly testing the string hence using an OUStringBuffer isn't of much use since there's // no simple/efficient way of repeatedly getting a testable OUString from the buffer. - sCurrent += sToken; - } else if (sCurrent.getLength() > 0) { + sUnitString += sToken; + } else if (sUnitString.getLength() > 0) { // If we have units, followed by text, followed by units, we should still flag an error since // that's ambiguous (unless the desired units are enclose in [] in which case we've // already extracted these desired units in step 1 above. @@ -282,12 +282,13 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn // We test the length to make sure we don't return the dimensionless unit 1 if we haven't found any units // in the header. - if (sCurrent.getLength() && UtUnit::createUnit(sCurrent, aUnit, mpUnitSystem)) { + if (sUnitString.getLength() && UtUnit::createUnit(sUnitString, aUnit, mpUnitSystem)) { return true; } // 3. Give up aUnit = UtUnit(); // assign invalid + sUnitString.clear(); return false; } @@ -321,8 +322,9 @@ UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaA // differently defined units of their own. (However as these intervening cells // will have the unit stored in the number format it would be ignored when // checking the cell's string anyway.) - if (pDoc->GetCellType(aAddress) == CELLTYPE_STRING && - extractUnitFromHeaderString(pDoc->GetString(aAddress), aUnit)) { + UtUnit aUnit; + if (pDoc->GetCellType(rHeaderAddress) == CELLTYPE_STRING && + extractUnitFromHeaderString(pDoc->GetString(rHeaderAddress), aUnit, rsHeaderUnitString)) { // TODO: one potential problem is that we could have a text only "united" data cell // (where the unit wasn't automatically extracted due to being entered via // a different spreadsheet program). diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx index 909916a..33c2fb9 100644 --- a/sc/source/core/units/unitsimpl.hxx +++ b/sc/source/core/units/unitsimpl.hxx @@ -67,7 +67,7 @@ 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); + bool extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit, OUString& sUnitString); UtUnit getUnitForRef(formula::FormulaToken* pToken, const ScAddress& rFormulaAddress, ScDocument* pDoc); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits