GNOME.org
 

[gnumeric] Names: minor cleanup and debug infrastructure.

commit dbbcda4a5e3de8d1fade03b28788d955fcd963cb
Author: Morten Welinder <terra gnome org>
Date:   Tue May 17 13:42:49 2011 -0400

    Names: minor cleanup and debug infrastructure.

 ChangeLog                       |   15 ++++++
 plugins/excel/ms-formula-read.c |    2 +-
 src/expr-name.c                 |  103 ++++++++++++++++++++++++---------------
 src/expr-name.h                 |    1 +
 src/sheet.c                     |    5 +-
 src/workbook.c                  |    2 +-
 src/xml-sax-write.c             |   27 +++++-----
 7 files changed, 98 insertions(+), 57 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index e658e47..e000668 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2011-05-17  Morten Welinder  <terra gnome org>
+
+	* src/xml-sax-write.c (xml_write_named_expressions): Do nothing if
+	there are no names.
+
+	* src/expr-name.c (gnm_named_expr_collection_new): Make public.
+	(expr_name_new): Temporarily undo last change.  Remove second
+	argument.
+
+	* src/sheet.c (gnm_sheet_init): Always have a names member.
+	* src/workbook.c (workbook_init): Ditto.
+
+	* src/expr-name.c (expr_name_add): Do not create named collections
+	on demand.
+
 2011-05-17  Andreas J. Guelzow <aguelzow pyrshep ca>
 
 	* src/print-cell.c (print_cell_background_gtk): don't
diff --git a/plugins/excel/ms-formula-read.c b/plugins/excel/ms-formula-read.c
index 2e7d87a..e6fb394 100644
--- a/plugins/excel/ms-formula-read.c
+++ b/plugins/excel/ms-formula-read.c
@@ -1461,7 +1461,7 @@ excel_parse_formula1 (MSContainer const *container,
 					if (name_idx >= names->len)
 						g_ptr_array_set_size (names, name_idx);
 					nexpr = g_ptr_array_index (names, name_idx-1) =
-						expr_name_new (stub_name, TRUE);
+						expr_name_new (stub_name);
 					name = gnm_expr_new_name (nexpr, NULL, NULL);
 					d (1, fprintf (stderr, "creating stub '%s'", stub_name););
 					g_free (stub_name);
diff --git a/src/expr-name.c b/src/expr-name.c
index 2cc9fcd..8de925c 100644
--- a/src/expr-name.c
+++ b/src/expr-name.c
@@ -174,7 +174,7 @@ expr_name_relink_deps (GnmNamedExpr *nexpr)
 	g_slist_free (deps);
 }
 
-static GnmNamedExprCollection *
+GnmNamedExprCollection *
 gnm_named_expr_collection_new (void)
 {
 	GnmNamedExprCollection *res = g_new (GnmNamedExprCollection, 1);
@@ -291,6 +291,17 @@ static void
 gnm_named_expr_collection_insert (GnmNamedExprCollection const *scope,
 				  GnmNamedExpr *nexpr)
 {
+	if (gnm_debug_flag ("names")) {
+		char *scope_name = nexpr->pos.sheet
+			? g_strdup_printf ("sheet %s", nexpr->pos.sheet->name_quoted)
+			: g_strdup ("workbook");
+		g_printerr ("Inserting name %s into its %s container%s\n",
+			    nexpr->name->str,
+			    scope_name,
+			    nexpr->is_placeholder ? " as a placeholder" : "");
+		g_free (scope_name);
+	}
+
 	/* name can be active at this point, eg we are converting a
 	 * placeholder, or changing a scope */
 	nexpr->active = TRUE;
@@ -440,8 +451,8 @@ expr_name_lookup (GnmParsePos const *pp, char const *name)
  *
  * Creates a new name without linking it into any container.
  **/
-static GnmNamedExpr *
-expr_name_new (char const *name, gboolean is_placeholder)
+GnmNamedExpr *
+expr_name_new (char const *name)
 {
 	GnmNamedExpr *nexpr;
 
@@ -454,12 +465,13 @@ expr_name_new (char const *name, gboolean is_placeholder)
 	nexpr->name		= go_string_new (name);
 	nexpr->texpr		= NULL;
 	nexpr->dependents	= NULL;
-	nexpr->is_placeholder	= is_placeholder;
+	nexpr->is_placeholder	= TRUE;
 	nexpr->is_hidden	= FALSE;
 	nexpr->is_permanent	= FALSE;
 	nexpr->is_editable	= TRUE;
 
-	g_return_val_if_fail (nexpr->name != NULL, NULL);
+	if (gnm_debug_flag ("names"))
+		g_printerr ("Created new name %s\n", name);
 
 	return nexpr;
 }
@@ -572,42 +584,37 @@ expr_name_add (GnmParsePos const *pp, char const *name,
 	}
 
 	scope = (pp->sheet != NULL) ? pp->sheet->names : pp->wb->names;
-	if (scope != NULL) {
-		/* see if there was a place holder */
-		nexpr = g_hash_table_lookup (scope->placeholders, name);
-		if (nexpr != NULL) {
-			if (texpr == NULL) {
-				/* there was already a placeholder for this */
-				expr_name_ref (nexpr);
-				return nexpr;
-			}
+	/* see if there was a place holder */
+	nexpr = g_hash_table_lookup (scope->placeholders, name);
+	if (nexpr != NULL) {
+		if (texpr == NULL) {
+			/* there was already a placeholder for this */
+			expr_name_ref (nexpr);
+			return nexpr;
+		}
 
-			/* convert the placeholder into a real name */
-			g_hash_table_steal (scope->placeholders, name);
-			nexpr->is_placeholder = FALSE;
-		} else {
-			nexpr = g_hash_table_lookup (scope->names, name);
-			/* If this is a permanent name, we may be adding it */
-			/* on opening of a file, although */
-			/* the name is already in place. */
-			if (nexpr != NULL) {
-				if (nexpr->is_permanent)
-					link_to_container = FALSE;
-				else {
-					if (error_msg != NULL)
-						*error_msg = (pp->sheet != NULL)
-							? g_strdup_printf (_("'%s' is already defined in sheet"), name)
-							: g_strdup_printf (_("'%s' is already defined in workbook"), name);
-
-					gnm_expr_top_unref (texpr);
-					return NULL;
-				}
+		/* convert the placeholder into a real name */
+		g_hash_table_steal (scope->placeholders, name);
+		nexpr->is_placeholder = FALSE;
+	} else {
+		nexpr = g_hash_table_lookup (scope->names, name);
+		/* If this is a permanent name, we may be adding it */
+		/* on opening of a file, although */
+		/* the name is already in place. */
+		if (nexpr != NULL) {
+			if (nexpr->is_permanent)
+				link_to_container = FALSE;
+			else {
+				if (error_msg != NULL)
+					*error_msg = (pp->sheet != NULL)
+						? g_strdup_printf (_("'%s' is already defined in sheet"), name)
+						: g_strdup_printf (_("'%s' is already defined in workbook"), name);
+
+				gnm_expr_top_unref (texpr);
+				return NULL;
 			}
 		}
-	} else if (pp->sheet != NULL)
-		scope = pp->sheet->names = gnm_named_expr_collection_new ();
-	else
-		scope = pp->wb->names = gnm_named_expr_collection_new ();
+	}
 
 	if (error_msg)
 		*error_msg = NULL;
@@ -618,8 +625,10 @@ expr_name_add (GnmParsePos const *pp, char const *name,
 			stub->is_placeholder = FALSE;
 			go_string_unref (stub->name);
 			stub->name = go_string_new (name);
-		} else
-			nexpr = expr_name_new (name, texpr == NULL);
+		} else {
+			nexpr = expr_name_new (name);
+			nexpr->is_placeholder = (texpr == NULL);
+		}
 	}
 	parse_pos_init (&nexpr->pos,
 		pp->wb, pp->sheet, pp->eval.col, pp->eval.row);
@@ -651,6 +660,9 @@ expr_name_unref (GnmNamedExpr *nexpr)
 
 	g_return_if_fail (!nexpr->active);
 
+	if (gnm_debug_flag ("names"))
+		g_printerr ("Finalizing name %s\n", nexpr->name->str);
+
 	if (nexpr->name) {
 		go_string_unref (nexpr->name);
 		nexpr->name = NULL;
@@ -692,6 +704,17 @@ expr_name_remove (GnmNamedExpr *nexpr)
 
 	g_return_if_fail (scope != NULL);
 
+	if (gnm_debug_flag ("names")) {
+		char *scope_name = nexpr->pos.sheet
+			? g_strdup_printf ("sheet %s", nexpr->pos.sheet->name_quoted)
+			: g_strdup ("workbook");
+		g_printerr ("Removing name %s from its %s container%s\n",
+			    nexpr->name->str,
+			    scope_name,
+			    nexpr->is_placeholder ? " as a placeholder" : "");
+		g_free (scope_name);
+	}
+
 	g_hash_table_remove (
 		nexpr->is_placeholder ? scope->placeholders : scope->names,
 		nexpr->name->str);
diff --git a/src/expr-name.h b/src/expr-name.h
index 8c1097f..af52f38 100644
--- a/src/expr-name.h
+++ b/src/expr-name.h
@@ -69,6 +69,7 @@ struct _GnmNamedExprCollection {
 	GHashTable *placeholders;
 };
 
+GnmNamedExprCollection *gnm_named_expr_collection_new (void);
 void gnm_named_expr_collection_free (GnmNamedExprCollection *names);
 void gnm_named_expr_collection_unlink (GnmNamedExprCollection *names);
 void gnm_named_expr_collection_relink (GnmNamedExprCollection *names);
diff --git a/src/sheet.c b/src/sheet.c
index 1f9223e..83f68d4 100644
--- a/src/sheet.c
+++ b/src/sheet.c
@@ -800,7 +800,7 @@ gnm_sheet_init (Sheet *sheet)
 	/* Init menu states */
 	sheet->priv->enable_showhide_detail = TRUE;
 
-	sheet->names = NULL;
+	sheet->names = gnm_named_expr_collection_new ();
 	sheet->style_data = NULL;
 
 	sheet->index_in_wb = -1;
@@ -3435,7 +3435,8 @@ sheet_colrow_optimize1 (int max, int max_used, ColRowCollection *collection)
 				segment->info[j] = NULL;
 			} else {
 				any = TRUE;
-				max_used = i + j;
+				if (i + j >= first_unused)
+					max_used = i + j;
 			}
 		}
 
diff --git a/src/workbook.c b/src/workbook.c
index 710340d..489912a 100644
--- a/src/workbook.c
+++ b/src/workbook.c
@@ -173,7 +173,7 @@ workbook_init (GObject *object)
 	wb->sheet_hash_private = g_hash_table_new (g_str_hash, g_str_equal);
 	wb->sheet_order_dependents = NULL;
 	wb->sheet_local_functions = NULL;
-	wb->names        = NULL;
+	wb->names = gnm_named_expr_collection_new ();
 
 	/* Nothing to undo or redo */
 	wb->undo_commands = wb->redo_commands = NULL;
diff --git a/src/xml-sax-write.c b/src/xml-sax-write.c
index 7f58497..9e6dce1 100644
--- a/src/xml-sax-write.c
+++ b/src/xml-sax-write.c
@@ -215,20 +215,21 @@ xml_write_name (GnmOutputXML *state, GnmNamedExpr *nexpr)
 static void
 xml_write_named_expressions (GnmOutputXML *state, GnmNamedExprCollection *scope)
 {
-	if (scope != NULL) {
-		GSList *names =
-			g_slist_sort (gnm_named_expr_collection_list (scope),
-				      (GCompareFunc)expr_name_cmp_by_name);
-		GSList *p;
-
-		gsf_xml_out_start_element (state->output, GNM "Names");
-		for (p = names; p; p = p->next) {
-			GnmNamedExpr *nexpr = p->data;
-			xml_write_name (state, nexpr);
-		}
-		gsf_xml_out_end_element (state->output); /* </gnm:Names> */
-		g_slist_free (names);
+	GSList *names =
+		g_slist_sort (gnm_named_expr_collection_list (scope),
+			      (GCompareFunc)expr_name_cmp_by_name);
+	GSList *p;
+
+	if (!names)
+		return;
+
+	gsf_xml_out_start_element (state->output, GNM "Names");
+	for (p = names; p; p = p->next) {
+		GnmNamedExpr *nexpr = p->data;
+		xml_write_name (state, nexpr);
 	}
+	gsf_xml_out_end_element (state->output); /* </gnm:Names> */
+	g_slist_free (names);
 }
 
 static void
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]