GNOME Bugzilla – Bug 132326
Delete command that bypasses trash does not show up on Tree View items
Last modified: 2006-11-28 10:04:24 UTC
"Delete command that bypasses Trash" does show up in the context menu of the file views, but does not show in the context menu of items in the treeview. Long Description: 1) Open the preference dialog of nautilus, go to tab "Behavior" and enable "Include delete command that bypasses Trash" 2) Right click on an item in the treeview, e.g. your home folder Expected Result: the delete command in the context menu is visible Real Result : the delete command is not visible
Note: The bug report is about the lack of a delete command in the tree view in the side pane. On that note, I would like to say that I do not think that including a delete command would be a good idea, simple because it would enable even easier deletion of entire directories.
Yes, the delete command is dangerous, but therefore it is optional: the user is completely responsible. That being said, the delete command does appear on directories in the icon and list views. So it should appear on the treeview too.
Still valid for 2.6.0.
Really not sure why this would be 'high'; it is sort of irritating but not that huge a deal. see- http://bugzilla.gnome.org/bug_status.html#priority High Seriously broken, but not as high impact. Should be fixed before next major release. Frequently includes cosmetic bugs of particularly high visibility, regressions from functionality provided in previous releases, and more minor bugs that are frequently reported.
Created attachment 38793 [details] [review] This patch fixes the bug.. it adds Delete item in popup menu and performs the neccessary actions. The patch is tested and works well
Created attachment 38832 [details] [review] This is a modified patch (devoid of unncessary blank spaces unlike the previous patch). The patch after deleting the folder moves the selection to parent directory that contained the deleted folder
/* to add delete */ In create_popup_menu(), Create the menu_image and later show it, + gtk_widget_show (menu_image); + menu_item = gtk_image_menu_item_new_with_label (_("Delete")); Here set image for menu item. + g_signal_connect (menu_item, "activate", + G_CALLBACK (fm_tree_view_delete_cb), In fm_tree_view_delete_cb(), Can you just use nautilus_file_operations_delete() directly and move ahead. Look at fm_tree_view_trash_cb() for reference.... In confirm_delete_directly(), Add g_assert (FM_IS_TREE_VIEW (view)); Please take care of spacings :) Rest looks ok to me.
Created attachment 38897 [details] [review] This is a slightly modified patch of the previous one. I have made the changes according to previous comment and with proper identation. Thanks to Kaushal and Vijay
+ 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?
Nirmal: Maybe you could give it another try? Would be nice to have this fixed.
Nirmal, are you still willing to do some coding? :)
Nirmal?
Updating version and milestoning to 2.14. http://mail.gnome.org/archives/nautilus-list/2006-January/msg00021.html has a patch by Ferran Puig.
Created attachment 56835 [details] [review] Patch with suggestions from Christian Neumair (incomplete) Patch from http://mail.gnome.org/archives/nautilus-list/2006-January/msg00021.html with changes suggested by Christian Neumair, but still incomplete.
"Trying to detect if the file is a special link doesn't work, although I've tried using the same macro that the other views use (NAUTILUS_IS_DESKTOP_ICON_FILE)." NautilusDesktopIconFile-s usually live on x-nautilus-desktop:/// ("Home", "Computer", "Trash", etc.). I don't think they'll ever appear in the sidebar. However, I think you should keep the code in for the sake of consistency. + /* No parent directory, return FALSE */ + if (parent == NULL) { + return TRUE; + } looks suspicious. Please synchronize the comment and the retval. It may well be the reason for "(...) trying to detect if the parent file is writable doesn't work... I don't know why, but I get results like / is writable...". Other than that, it looks really nice, except for an unneccessary + if (confirm_delete_directly (view, (char *)directory_uri)) { (char *) cast and the fact that the second argument to confirm_delete_directly should be const char *. Sorry for not having pointed out these two nit-picks earlier.
Created attachment 75584 [details] [review] Improved patch with suggestions from comment #16 Improved patch with suggestions on comment #16. Also changed the confirmation dialog to be the same which gets shown in fm-directory-view.c
Commited to HEAD. Thanks.