Re: Bug No:132326

[Alexander Larsson <alexl redhat com>]

On Tue, 2005-03-22 at 18:45 +0530, nirmal r wrote:
> Hi Everyone!!
> I have added a patch for the bug 132326..
> Please review the bug..
> http://bugzilla.gnome.org/show_bug.cgi?id=132326


+		 eel_preferences_add_auto_boolean (NAUTILUS_PREFERENCES_ENABLE_DELETE,
+                                                  &show_delete_command_auto_value);

This is called on each event, leading to many watches being added on
the gconf setting. It needs to be called only once.

 		gtk_widget_set_sensitive (view->details->popup_trash, can_move_uri_to_trash (uri));
+		gtk_widget_set_sensitive (view->details->popup_delete, show_delete_command_auto_value);

This is wrong. You set delete sensitive even if move to trash isn't!

+GtkWindow *
+fm_tree_view_get_containing_window (FMTreeView *view);
+

Is this a global function? If so, it should go in a header. If not it
should be static.

+	/* Just Say Yes if the preference says not to confirm.  */
+	if (!show_delete_command_auto_value) {
+		return TRUE;
+	}
+		file_name = nautilus_file_get_display_name (view->details->popup_file);
+		prompt = g_strdup_printf (_("Are you sure you want to permanently delete \"%s\"?"), 
+					  file_name);
+		g_free (file_name);


The if is totally wrong. It never lets you confirm is you set the
"display a separate delete command in the menu" preference.

Why does this function take a list of uris, and then only look at
view->details->popup_file? 

Also, the indentation here looks strange. There are several similarly
weird indentations in the patch.

+	selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (view->details->tree_widget));
+	gtk_tree_selection_get_selected (selection, &model , &iter);
+	path = gtk_tree_model_get_path (model, &iter);
+	gtk_tree_path_up (path);
+	g_signal_handlers_block_by_func (G_OBJECT(view), G_CALLBACK (selection_changed_callback),NULL);
+	gtk_tree_model_get_iter (model, &iter, path);
+	cancel_activation (view);
+	view->details->activation_file = sort_model_iter_to_file (view, &iter);
+
+        	if (view->details->activation_file == NULL) {
+                	return ;
+        	}
+
+	view->details->activation_in_new_window = FALSE;
+	attributes = NAUTILUS_FILE_ATTRIBUTE_ACTIVATION_URI;
+	nautilus_file_call_when_ready (view->details->activation_file, attributes, got_activation_uri_callback, view);
+	nautilus_file_operations_delete (list, GTK_WIDGET (view));
+	g_signal_handlers_unblock_by_func (G_OBJECT(view), G_CALLBACK (selection_changed_callback), NULL);

Why is all this complexity required? We do none of this in the move to
trash case. It looks extremely complex and fragile. What are you
trying to acomplish?



=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a fast talking flyboy boxer on the run. She's a beautiful tempestuous 
college professor with her own daytime radio talk show. They fight crime!