[gnumeric] SUMIF: handle bools right.

[Morten Welinder <mortenw src gnome org>]

commit c18dc9c206c1cf920b5ab0fecbc77e659ab92ca2
Author: Morten Welinder <terra gnome org>
Date:   Thu Dec 3 13:26:35 2009 -0500

    SUMIF: handle bools right.

 plugins/fn-math/functions.c |   41 +++++++++++++++++++----------------------
 src/value.c                 |   38 ++++++++++++++++++++++----------------
 2 files changed, 41 insertions(+), 38 deletions(-)
---
diff --git a/plugins/fn-math/functions.c b/plugins/fn-math/functions.c
index ae46e78..d77d1d6 100644
--- a/plugins/fn-math/functions.c
+++ b/plugins/fn-math/functions.c
@@ -476,7 +476,7 @@ cb_countif (GnmCellIter const *iter, CountIfClosure *res)
 		gnm_cell_eval (cell);
 		v = cell->value;
 	} else
-		v = NULL;
+		v = value_new_empty ();  /* Never released */
 
 	if (!VALUE_IS_EMPTY (v) && !VALUE_IS_NUMBER (v) && !VALUE_IS_STRING (v))
 		return NULL;
@@ -552,7 +552,7 @@ cb_sumif (GnmCellIter const *iter, SumIfClosure *res)
 		gnm_cell_eval (cell);
 		v = cell->value;
 	} else
-		v = NULL;
+		v = value_new_empty ();  /* Never released */
 
 	if (!VALUE_IS_EMPTY (v) && !VALUE_IS_NUMBER (v) && !VALUE_IS_STRING (v))
 		return NULL;
@@ -561,23 +561,22 @@ cb_sumif (GnmCellIter const *iter, SumIfClosure *res)
 		return NULL;
 
 	if (NULL != res->target_sheet) {
-		cell = sheet_cell_get (res->target_sheet,
-				       iter->pp.eval.col + res->offset_col,
-				       iter->pp.eval.row + res->offset_row);
+		GnmCell *cell = sheet_cell_get
+			(res->target_sheet,
+			 iter->pp.eval.col + res->offset_col,
+			 iter->pp.eval.row + res->offset_row);
 		if (!cell)
 			return NULL;
 
 		gnm_cell_eval (cell);
 		v = cell->value;
-
-		/* FIXME: Check bools.  */
-		if (!VALUE_IS_FLOAT (v))
-			return NULL;
 	}
 
+	if (!VALUE_IS_FLOAT (v))
+		return NULL;
+
 	/* FIXME: Check bools and strings.  */
-	if (v)
-		res->sum += value_get_as_float (v);
+	res->sum += value_get_as_float (v);
 
 	return NULL;
 }
@@ -664,7 +663,7 @@ cb_averageif (GnmCellIter const *iter, AverageIfClosure *res)
 		gnm_cell_eval (cell);
 		v = cell->value;
 	} else
-		v = NULL;
+		v = value_new_empty ();  /* Never released */
 
 	if (!VALUE_IS_EMPTY (v) && !VALUE_IS_NUMBER (v) && !VALUE_IS_STRING (v))
 		return NULL;
@@ -673,23 +672,21 @@ cb_averageif (GnmCellIter const *iter, AverageIfClosure *res)
 		return NULL;
 
 	if (NULL != res->target_sheet) {
-		cell = sheet_cell_get (res->target_sheet,
-				       iter->pp.eval.col + res->offset_col,
-				       iter->pp.eval.row + res->offset_row);
+		GnmCell *cell = sheet_cell_get
+			(res->target_sheet,
+			 iter->pp.eval.col + res->offset_col,
+			 iter->pp.eval.row + res->offset_row);
 		if (!cell)
 			return NULL;
 
 		gnm_cell_eval (cell);
 		v = cell->value;
-
-		/* FIXME: Check bools.  */
-		if (!VALUE_IS_FLOAT (v))
-			return NULL;
 	}
 
-	/* FIXME: Check bools.  */
-	if (v)
-		res->sum += value_get_as_float (v);
+	if (!VALUE_IS_FLOAT (v))
+		return NULL;
+
+	res->sum += value_get_as_float (v);
 	res->count++;
 
 	return NULL;
diff --git a/src/value.c b/src/value.c
index 8b8810f..9225bb7 100644
--- a/src/value.c
+++ b/src/value.c
@@ -1339,7 +1339,7 @@ value_set_fmt (GnmValue *v, GOFormat const *fmt)
 
 /****************************************************************************/
 
-typedef enum { CRIT_NULL, CRIT_FLOAT, CRIT_BADFLOAT, CRIT_STRING } CritType;
+typedef enum { CRIT_NULL, CRIT_FLOAT, CRIT_WRONGTYPE, CRIT_STRING } CritType;
 
 static CritType
 criteria_inspect_values (GnmValue const *x, gnm_float *xr, gnm_float *yr,
@@ -1356,13 +1356,18 @@ criteria_inspect_values (GnmValue const *x, gnm_float *xr, gnm_float *yr,
 	*yr = value_get_as_float (y);
 
 	if (VALUE_IS_NUMBER (x)) {
+		if (VALUE_IS_BOOLEAN (y) != VALUE_IS_BOOLEAN (x))
+			return CRIT_WRONGTYPE;
 		*xr = value_get_as_float (x);
 		return CRIT_FLOAT;
 	}
 
 	vx = format_match (value_peek_string (x), NULL, crit->date_conv);
-	if (!vx)
-		return CRIT_BADFLOAT;
+	if (VALUE_IS_EMPTY (vx) ||
+	    VALUE_IS_BOOLEAN (y) != VALUE_IS_BOOLEAN (vx)) {
+		value_release (vx);
+		return CRIT_WRONGTYPE;
+	}
 
 	*xr = value_get_as_float (vx);
 	value_release (vx);
@@ -1380,7 +1385,7 @@ criteria_test_equal (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 		return FALSE;
 	case CRIT_FLOAT:
 		return xf == yf;
@@ -1400,7 +1405,7 @@ criteria_test_unequal (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 		return FALSE;
 	case CRIT_FLOAT:
 		return xf != yf;
@@ -1420,7 +1425,7 @@ criteria_test_less (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 	case CRIT_STRING:
 		return FALSE;
 	case CRIT_FLOAT:
@@ -1437,7 +1442,7 @@ criteria_test_greater (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 	case CRIT_STRING:
 		return FALSE;
 	case CRIT_FLOAT:
@@ -1454,7 +1459,7 @@ criteria_test_less_or_equal (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 	case CRIT_STRING:
 		return FALSE;
 	case CRIT_FLOAT:
@@ -1471,7 +1476,7 @@ criteria_test_greater_or_equal (GnmValue const *x, GnmCriteria *crit)
 	default:
 		g_assert_not_reached ();
 	case CRIT_NULL:
-	case CRIT_BADFLOAT:
+	case CRIT_WRONGTYPE:
 	case CRIT_STRING:
 		return FALSE;
 	case CRIT_FLOAT:
@@ -1482,12 +1487,11 @@ criteria_test_greater_or_equal (GnmValue const *x, GnmCriteria *crit)
 static gboolean
 criteria_test_match (GnmValue const *x, GnmCriteria *crit)
 {
-	const char *xs;
 	if (!crit->has_rx)
 		return FALSE;
 
-	xs = x ? value_peek_string (x) : "";
-	return go_regexec (&crit->rx, xs, 0, NULL, 0) == GO_REG_OK;
+	return go_regexec (&crit->rx, value_peek_string (x), 0, NULL, 0) ==
+		GO_REG_OK;
 }
 
 /*
@@ -1591,6 +1595,7 @@ parse_criteria (GnmValue const *crit_val, GODateConventions const *date_conv)
 	int len;
 	char const *criteria;
 	GnmCriteria *res = g_new0 (GnmCriteria, 1);
+	GnmValue *empty;
 
 	res->iter_flags = CELL_ITER_IGNORE_BLANK;
 	res->date_conv = date_conv;
@@ -1611,7 +1616,6 @@ parse_criteria (GnmValue const *crit_val, GODateConventions const *date_conv)
 	} else if (strncmp (criteria, "<>", 2) == 0) {
 		res->fun = criteria_test_unequal;
 		len = 2;
-		res->iter_flags &= ~CELL_ITER_IGNORE_BLANK;
 	} else if (*criteria == '<') {
 		res->fun = criteria_test_less;
 		len = 1;
@@ -1624,9 +1628,6 @@ parse_criteria (GnmValue const *crit_val, GODateConventions const *date_conv)
 	} else {
 		res->fun = criteria_test_match;
 		res->has_rx = (gnm_regcomp_XL (&res->rx, criteria, 0, TRUE) == GO_REG_OK);
-		if (res->fun (NULL, res))
-			res->iter_flags &= ~CELL_ITER_IGNORE_BLANK;
-
 		len = 0;
 	}
 
@@ -1634,6 +1635,11 @@ parse_criteria (GnmValue const *crit_val, GODateConventions const *date_conv)
 	if (res->x == NULL)
 		res->x = value_new_string (criteria + len);
 
+	empty = value_new_empty ();
+	if (res->fun (empty, res))
+		res->iter_flags &= ~CELL_ITER_IGNORE_BLANK;
+	value_release (empty);
+
 	return res;
 }