[gnumeric] Names: cleanups.

[Morten Welinder <mortenw src gnome org>]

commit 6a4af3406612d04f65542559d7a57b9e82dd5a72
Author: Morten Welinder <terra gnome org>
Date:   Sat May 21 17:43:17 2011 -0400

    Names: cleanups.

 ChangeLog                            |    8 +++
 plugins/openoffice/ChangeLog         |    4 +
 plugins/openoffice/openoffice-read.c |   16 +++---
 src/commands.c                       |    4 +-
 src/expr-name.c                      |  115 ++++++++++++++++++++--------------
 src/expr-name.h                      |    9 +--
 src/ssconvert.c                      |   28 +++++----
 7 files changed, 112 insertions(+), 72 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index eb2eab5..26217ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-05-21  Morten Welinder  <terra gnome org>
+
+	* src/expr-name.c (expr_name_set_pos): Rename from
+	expr_name_set_scope and extended to handle all position changes.
+	All callers changed.
+	(gnm_named_expr_collection_rename): Remove.
+	(expr_name_set_name): New function.
+
 2011-05-21  Morten Welinder <terra gnome org>
 
 	* configure.in: Post-release bump.
diff --git a/plugins/openoffice/ChangeLog b/plugins/openoffice/ChangeLog
index 0199c03..fec5db3 100644
--- a/plugins/openoffice/ChangeLog
+++ b/plugins/openoffice/ChangeLog
@@ -1,3 +1,7 @@
+2011-05-21  Morten Welinder  <terra gnome org>
+
+	* openoffice-read.c (odf_fix_en_apply): Use expr_name_set_name.
+
 2011-05-21  Morten Welinder <terra gnome org>
 
 	* Release 1.10.15
diff --git a/plugins/openoffice/openoffice-read.c b/plugins/openoffice/openoffice-read.c
index 5acc371..d005a8d 100644
--- a/plugins/openoffice/openoffice-read.c
+++ b/plugins/openoffice/openoffice-read.c
@@ -1321,14 +1321,11 @@ odf_fix_en_collect (gchar const *key, GnmNamedExpr *nexpr, odf_fix_expr_names_t
 }
 
 static void 
-odf_fix_en_apply (gchar const *orig, gchar const *fixed, OOParseState *state)
+odf_fix_en_apply (const char *orig, GnmNamedExpr *nexpr, GHashTable *orig2fixed)
 {
-	GSList *sheets = workbook_sheets (state->pos.wb), *l;
-	gnm_named_expr_collection_rename (state->pos.wb->names, orig, fixed);
-	for (l = sheets; l != NULL; l = l->next) {
-		Sheet *sheet = l->data;
-		gnm_named_expr_collection_rename (sheet->names, orig, fixed);
-	}
+	const char *fixed = g_hash_table_lookup (orig2fixed, orig);
+	if (fixed)
+		expr_name_set_name (nexpr, fixed);
 }
 
 /**
@@ -1355,7 +1352,10 @@ odf_fix_expr_names (OOParseState *state)
 		g_hash_table_foreach (sheet->names->placeholders, 
 				      (GHFunc) odf_fix_en_collect, fen);
 	}
-	g_hash_table_foreach (fen->orig2fixed, (GHFunc) odf_fix_en_apply, state);
+
+	workbook_foreach_name (state->pos.wb, FALSE,
+			       (GHFunc)odf_fix_en_apply, fen->orig2fixed);
+
 	odf_fix_expr_names_t_free (fen);
 }
 
diff --git a/src/commands.c b/src/commands.c
index 51c3ee6..f7c84a1 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -6167,8 +6167,10 @@ cmd_rescope_name_redo (GnmCommand *cmd, WorkbookControl *wbc)
 	CmdRescopeName *me = CMD_RESCOPE_NAME (cmd);
 	Sheet *old_scope = me->nexpr->pos.sheet;
 	char *err;
+	GnmParsePos pp = me->nexpr->pos;
 
-	err = expr_name_set_scope (me->nexpr, me->scope);
+	pp.sheet = me->scope;
+	err = expr_name_set_pos (me->nexpr, &pp);
 
 	if (err != NULL) {
 		go_cmd_context_error_invalid (GO_CMD_CONTEXT (wbc), _("Change Scope of Name"), err);
diff --git a/src/expr-name.c b/src/expr-name.c
index 9ce73af..b179731 100644
--- a/src/expr-name.c
+++ b/src/expr-name.c
@@ -367,32 +367,6 @@ gnm_named_expr_collection_foreach (GnmNamedExprCollection *names,
 	g_hash_table_foreach (names->placeholders, func, data);
 }
 
-static void
-gnm_named_expr_collection_rename_in_hash (GHashTable *hash, 
-					  gchar const *old_name, 
-					  gchar const *new_name)
-{
-	GnmNamedExpr *nexpr;
-
-	if ((nexpr = g_hash_table_lookup (hash, old_name)) != NULL) {
-		g_hash_table_steal (hash, old_name);
-		go_string_unref (nexpr->name);
-		nexpr->name = go_string_new (new_name);
-		g_hash_table_insert (hash, (gpointer)nexpr->name->str, nexpr);
-	}
-}
-
-/* rename old_name to new_name (if such a name exists) */
-void gnm_named_expr_collection_rename (GnmNamedExprCollection *names,
-				       gchar const *old_name,
-				       gchar const *new_name)
-{
-	gnm_named_expr_collection_rename_in_hash 
-		(names->names, old_name, new_name);
-	gnm_named_expr_collection_rename_in_hash 
-		(names->placeholders, old_name, new_name);
-}
-
 /******************************************************************************/
 
 /**
@@ -736,6 +710,54 @@ expr_name_name (GnmNamedExpr const *nexpr)
 }
 
 /**
+ * expr_name_set_name :
+ * @nexpr : the named expression
+ * @new_name : the new name of the expression
+ *
+ * returns: TRUE on error.
+ */
+gboolean
+expr_name_set_name (GnmNamedExpr *nexpr,
+		    const char *new_name)
+{
+	const char *old_name;
+	GHashTable *h;
+
+	g_return_val_if_fail (nexpr != NULL, TRUE);
+	g_return_val_if_fail (nexpr->scope == NULL || new_name, TRUE);
+
+	old_name = nexpr->name->str;
+	if (go_str_compare (new_name, old_name) == 0)
+		return FALSE;
+
+	h = nexpr->scope
+		? (nexpr->is_placeholder
+		   ? nexpr->scope->placeholders
+		   : nexpr->scope->names)
+		: NULL;
+	if (h) {
+		if (new_name &&
+		    (g_hash_table_lookup (nexpr->scope->placeholders, new_name) ||
+		     g_hash_table_lookup (nexpr->scope->names, new_name))) {
+			/* The only error not to be blamed on the programmer is
+			   already-in-use.  */
+			return TRUE;
+		}
+
+		g_hash_table_steal (h, old_name);
+	}
+
+	go_string_unref (nexpr->name);
+	nexpr->name = go_string_new (new_name);
+
+	if (h)
+		g_hash_table_insert (h, (gpointer)nexpr->name->str, nexpr);
+
+	return FALSE;
+}
+
+
+/**
  * expr_name_as_string :
  * @nexpr :
  * @pp : optionally null.
@@ -795,40 +817,41 @@ expr_name_downgrade_to_placeholder (GnmNamedExpr *nexpr)
  * Manage things that depend on named expressions.
  */
 /**
- * expr_name_set_scope:
- * @nexpr:
- * @sheet:
+ * expr_name_set_pos:
+ * @nexpr : the named expression
+ * @pp: the new position
  *
  * Returns a translated error string which the caller must free if something
  * goes wrong.
  **/
 char *
-expr_name_set_scope (GnmNamedExpr *nexpr, Sheet *sheet)
+expr_name_set_pos (GnmNamedExpr *nexpr, GnmParsePos const *pp)
 {
-	GnmNamedExprCollection *scope, *new_scope;
+	GnmNamedExprCollection *old_scope, *new_scope;
+	const char *name;
 
 	g_return_val_if_fail (nexpr != NULL, NULL);
-	g_return_val_if_fail (nexpr->pos.sheet != NULL || nexpr->pos.wb != NULL, NULL);
 	g_return_val_if_fail (nexpr->scope != NULL, NULL);
+	g_return_val_if_fail (pp != NULL, NULL);
 
-	scope = nexpr->scope;
-
-	g_return_val_if_fail (scope != NULL, NULL);
-
-	new_scope = sheet ? sheet->names : nexpr->pos.wb->names;
-	if (new_scope != NULL) {
-		if (NULL != g_hash_table_lookup (new_scope->placeholders, nexpr->name->str) ||
-		    NULL != g_hash_table_lookup (new_scope->names, nexpr->name->str))
-			return g_strdup_printf (((sheet != NULL)
-				? _("'%s' is already defined in sheet")
-				: _("'%s' is already defined in workbook")), nexpr->name->str);
+	old_scope = nexpr->scope;
+	new_scope = pp->sheet ? pp->sheet->names : pp->wb->names;
+
+	name = nexpr->name->str;
+	if (old_scope != new_scope &&
+	    (g_hash_table_lookup (new_scope->placeholders, name) ||
+	     g_hash_table_lookup (new_scope->names, name))) {
+		const char *fmt = pp->sheet
+			? _("'%s' is already defined in sheet")
+			: _("'%s' is already defined in workbook");
+		return g_strdup_printf (fmt, name);
 	}
 
 	g_hash_table_steal (
-		nexpr->is_placeholder ? scope->placeholders : scope->names,
-		nexpr->name->str);
+		nexpr->is_placeholder ? old_scope->placeholders : old_scope->names,
+		name);
 
-	nexpr->pos.sheet = sheet;
+	nexpr->pos = *pp;
 	gnm_named_expr_collection_insert (new_scope, nexpr);
 	return NULL;
 }
diff --git a/src/expr-name.h b/src/expr-name.h
index 2ce6ff8..9789034 100644
--- a/src/expr-name.h
+++ b/src/expr-name.h
@@ -40,10 +40,13 @@ void	 expr_name_unref      (GnmNamedExpr *nexpr);
 void     expr_name_remove     (GnmNamedExpr *nexpr);
 GnmValue*expr_name_eval       (GnmNamedExpr const *ne, GnmEvalPos const *ep,
 			       GnmExprEvalFlags flags);
+
 const char *expr_name_name    (GnmNamedExpr const *nexpr);
+gboolean expr_name_set_name   (GnmNamedExpr *nexpr, const char *new_name);
+
 char    *expr_name_as_string  (GnmNamedExpr const *ne, GnmParsePos const *pp,
 			       GnmConventions const *fmt);
-char    *expr_name_set_scope  (GnmNamedExpr *ne, Sheet *sheet);
+char    *expr_name_set_pos    (GnmNamedExpr *ne, GnmParsePos const *pp);
 void	 expr_name_set_expr   (GnmNamedExpr *ne, GnmExprTop const *texpr);
 void	 expr_name_add_dep    (GnmNamedExpr *ne, GnmDependent *dep);
 void	 expr_name_remove_dep (GnmNamedExpr *ne, GnmDependent *dep);
@@ -81,10 +84,6 @@ void gnm_named_expr_collection_foreach (GnmNamedExprCollection *names,
 
 GnmNamedExpr *gnm_named_expr_collection_lookup (GnmNamedExprCollection const *scope,
 						char const *name);
-void gnm_named_expr_collection_rename (GnmNamedExprCollection *names,
-				       gchar const *old_name,
-				       gchar const *new_name);
-
 G_END_DECLS
 
 #endif /* _GNM_EXPR_NAME_H_ */
diff --git a/src/ssconvert.c b/src/ssconvert.c
index 47c887a..49f1380 100644
--- a/src/ssconvert.c
+++ b/src/ssconvert.c
@@ -303,8 +303,15 @@ suggest_size (GSList *wbs, int *csuggest, int *rsuggest)
 static void
 cb_fixup_name_wb (const char *name, GnmNamedExpr *nexpr, Workbook *wb)
 {
-	if (nexpr->pos.wb)
-		nexpr->pos.wb = wb;
+	GnmParsePos newpos = nexpr->pos;
+
+	if (!expr_name_is_active (nexpr))
+		return;
+
+	if (nexpr->pos.wb) {
+		newpos.wb = wb;
+		expr_name_set_pos (nexpr, &newpos);
+	}
 }
 
 
@@ -320,26 +327,25 @@ merge_single (Workbook *wb, Workbook *wb2,
 	GSList *names = g_slist_sort (gnm_named_expr_collection_list (wb2->names),
 				      (GCompareFunc)expr_name_cmp_by_name);
 	GSList *p;
-	GnmParsePos pp;
-
-	parse_pos_init (&pp, wb, NULL, 0, 0);
 
 	for (p = names; p; p = p->next) {
 		GnmNamedExpr *nexpr = p->data;
 		const char *name = expr_name_name (nexpr);
 		GnmNamedExpr *nexpr2;
-		Sheet *sheet;
+		GnmParsePos pp;
+		GnmParsePos newpos = nexpr->pos;
 
 		if (!expr_name_is_active (nexpr))
 			continue;
 
-		if (nexpr->pos.wb == NULL || nexpr->pos.sheet != NULL)
+		if (nexpr->pos.wb != wb2 || nexpr->pos.sheet != NULL)
 			continue;
 
 		/* Check for clash with existing name */
 
+		parse_pos_init (&pp, wb, NULL, 0, 0);
 		nexpr2 = expr_name_lookup (&pp, name);
-		if (nexpr2 != NULL) {
+		if (nexpr2 /* FIXME: && nexpr2-is-not-the-same-as-nexpr */) {
 			g_printerr (_("Name conflict during merge: '%s' appears twice at workbook scope.\n"),
 				    name);
 			g_slist_free (names);
@@ -347,10 +353,8 @@ merge_single (Workbook *wb, Workbook *wb2,
 		}
 
 		/* Move name scope to workbook wb */
-		sheet = workbook_sheet_by_index (wb2, 0);
-		expr_name_set_scope (nexpr, sheet);
-		nexpr->pos.wb = wb;
-		expr_name_set_scope (nexpr, NULL);
+		newpos.wb = wb;
+		expr_name_set_pos (nexpr, &newpos);
 	}
 	g_slist_free (names);