Split convertCellUnits and change contract

This is in preparation for rewriting convertCellUnits to handle
ScRangeList's.

Change-Id: I17fecdb64674af79a33f2b1a62b4b46150177af5

3 files changed, 98 insertions, 66 deletions

diff --git a/sc/inc/units.hxx b/sc/inc/units.hxx
index 4d833166e94e..b5e12da42909 100644
--- a/sc/inc/units.hxx
+++ b/sc/inc/units.hxx
@@ -96,9 +96,8 @@ public:
      * If possible the input unit will be determined automatically (using local
      * and header units).
      *
-     * Returns false if input units are not compatible with the desired output units,
-     * (including the case where some of the input units are compatibles but others
-     *  aren't).
+     * Returns false if not input units are not compatible with the desired output units,
+     * however this method still converts all cells containing compatible units.
      *
      * Local and header unit annotations are modified as appropriate such that the output
      * remains unambiguous. Hence, if the header cell is included in rRange, its unit
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 497331af9fce..f2aa9e0e3315 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -715,78 +715,72 @@ bool UnitsImpl::convertCellToHeaderUnit(const ScAddress& rCellAddress,
     return false;
 }
 
-bool UnitsImpl::convertCellUnits(const ScRange& rRange,
-                                 ScDocument* pDoc,
-                                 const OUString& rsOutputUnit) {
-    UtUnit aOutputUnit;
-    if (!UtUnit::createUnit(rsOutputUnit, aOutputUnit, mpUnitSystem)) {
-        return false;
-    }
-
-    ScRange aRange(rRange);
-    aRange.PutInOrder();
-
-    SCCOL nStartCol, nEndCol;
-    SCROW nStartRow, nEndRow;
-    SCTAB nStartTab, nEndTab;
-    aRange.GetVars(nStartCol, nStartRow, nStartTab,
-                   nEndCol, nEndRow, nEndTab);
-
-    // Can only handle ranges in a single sheet for now
-    assert(nStartTab == nEndTab);
-
-    // Each column is independent hence we are able to handle each separately.
-    for (SCCOL nCol = nStartCol; nCol <= nEndCol; nCol++) {
-        HeaderUnitDescriptor aHeader = { false, UtUnit(), boost::optional< ScAddress >(), "", -1 };
-
-        for (SCROW nRow = nEndRow; nRow >= nStartRow; nRow--) {
-            ScAddress aCurrent(nCol, nRow, nStartTab);
-
-            if (aCurrent == aHeader.address) {
-                OUString sHeader = pDoc->GetString(aCurrent);
-                sHeader = sHeader.replaceAt(aHeader.unitStringPosition, aHeader.unitString.getLength(), rsOutputUnit);
-                pDoc->SetString(aCurrent, sHeader);
-
-                aHeader.valid = false;
-            } else if (pDoc->GetCellType(aCurrent) != CELLTYPE_STRING) {
+bool UnitsImpl::convertCellUnitsForColumnRange(const ScRange& rRange,
+                                               ScDocument* pDoc,
+                                               const UtUnit& rOutputUnit) {
+    assert(rRange.aStart.Row() <= rRange.aEnd.Row());
+    assert(rRange.aStart.Col() == rRange.aEnd.Col());
+    assert(rRange.aStart.Tab() == rRange.aEnd.Tab());
+    assert(rOutputUnit.getInputString());
+
+    HeaderUnitDescriptor aHeader = { false, UtUnit(), boost::optional< ScAddress >(), "", -1 };
+
+    SCCOL nCol = rRange.aStart.Col();
+    SCROW nStartRow = rRange.aStart.Row();
+    SCROW nEndRow = rRange.aEnd.Row();
+    SCTAB nTab = rRange.aStart.Tab();
+
+    bool bAllConverted = true;
+
+    for (SCROW nRow = nEndRow; nRow >= nStartRow; nRow--) {
+        ScAddress aCurrent(nCol, nRow, nTab);
+
+        // It's possible that the header refers to an incompatible unit, hence
+        // shouldn't be modified when we're converting.
+        if (aCurrent == aHeader.address &&
+            aHeader.unit.areConvertibleTo(rOutputUnit)) {
+            OUString sHeader = pDoc->GetString(aCurrent);
+            sHeader = sHeader.replaceAt(aHeader.unitStringPosition, aHeader.unitString.getLength(), *rOutputUnit.getInputString());
+            pDoc->SetString(aCurrent, sHeader);
+
+            aHeader.valid = false;
+        } else if (pDoc->GetCellType(aCurrent) != CELLTYPE_STRING) {
+            if (!aHeader.valid) {
+                aHeader = findHeaderUnitForCell(aCurrent, pDoc);
+
+                // If there is no header we get an invalid unit returned from findHeaderUnitForCell,
+                // and therfore assume the dimensionless unit 1.
                 if (!aHeader.valid) {
-                    aHeader = findHeaderUnitForCell(aCurrent, pDoc);
-
-                    // If there is no header we get an invalid unit returned from findHeaderUnitForCell,
-                    // and therfore assume the dimensionless unit 1.
-                    if (!aHeader.valid) {
-                        UtUnit::createUnit("", aHeader.unit, mpUnitSystem);
-                        aHeader.valid = true;
-                    }
+                    UtUnit::createUnit("", aHeader.unit, mpUnitSystem);
+                    aHeader.valid = true;
                 }
+            }
 
-                OUString sLocalUnit(extractUnitStringForCell(aCurrent, pDoc));
-                UtUnit aLocalUnit;
-                if (sLocalUnit.isEmpty()) {
-                    aLocalUnit = aHeader.unit;
-                } else { // override header unit with annotation unit
-                    if (!UtUnit::createUnit(sLocalUnit, aLocalUnit, mpUnitSystem)) {
-                        // but assume dimensionless if invalid
-                        UtUnit::createUnit("", aLocalUnit, mpUnitSystem);
-                    }
+            OUString sLocalUnit(extractUnitStringForCell(aCurrent, pDoc));
+            UtUnit aLocalUnit;
+            if (sLocalUnit.isEmpty()) {
+                aLocalUnit = aHeader.unit;
+            } else { // override header unit with annotation unit
+                if (!UtUnit::createUnit(sLocalUnit, aLocalUnit, mpUnitSystem)) {
+                    // but assume dimensionless if invalid
+                    UtUnit::createUnit("", aLocalUnit, mpUnitSystem);
                 }
+            }
 
-                bool bLocalAnnotationRequired = (!aRange.In(*aHeader.address)) &&
-                                                (aOutputUnit != aHeader.unit);
-                double nValue = pDoc->GetValue(aCurrent);
+            bool bLocalAnnotationRequired = (!rRange.In(*aHeader.address)) &&
+                (rOutputUnit != aHeader.unit);
+            double nValue = pDoc->GetValue(aCurrent);
 
-                if (!aLocalUnit.areConvertibleTo(aOutputUnit)) {
-                    // TODO: in future we should undo all our changes here.
-                    return false;
-                }
-
-                double nNewValue = aLocalUnit.convertValueTo(nValue, aOutputUnit);
+            if (!aLocalUnit.areConvertibleTo(rOutputUnit)) {
+                bAllConverted = false;
+            } else {
+                double nNewValue = aLocalUnit.convertValueTo(nValue, rOutputUnit);
                 pDoc->SetValue(aCurrent, nNewValue);
 
                 if (bLocalAnnotationRequired) {
                     // All a local dirty hack too - needs to be refactored and improved.
                     // And ideally we should reuse the existing format.
-                    OUString sNewFormat = "General\"" + rsOutputUnit + "\"";
+                    OUString sNewFormat = "General\"" + *rOutputUnit.getInputString() + "\"";
                     sal_uInt32 nFormatKey;
                     short nType = css::util::NumberFormat::DEFINED;
                     sal_Int32 nErrorPosition; // Unused, because we should be creating working number formats.
@@ -799,11 +793,41 @@ bool UnitsImpl::convertCellUnits(const ScRange& rRange,
                     pDoc->SetNumberFormat(aCurrent, 0);
                 }
             }
-
         }
+
     }
+    return bAllConverted;
+}
 
-    return true;
+bool UnitsImpl::convertCellUnits(const ScRange& rRange,
+                                 ScDocument* pDoc,
+                                 const OUString& rsOutputUnit) {
+    UtUnit aOutputUnit;
+    if (!UtUnit::createUnit(rsOutputUnit, aOutputUnit, mpUnitSystem)) {
+        return false;
+    }
+
+    ScRange aRange(rRange);
+    aRange.PutInOrder();
+
+    SCCOL nStartCol, nEndCol;
+    SCROW nStartRow, nEndRow;
+    SCTAB nStartTab, nEndTab;
+    aRange.GetVars(nStartCol, nStartRow, nStartTab,
+                   nEndCol, nEndRow, nEndTab);
+
+    // Can only handle ranges in a single sheet for now
+    assert(nStartTab == nEndTab);
+
+    // Each column is independent hence we are able to handle each separately.
+    bool bAllConverted = true;
+    for (SCCOL nCol = nStartCol; nCol <= nEndCol; nCol++) {
+        ScRange aSubRange(ScAddress(nCol, nStartRow, nStartTab), ScAddress(nCol, nEndRow, nStartTab));
+        bAllConverted = bAllConverted &&
+                        convertCellUnitsForColumnRange(aSubRange, pDoc, aOutputUnit);
+    }
+
+    return bAllConverted;
 }
 
 bool UnitsImpl::areUnitsCompatible(const OUString& rsUnit1, const OUString& rsUnit2) {
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index ebd20b0ba2f2..e7b4597840c0 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -156,6 +156,15 @@ private:
                          ScDocument* pDoc);
 
     /**
+     * Convert cells within a given range. The range MUST be restricted
+     * to being a group of cells within one column, in one sheet/tab.
+     * rOutputUnit MUST possess an input unit string.
+     */
+    bool convertCellUnitsForColumnRange(const ScRange& rRange,
+                                        ScDocument* pDoc,
+                                        const UtUnit& rOutputUnit);
+
+    /**
      * 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