FileManager: Move delete and force-delete actions to DirectoryView

This is a little bit messy since the left-side treeview also has a delete action. Because of that, we have to put a focus-dependent action that delegates to the relevant view-specific action in the tool bar and menu bar. I'm not sure yet what a good abstraction would be for this. We'll see what we can think of.
2025-05-23 21:55:08 +00:00 · 2020-09-13 21:38:44 +02:00 · 2020-09-13 21:38:44 +02:00 · ef50e5aaee
commit ef50e5aaee
parent 174527b580
5 changed files with 118 additions and 97 deletions
--- a/Applications/FileManager/DirectoryView.cpp
+++ b/Applications/FileManager/DirectoryView.cpp
@ -25,6 +25,7 @@
 */

 #include "DirectoryView.h"
+#include "FileUtils.h"
 #include <AK/LexicalPath.h>
 #include <AK/NumberFormat.h>
 #include <AK/StringBuilder.h>
@ -203,9 +204,7 @@ void DirectoryView::setup_icon_view()
        handle_activation(index);
    };
    m_icon_view->on_selection_change = [this] {
-        update_statusbar();
-        if (on_selection_change)
-            on_selection_change(*m_icon_view);
+        handle_selection_change();
    };
    m_icon_view->on_context_menu_request = [this](auto& index, auto& event) {
        if (on_context_menu_request)
@ -228,9 +227,7 @@ void DirectoryView::setup_columns_view()
    };

    m_columns_view->on_selection_change = [this] {
-        update_statusbar();
-        if (on_selection_change)
-            on_selection_change(*m_columns_view);
+        handle_selection_change();
    };

    m_columns_view->on_context_menu_request = [this](auto& index, auto& event) {
@ -256,9 +253,7 @@ void DirectoryView::setup_table_view()
    };

    m_table_view->on_selection_change = [this] {
-        update_statusbar();
-        if (on_selection_change)
-            on_selection_change(*m_table_view);
+        handle_selection_change();
    };

    m_table_view->on_context_menu_request = [this](auto& index, auto& event) {
@ -439,6 +434,25 @@ Vector<String> DirectoryView::selected_file_paths() const
    return paths;
 }

+void DirectoryView::do_delete(bool should_confirm)
+{
+    auto paths = selected_file_paths();
+    ASSERT(!paths.is_empty());
+    FileUtils::delete_paths(paths, should_confirm, window());
+}
+
+void DirectoryView::handle_selection_change()
+{
+    update_statusbar();
+
+    bool can_delete = !current_view().selection().is_empty() && access(path().characters(), W_OK) == 0;
+    m_delete_action->set_enabled(can_delete);
+    m_force_delete_action->set_enabled(can_delete);
+
+    if (on_selection_change)
+        on_selection_change(*m_table_view);
+}
+
 void DirectoryView::setup_actions()
 {
    m_mkdir_action = GUI::Action::create("New directory...", { Mod_Ctrl | Mod_Shift, Key_N }, Gfx::Bitmap::load_from_file("/res/icons/16x16/mkdir.png"), [&](const GUI::Action&) {
@ -499,4 +513,11 @@ void DirectoryView::setup_actions()
        }
        posix_spawn_file_actions_destroy(&spawn_actions);
    });
+
+    m_delete_action = GUI::CommonActions::make_delete_action([this](auto&) { do_delete(true); }, window());
+
+    m_force_delete_action = GUI::Action::create(
+        "Delete without confirmation", { Mod_Shift, Key_Delete },
+        [this](auto&) { do_delete(false); },
+        window());
 }
--- a/Applications/FileManager/DirectoryView.h
+++ b/Applications/FileManager/DirectoryView.h
@ -137,12 +137,18 @@ public:
    GUI::Action& mkdir_action() { return *m_mkdir_action; }
    GUI::Action& touch_action() { return *m_touch_action; }
    GUI::Action& open_terminal_action() { return *m_open_terminal_action; }
+    GUI::Action& delete_action() { return *m_delete_action; }
+    GUI::Action& force_delete_action() { return *m_force_delete_action; }

 private:
    explicit DirectoryView(Mode);
+
    const GUI::FileSystemModel& model() const { return *m_model; }
    GUI::FileSystemModel& model() { return *m_model; }

+    void handle_selection_change();
+    void do_delete(bool should_confirm);
+
    // ^GUI::ModelClient
    virtual void model_did_update(unsigned) override;

@ -166,6 +172,11 @@ private:
    Vector<String> m_path_history;
    void add_path_to_history(const StringView& path);

+    enum class ConfirmBeforeDelete {
+        No,
+        Yes
+    };
+
    RefPtr<GUI::TableView> m_table_view;
    RefPtr<GUI::IconView> m_icon_view;
    RefPtr<GUI::ColumnsView> m_columns_view;
@ -173,4 +184,6 @@ private:
    RefPtr<GUI::Action> m_mkdir_action;
    RefPtr<GUI::Action> m_touch_action;
    RefPtr<GUI::Action> m_open_terminal_action;
+    RefPtr<GUI::Action> m_delete_action;
+    RefPtr<GUI::Action> m_force_delete_action;
 };
--- a/Applications/FileManager/FileUtils.cpp
+++ b/Applications/FileManager/FileUtils.cpp
@ -30,6 +30,7 @@
 #include <AK/StringBuilder.h>
 #include <LibCore/DirIterator.h>
 #include <LibCore/File.h>
+#include <LibGUI/MessageBox.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/stat.h>
@ -37,6 +38,57 @@

 namespace FileUtils {

+void delete_paths(const Vector<String>& paths, bool should_confirm, GUI::Window* parent_window)
+{
+    String message;
+    if (paths.size() == 1) {
+        message = String::format("Really delete %s?", LexicalPath(paths[0]).basename().characters());
+    } else {
+        message = String::format("Really delete %d files?", paths.size());
+    }
+
+    if (should_confirm) {
+        auto result = GUI::MessageBox::show(parent_window,
+            message,
+            "Confirm deletion",
+            GUI::MessageBox::Type::Warning,
+            GUI::MessageBox::InputType::OKCancel);
+        if (result == GUI::MessageBox::ExecCancel)
+            return;
+    }
+
+    for (auto& path : paths) {
+        struct stat st;
+        if (lstat(path.characters(), &st)) {
+            GUI::MessageBox::show(parent_window,
+                String::format("lstat(%s) failed: %s", path.characters(), strerror(errno)),
+                "Delete failed",
+                GUI::MessageBox::Type::Error);
+            break;
+        }
+
+        if (S_ISDIR(st.st_mode)) {
+            String error_path;
+            int error = FileUtils::delete_directory(path, error_path);
+
+            if (error) {
+                GUI::MessageBox::show(parent_window,
+                    String::format("Failed to delete directory \"%s\": %s", error_path.characters(), strerror(error)),
+                    "Delete failed",
+                    GUI::MessageBox::Type::Error);
+                break;
+            }
+        } else if (unlink(path.characters()) < 0) {
+            int saved_errno = errno;
+            GUI::MessageBox::show(parent_window,
+                String::format("unlink(%s) failed: %s", path.characters(), strerror(saved_errno)),
+                "Delete failed",
+                GUI::MessageBox::Type::Error);
+            break;
+        }
+    }
+}
+
 int delete_directory(String directory, String& file_that_caused_error)
 {
    Core::DirIterator iterator(directory, Core::DirIterator::SkipDots);
--- a/Applications/FileManager/FileUtils.h
+++ b/Applications/FileManager/FileUtils.h
@ -28,10 +28,12 @@

 #include <AK/String.h>
 #include <LibCore/Forward.h>
+#include <LibGUI/Forward.h>
 #include <sys/stat.h>

 namespace FileUtils {

+void delete_paths(const Vector<String>&, bool should_confirm, GUI::Window*);
 int delete_directory(String directory, String& file_that_caused_error);
 bool copy_file_or_directory(const String& src_path, const String& dst_path);
 String get_duplicate_name(const String& path, int duplicate_count);
--- a/Applications/FileManager/main.cpp
+++ b/Applications/FileManager/main.cpp
@ -386,11 +386,6 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
            },
            window);

-    enum class ConfirmBeforeDelete {
-        No,
-        Yes
-    };
-
    auto do_paste = [&](const GUI::Action& action) {
        auto data_and_type = GUI::Clipboard::the().data_and_type();
        if (data_and_type.mime_type != "text/uri-list") {
@ -428,68 +423,6 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
        }
    };

-    auto do_delete = [&](ConfirmBeforeDelete confirm, const GUI::Action&) {
-        auto paths = directory_view.selected_file_paths();
-
-        if (!paths.size())
-            paths = tree_view_selected_file_paths();
-
-        if (paths.is_empty())
-            ASSERT_NOT_REACHED();
-
-        String message;
-        if (paths.size() == 1) {
-            message = String::format("Really delete %s?", LexicalPath(paths[0]).basename().characters());
-        } else {
-            message = String::format("Really delete %d files?", paths.size());
-        }
-
-        if (confirm == ConfirmBeforeDelete::Yes) {
-            auto result = GUI::MessageBox::show(window,
-                message,
-                "Confirm deletion",
-                GUI::MessageBox::Type::Warning,
-                GUI::MessageBox::InputType::OKCancel);
-            if (result == GUI::MessageBox::ExecCancel)
-                return;
-        }
-
-        for (auto& path : paths) {
-            struct stat st;
-            if (lstat(path.characters(), &st)) {
-                GUI::MessageBox::show(window,
-                    String::format("lstat(%s) failed: %s", path.characters(), strerror(errno)),
-                    "Delete failed",
-                    GUI::MessageBox::Type::Error);
-                break;
-            } else {
-                refresh_tree_view();
-            }
-
-            if (S_ISDIR(st.st_mode)) {
-                String error_path;
-                int error = FileUtils::delete_directory(path, error_path);
-
-                if (error) {
-                    GUI::MessageBox::show(window,
-                        String::format("Failed to delete directory \"%s\": %s", error_path.characters(), strerror(error)),
-                        "Delete failed",
-                        GUI::MessageBox::Type::Error);
-                    break;
-                } else {
-                    refresh_tree_view();
-                }
-            } else if (unlink(path.characters()) < 0) {
-                int saved_errno = errno;
-                GUI::MessageBox::show(window,
-                    String::format("unlink(%s) failed: %s", path.characters(), strerror(saved_errno)),
-                    "Delete failed",
-                    GUI::MessageBox::Type::Error);
-                break;
-            }
-        }
-    };
-
    auto paste_action = GUI::CommonActions::make_paste_action(
        [&](const GUI::Action& action) {
            do_paste(action);
@ -502,19 +435,6 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
        },
        window);

-    auto force_delete_action = GUI::Action::create(
-        "Delete without confirmation", { Mod_Shift, Key_Delete }, [&](const GUI::Action& action) {
-            do_delete(ConfirmBeforeDelete::No, action);
-        },
-        window);
-
-    auto delete_action = GUI::CommonActions::make_delete_action(
-        [&](const GUI::Action& action) {
-            do_delete(ConfirmBeforeDelete::Yes, action);
-        },
-        window);
-    delete_action->set_enabled(false);
-
    auto go_back_action = GUI::CommonActions::make_go_back_action(
        [&](auto&) {
            directory_view.open_previous_directory();
@ -538,6 +458,21 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
        paste_action->set_enabled(data_type == "text/uri-list" && access(current_location.characters(), W_OK) == 0);
    };

+    auto tree_view_delete_action = GUI::CommonActions::make_delete_action(
+        [&](auto&) {
+            FileUtils::delete_paths(tree_view_selected_file_paths(), true, window);
+        },
+        &tree_view);
+
+    // This is a little awkward. The menu action does something different depending on which view has focus.
+    // It would be nice to find a good abstraction for this instead of creating a branching action like this.
+    auto focus_dependent_delete_action = GUI::CommonActions::make_delete_action([&](auto&) {
+        if (tree_view.is_focused())
+            tree_view_delete_action->activate();
+        else
+            directory_view.delete_action().activate();
+    });
+
    auto menubar = GUI::MenuBar::construct();

    auto& app_menu = menubar->add_menu("File Manager");
@ -545,7 +480,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
    app_menu.add_action(directory_view.touch_action());
    app_menu.add_action(copy_action);
    app_menu.add_action(paste_action);
-    app_menu.add_action(delete_action);
+    app_menu.add_action(focus_dependent_delete_action);
    app_menu.add_action(directory_view.open_terminal_action());
    app_menu.add_separator();
    app_menu.add_action(properties_action);
@ -591,7 +526,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
    main_toolbar.add_action(directory_view.touch_action());
    main_toolbar.add_action(copy_action);
    main_toolbar.add_action(paste_action);
-    main_toolbar.add_action(delete_action);
+    main_toolbar.add_action(focus_dependent_delete_action);
    main_toolbar.add_action(directory_view.open_terminal_action());

    main_toolbar.add_separator();
@ -655,14 +590,12 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
    directory_view.on_selection_change = [&](GUI::AbstractView& view) {
        // FIXME: Figure out how we can enable/disable the paste action, based on clipboard contents.
        auto& selection = view.selection();
-
-        delete_action->set_enabled(!selection.is_empty() && access(directory_view.path().characters(), W_OK) == 0);
        copy_action->set_enabled(!selection.is_empty());
    };

    directory_context_menu->add_action(copy_action);
    directory_context_menu->add_action(folder_specific_paste_action);
-    directory_context_menu->add_action(delete_action);
+    directory_context_menu->add_action(directory_view.delete_action());
    directory_context_menu->add_separator();
    directory_context_menu->add_action(properties_action);

@ -675,7 +608,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio

    tree_view_directory_context_menu->add_action(copy_action);
    tree_view_directory_context_menu->add_action(paste_action);
-    tree_view_directory_context_menu->add_action(delete_action);
+    tree_view_directory_context_menu->add_action(directory_view.delete_action());
    tree_view_directory_context_menu->add_separator();
    tree_view_directory_context_menu->add_action(properties_action);
    tree_view_directory_context_menu->add_separator();
@ -701,7 +634,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
                file_context_menu = GUI::Menu::construct("Directory View File");
                file_context_menu->add_action(copy_action);
                file_context_menu->add_action(paste_action);
-                file_context_menu->add_action(delete_action);
+                file_context_menu->add_action(directory_view.delete_action());

                file_context_menu->add_separator();
                bool added_open_menu_items = false;
@ -796,7 +729,7 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
            return;
        directory_view.open(path);
        copy_action->set_enabled(!tree_view.selection().is_empty());
-        delete_action->set_enabled(!tree_view.selection().is_empty());
+        directory_view.delete_action().set_enabled(!tree_view.selection().is_empty());
    };

    tree_view.on_context_menu_request = [&](const GUI::ModelIndex& index, const GUI::ContextMenuEvent& event) {