[gnumeric] some more define-names adjustments

[Andreas J. Guelzow <guelzow src gnome org>]

commit d58125ceb903a932965677d1fc93c9c65c7c170c
Author: Andreas J Guelzow <aguelzow pyrshep ca>
Date:   Fri Jun 4 15:57:15 2010 -0600

    some more define-names adjustments
    
    2010-06-04 Andreas J. Guelzow <aguelzow pyrshep ca>
    
    	* dialog-define-names.c (name_guru_parse_pos_init): new
    	(name_guru_check_expression): use name_guru_parse_pos_init
    	(cb_name_guru_name_edited): don't duplicate errorchecks already
    	  in cmd_define_name but make sure that we don't just redefine
    	  names
    
    2010-06-04 Andreas J. Guelzow <aguelzow pyrshep ca>
    
    	* src/commands.c (cmd_define_name): be clearer with the error
    	  messages.
    	(cmd_rescope_name_redo): watch out for returned error strings

 ChangeLog                         |    6 ++
 src/commands.c                    |   26 ++++++++--
 src/dialogs/ChangeLog             |    8 +++
 src/dialogs/dialog-define-names.c |  109 ++++++++++++++++++++-----------------
 4 files changed, 95 insertions(+), 54 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index a37745f..258951a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-06-04 Andreas J. Guelzow <aguelzow pyrshep ca>
+
+	* src/commands.c (cmd_define_name): be clearer with the error 
+	  messages.
+	(cmd_rescope_name_redo): watch out for returned error strings
+
 2010-06-04  Morten Welinder  <terra gnome org>
 
 	* src/gnm-pane.c (gnm_pane_display_obj_size_tip): Fix critical.
diff --git a/src/commands.c b/src/commands.c
index 162fde5..8a58262 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -5837,10 +5837,21 @@ cmd_define_name (WorkbookControl *wbc, char const *name,
 	g_return_val_if_fail (pp != NULL, TRUE);
 	g_return_val_if_fail (texpr != NULL, TRUE);
 
+	if (name[0] == '\0') {
+		go_cmd_context_error_invalid 
+			(GO_CMD_CONTEXT (wbc), _("Defined Name"),
+			 _("An empty string is not allowed as defined name."));
+		gnm_expr_top_unref (texpr);
+		return TRUE;
+	}
+
 	sheet = wb_control_cur_sheet (wbc);
 	if (!expr_name_validate (name, sheet)) {
-		go_cmd_context_error_invalid (GO_CMD_CONTEXT (wbc), name,
-					_("is not allowed as defined name"));
+		gchar *err = g_strdup_printf 
+			(_("'%s' is not allowed as defined name."), name);
+		go_cmd_context_error_invalid (GO_CMD_CONTEXT (wbc), 
+					      _("Defined Name"), err);
+		g_free (err);
 		gnm_expr_top_unref (texpr);
 		return TRUE;
 	}
@@ -5997,12 +6008,19 @@ typedef struct {
 MAKE_GNM_COMMAND (CmdRescopeName, cmd_rescope_name, NULL)
 
 static gboolean
-cmd_rescope_name_redo (GnmCommand *cmd, G_GNUC_UNUSED WorkbookControl *wbc)
+cmd_rescope_name_redo (GnmCommand *cmd, WorkbookControl *wbc)
 {
 	CmdRescopeName *me = CMD_RESCOPE_NAME (cmd);
 	Sheet *old_scope = me->nexpr->pos.sheet;
+	char *err;
 	
-	expr_name_set_scope (me->nexpr, me->scope);
+	err = expr_name_set_scope (me->nexpr, me->scope);
+
+	if (err != NULL) {
+		go_cmd_context_error_invalid (GO_CMD_CONTEXT (wbc), _("Change Scope of Name"), err);
+		g_free (err);
+		return TRUE;
+	}
 
 	me->scope = old_scope;
 	return FALSE;
diff --git a/src/dialogs/ChangeLog b/src/dialogs/ChangeLog
index 4922f8b..3abd4b2 100644
--- a/src/dialogs/ChangeLog
+++ b/src/dialogs/ChangeLog
@@ -1,5 +1,13 @@
 2010-06-04 Andreas J. Guelzow <aguelzow pyrshep ca>
 
+	* dialog-define-names.c (name_guru_parse_pos_init): new
+	(name_guru_check_expression): use name_guru_parse_pos_init
+	(cb_name_guru_name_edited): don't duplicate errorchecks already
+	  in cmd_define_name but make sure that we don't just redefine 
+	  names
+
+2010-06-04 Andreas J. Guelzow <aguelzow pyrshep ca>
+
 	* dialog-define-names.c (name_guru_move_record): change
 	  argument to iter from path_string, change caller
 	(cb_name_guru_name_edited): move the modified record into the
diff --git a/src/dialogs/dialog-define-names.c b/src/dialogs/dialog-define-names.c
index 2df3166..27ed6dd 100644
--- a/src/dialogs/dialog-define-names.c
+++ b/src/dialogs/dialog-define-names.c
@@ -820,37 +820,45 @@ cb_name_guru_switch_scope (G_GNUC_UNUSED GtkCellRendererToggle *cell,
 	}
 }
 
-/*
- * Return the expression if it is acceptable.
- * The parse position will be initialized then.
- */
-
-static  GnmExprTop const*
-name_guru_check_expression (NameGuruState *state, gchar *text, 
-			    GnmParsePos *pp, item_type_t type)
+static gboolean
+name_guru_parse_pos_init (NameGuruState *state,
+			  GnmParsePos *pp, item_type_t type)
 {
-	GnmExprTop const *texpr;
-	GnmParseError	  perr;
-
 	switch (type) {
 	case item_type_available_wb_name:
 	case item_type_new_unsaved_wb_name:
 		parse_pos_init (pp, state->wb, NULL,
 				state->pp.eval.col, state->pp.eval.row);
-		break;
+		return TRUE;
 	case item_type_available_sheet_name:
 	case item_type_new_unsaved_sheet_name:
 		parse_pos_init (pp, state->wb, state->sheet,
 				state->pp.eval.col, state->pp.eval.row);
-		break;
+		return TRUE;
 	case item_type_workbook:
 	case item_type_main_sheet:
 	case item_type_other_sheet:
 	case item_type_locked_name:
 	case item_type_foreign_name:
 	default:
+		return FALSE;
+	}	
+}
+
+/*
+ * Return the expression if it is acceptable.
+ * The parse position will be initialized then.
+ */
+
+static  GnmExprTop const*
+name_guru_check_expression (NameGuruState *state, gchar *text, 
+			    GnmParsePos *pp, item_type_t type)
+{
+	GnmExprTop const *texpr;
+	GnmParseError	  perr;
+
+	if (!name_guru_parse_pos_init (state, pp, type))
 		return NULL; /* We should have never gotten here. */
-	}
 
 	if (text == NULL || text[0] == '\0') {
 		go_gtk_notice_dialog (GTK_WINDOW (state->dialog), 
@@ -947,6 +955,8 @@ cb_name_guru_name_edited (G_GNUC_UNUSED GtkCellRendererText *cell,
 	gchar            *content;
 	GnmNamedExpr     *nexpr;
 
+	g_return_if_fail (new_text != NULL);
+
 	if (!name_guru_translate_pathstring_to_iter
 	    (state, &iter, path_string))
 		return;
@@ -960,48 +970,47 @@ cb_name_guru_name_edited (G_GNUC_UNUSED GtkCellRendererText *cell,
 	    type != item_type_new_unsaved_sheet_name)
 		return;
 
-	if (new_text == NULL || new_text[0] == '\0') {
-		go_gtk_notice_dialog (GTK_WINDOW (state->dialog), 
-				      GTK_MESSAGE_ERROR,
-				      _("The empty string is not a valid "
-					"name for a named expression!"));
-		return;
-	}
+	name_guru_parse_pos_init (state, &pp, type);
+	nexpr = expr_name_lookup (&pp, new_text);
 
-	if (!expr_name_validate (new_text, state->sheet)) {
-		go_gtk_notice_dialog (GTK_WINDOW (state->dialog),
-				      GTK_MESSAGE_ERROR,
-				      _("Invalid name"));
-		return;
+	if (nexpr != NULL) {
+		Sheet *scope = nexpr->pos.sheet;
+		if ((type == item_type_new_unsaved_wb_name && 
+		     scope == NULL) || 
+		    (type == item_type_new_unsaved_sheet_name)) {
+			go_gtk_notice_dialog (GTK_WINDOW (state->dialog), 
+					      GTK_MESSAGE_ERROR,
+					      _("This name is already in use!"));
+			return;
+		}
 	}
-
-#warning We need to check whter the name already exists!
-
+	
 	texpr = name_guru_check_expression (state, content, &pp , type);
 	if (texpr == NULL)
 		return;
-	
-	cmd_define_name (WORKBOOK_CONTROL (state->wbcg), 
-			 new_text, &pp,
-			 texpr, NULL);
-	nexpr = expr_name_lookup (&pp, new_text);
-
-	type = (type == item_type_new_unsaved_wb_name) ? 
-		item_type_available_wb_name :
-		item_type_new_unsaved_sheet_name;
 
-	gtk_tree_store_set 
-		(state->model, &iter, 
-		 ITEM_NAME, new_text,
-		 ITEM_NAME_POINTER, nexpr,
-		 ITEM_TYPE, type,
-		 ITEM_PASTABLE, TRUE,
-		 -1);
-	name_guru_set_images (state, &iter, type, TRUE);
-
-	if (gtk_tree_model_iter_parent (GTK_TREE_MODEL (state->model),
-					&parent_iter, &iter))
-		name_guru_move_record (state, &iter, &parent_iter, type);
+	if (!cmd_define_name (WORKBOOK_CONTROL (state->wbcg), 
+			      new_text, &pp,
+			      texpr, NULL)) {
+		nexpr = expr_name_lookup (&pp, new_text);
+		
+		type = (type == item_type_new_unsaved_wb_name) ? 
+			item_type_available_wb_name :
+			item_type_available_sheet_name;
+
+		gtk_tree_store_set 
+			(state->model, &iter, 
+			 ITEM_NAME, new_text,
+			 ITEM_NAME_POINTER, nexpr,
+			 ITEM_TYPE, type,
+			 ITEM_PASTABLE, TRUE,
+			 -1);
+		name_guru_set_images (state, &iter, type, TRUE);
+		
+		if (gtk_tree_model_iter_parent (GTK_TREE_MODEL (state->model),
+						&parent_iter, &iter))
+			name_guru_move_record (state, &iter, &parent_iter, type);
+	}
 }
 
 static void