Re: [evolution-patches] Dont' overwrite folders on rename (shell, #45982)

[Ettore Perazzoli <ettore ximian com>]

> Existing code, but the (shell != NULL) check is redundant now.  The Gtk2
> type checking macros have it built in.  We should be removing these all
> over.

Yeah, good point.

> > -	while (1) {
> > +	name_ok = FALSE;
> 
> Might as well initialize name_ok on the declaration line and kill this
> line.

No.  The boolean is only used in the while() loop and initializations
should be done near where they are needed, to make the code more
readable.

> shell_view is preconditioned != null above, no need to check it here.

Hmm yeah.  I think originally it was supposed to work with a NULL
shell_view, then it got changed.

> > +		if (! e_shell_folder_name_is_valid (new_name, &reason)) {
> 
> "if not valid, else."  Love that negative logic.

The negative logic is to keep the short branch in the if() part, which
makes the code more readable.

> > +			g_free (new_path);
> > +			old_base_path = g_path_get_dirname (folder_path);
> > +			new_path = g_build_filename (old_base_path, new_name, NULL);
> > +
> > +			if (e_storage_set_get_folder (storage_set, new_path) == NULL) {
> 
> Do we own the EFolder ref that's returned?  Is this a leak?  It's hard
> to tell since that's returned by a class method down in e-storage.c.

It's not a leak, the folder doesn't get reffed.

Anyway, the patch was indeed quite ugly, sorry.  I am not sure what I
was thinking, I guess I was having a bad day.

Updated, cleaned up patch attached.

-- Ettore

Index: e-shell-folder-commands.c
===================================================================
RCS file: /cvs/gnome/evolution/shell/e-shell-folder-commands.c,v
retrieving revision 1.58
diff -u -p -r1.58 e-shell-folder-commands.c
--- e-shell-folder-commands.c	7 May 2003 19:38:18 -0000	1.58
+++ e-shell-folder-commands.c	9 Jul 2003 18:33:13 -0000
@@ -517,13 +517,9 @@ e_shell_command_rename_folder (EShell *s
 	RenameCallbackData *callback_data;
 	const char *old_name;
 	char *prompt;
-	char *new_name;
-	char *old_base_path;
-	char *new_path;
+	gboolean done;
 
-	g_return_if_fail (shell != NULL);
 	g_return_if_fail (E_IS_SHELL (shell));
-	g_return_if_fail (shell_view != NULL);
 	g_return_if_fail (E_IS_SHELL_VIEW (shell_view));
 
 	storage_set = e_shell_get_storage_set (shell);
@@ -537,38 +533,39 @@ e_shell_command_rename_folder (EShell *s
 	old_name = e_folder_get_name (folder);
 	prompt = g_strdup_printf (_("Rename the \"%s\" folder to:"), old_name);
 
-	while (1) {
+	done = FALSE;
+	while (! done) {
 		const char *reason;
+		char *new_name;
 
-		new_name = e_request_string (shell_view != NULL ? GTK_WINDOW (shell_view) : NULL,
-					     _("Rename Folder"), prompt, old_name);
+		new_name = e_request_string (GTK_WINDOW (shell_view), _("Rename Folder"), prompt, old_name);
 
-		if (new_name == NULL)
-			return;
-
-		if (e_shell_folder_name_is_valid (new_name, &reason))
-			break;
-
-		e_notice (shell_view, GTK_MESSAGE_ERROR,
-			  _("The specified folder name is not valid: %s"), reason);
+		if (new_name == NULL || strcmp (old_name, new_name) == 0) {
+		        done = TRUE;
+		} else if (! e_shell_folder_name_is_valid (new_name, &reason)) {
+			e_notice (shell_view, GTK_MESSAGE_ERROR,
+				  _("The specified folder name is not valid: %s"), reason);
+		} else {
+			char *old_base_path = g_path_get_dirname (folder_path);
+			char *new_path = g_build_filename (old_base_path, new_name, NULL);
+
+			if (e_storage_set_get_folder (storage_set, new_path) != NULL) {
+				e_notice (shell_view, GTK_MESSAGE_ERROR,
+					  _("A folder named \"%s\" already exists. Please use a different name."),
+					  new_name);
+			} else {
+				callback_data = rename_callback_data_new (shell_view, new_path);
+				e_storage_set_async_xfer_folder (storage_set, folder_path, new_path, TRUE,
+								 rename_cb, callback_data);
+				done = TRUE;
+			}
+
+			g_free (old_base_path);
+			g_free (new_path);
+		}
 	}
 
 	g_free (prompt);
-
-	if (strcmp (old_name, new_name) == 0) {
-		g_free (new_name);
-		return;
-	}
-
-	old_base_path = g_path_get_dirname (folder_path);
-	new_path = g_build_filename (old_base_path, new_name, NULL);
-
-	callback_data = rename_callback_data_new (shell_view, new_path);
-	e_storage_set_async_xfer_folder (storage_set, folder_path, new_path, TRUE, rename_cb, callback_data);
-
-	g_free (old_base_path);
-	g_free (new_path);
-	g_free (new_name);
 }