Re: [Libreoffice] [REVIEW] Fix for fdo#33705

On Wed, 2011-06-08 at 21:50 -0400, Kohei Yoshida wrote:

> to see if that will solve the unit test failure issue.  The test passes
> in my build environment so I'm suggesting this entirely in blind, so I
> don't want to check this in before confirming that this is indeed the
> issue.

Ok.  This won't solve it.  Long story short, the attached patch should
work for everybody.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>

From dc243aa1c0b1b5219c0329e60eee646d83e06d16 Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <kyoshida@novell.com>
Date: Wed, 8 Jun 2011 12:53:21 -0400
Subject: [PATCH] fdo#33705: Fixed cell function N.

Also added unit test for this built-in function.
---
 sc/qa/unit/ucalc.cxx             |   64 ++++++++++++++++++++++++++++++++++++++
 sc/source/core/tool/interpr1.cxx |   48 +++++++++++++++++++---------
 2 files changed, 97 insertions(+), 15 deletions(-)

diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 7a6d46e..835f085 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -60,6 +60,7 @@
 #include "drwlayer.hxx"
 #include "scitems.hxx"
 #include "reffind.hxx"
+#include "markdata.hxx"
 
 #include "docsh.hxx"
 #include "funcdesc.hxx"
@@ -422,6 +423,69 @@ void Test::testCellFunctions()
     m_pDoc->GetValue(0, 4, 0, result);
     CPPUNIT_ASSERT_MESSAGE("Calculation of PRODUCT with inline array failed", result == 6.0);
 
+    {
+        // N
+
+        // Clear the area first.
+        ScMarkData aMarkData;
+        aMarkData.SetMarkArea(ScRange(0, 0, 0, 1, 20, 0));
+        m_pDoc->DeleteArea(0, 0, 1, 20, aMarkData, IDF_CONTENTS);
+
+        // Put values to reference.
+        val = 0;
+        m_pDoc->SetValue(0, 0, 0, val);
+        m_pDoc->SetString(0, 2, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("Text")));
+        val = 1;
+        m_pDoc->SetValue(0, 3, 0, val);
+        val = -1;
+        m_pDoc->SetValue(0, 4, 0, val);
+        val = 12.3;
+        m_pDoc->SetValue(0, 5, 0, val);
+        m_pDoc->SetString(0, 6, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("'12.3")));
+
+        // Cell references
+        m_pDoc->SetString(1, 0, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A1)")));
+        m_pDoc->SetString(1, 1, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A2)")));
+        m_pDoc->SetString(1, 2, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A3)")));
+        m_pDoc->SetString(1, 3, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A4)")));
+        m_pDoc->SetString(1, 4, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A5)")));
+        m_pDoc->SetString(1, 5, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A6)")));
+        m_pDoc->SetString(1, 6, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A9)")));
+
+        // In-line values
+        m_pDoc->SetString(1, 7, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(0)")));
+        m_pDoc->SetString(1, 8, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(1)")));
+        m_pDoc->SetString(1, 9, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(-1)")));
+        m_pDoc->SetString(1, 10, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(123)")));
+        m_pDoc->SetString(1, 11, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(\"\")")));
+        m_pDoc->SetString(1, 12, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(\"12\")")));
+        m_pDoc->SetString(1, 13, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(\"foo\")")));
+
+        // Range references
+        m_pDoc->SetString(1, 14, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A1:A8)")));
+        m_pDoc->SetString(1, 15, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A4:B8)")));
+        m_pDoc->SetString(1, 16, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A6:B8)")));
+        m_pDoc->SetString(1, 17, 0, OUString(RTL_CONSTASCII_USTRINGPARAM("=N(A2:B8)")));
+
+        // Calculate and check the results.
+        m_pDoc->CalcAll();
+        double checks[] = {
+            0, 0,  0,    1, -1, 12.3, 0, // cell reference
+            0, 1, -1, 123,  0,    0, 0, // in-line values
+            0, 1, 12.3, 0                // range references
+        };
+        for (size_t i = 0; i < SAL_N_ELEMENTS(checks); ++i)
+        {
+            m_pDoc->GetValue(1, i, 0, result);
+            bool bGood = result == checks[i];
+            if (!bGood)
+            {
+                cerr << "row " << (i+1) << ": expected=" << checks[i] << " actual=" << result << 
endl;
+                CPPUNIT_ASSERT_MESSAGE("Unexpected result for N", false);
+            }
+        }
+    }
+
     m_pDoc->DeleteTab(0);
 }
 
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index 1999bff..2327e94 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -2509,26 +2509,44 @@ void ScInterpreter::ScIsOdd()
     PushInt( !IsEven() );
 }
 
-
 void ScInterpreter::ScN()
 {
-    RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "sc", "er", "ScInterpreter::ScN" );
-    sal_uInt16 nErr = nGlobalError;
-    nGlobalError = 0;
-    // Temporarily override the ConvertStringToValue() error for
-    // GetCellValue() / GetCellValueOrZero()
-    sal_uInt16 nSErr = mnStringNoValueError;
-    mnStringNoValueError = errCellNoValue;
+    switch (GetRawStackType())
+    {
+        case svSingleRef:
+        case svDoubleRef:
+        case svMatrix:
+        case svExternalSingleRef:
+        case svExternalDoubleRef:
+        {
+            ScMatrixRef pMat = GetMatrix();
+            SCSIZE nC, nR;
+            pMat->GetDimensions(nC, nR);
+            if (!nC || !nR)
+                PushDouble(0);
+            else
+                PushDouble(pMat->GetDouble(0, 0));
+            return;
+        }
+        case svString:
+            PushDouble(0);
+            return;
+        default:
+            ;
+    }
+
+    // Default action
     double fVal = GetDouble();
-    mnStringNoValueError = nSErr;
-    if ( nGlobalError == NOTAVAILABLE || nGlobalError == errCellNoValue )
-        nGlobalError = 0;       // N(#NA) and N("text") are ok
-    if ( !nGlobalError && nErr != NOTAVAILABLE )
-        nGlobalError = nErr;
-    PushDouble( fVal );
+    if (nGlobalError)
+    {
+        // Don't propagate the error. Push 0 instead.
+        nGlobalError = 0;
+        PushDouble(0);
+        return;
+    }
+    PushDouble(fVal);
 }
 
-
 void ScInterpreter::ScTrim()
 {   // Doesn't only trim but writes out twice!
     String aVal( GetString() );
-- 
1.7.3.4