From f5e63f785a7c41606923fb02cf8df83cbe59fe3f Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Thu, 24 Jun 2021 15:43:05 +0100 Subject: [PATCH] FileManager: Remove clicked breadcrumbs for non-existing directories This fixes #8204. In the case that we just navigated up from a directory because it was deleted, we can detect that easily by checking if the child directory exists, and then remove the relevant breadcrumbs immediately. However, it's harder to notice if a child directory for a breadcrumb is deleted at another time. Previously, clicking that breadcrumb would crash, but now we check to see if the directory it points to actually exists. If it doesn't, we pop that breadcrumb and any after it, off of the breadcrumbbar. This may not be the ideal solution - maybe it should detect that the directory is gone automatically - but it works and doesn't involve managing additional directory watchers. --- Userland/Applications/FileManager/main.cpp | 29 +++++++++++++------ Userland/Libraries/LibGUI/FileSystemModel.cpp | 8 ++--- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/Userland/Applications/FileManager/main.cpp b/Userland/Applications/FileManager/main.cpp index 3133a647bc5..c9a671bb71d 100644 --- a/Userland/Applications/FileManager/main.cpp +++ b/Userland/Applications/FileManager/main.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2018-2021, Andreas Kling + * Copyright (c) 2021, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -922,16 +923,18 @@ int run_in_windowed_mode(RefPtr config, String initial_locatio { LexicalPath lexical_path(new_path); - auto segment_index_of_new_path_in_breadcrumbbar = [&]() -> Optional { - for (size_t i = 0; i < breadcrumbbar.segment_count(); ++i) { - if (breadcrumbbar.segment_data(i) == new_path) - return i; - } - return {}; - }(); + auto segment_index_of_new_path_in_breadcrumbbar = breadcrumbbar.find_segment_with_data(new_path); if (segment_index_of_new_path_in_breadcrumbbar.has_value()) { - breadcrumbbar.set_selected_segment(segment_index_of_new_path_in_breadcrumbbar.value()); + auto new_segment_index = segment_index_of_new_path_in_breadcrumbbar.value(); + breadcrumbbar.set_selected_segment(new_segment_index); + + // If the path change was because the directory we were in was deleted, + // remove the breadcrumbs for it. + if ((new_segment_index + 1 < breadcrumbbar.segment_count()) + && !Core::File::is_directory(breadcrumbbar.segment_data(new_segment_index + 1))) { + breadcrumbbar.remove_end_segments(new_segment_index + 1); + } } else { breadcrumbbar.clear_segments(); @@ -949,7 +952,15 @@ int run_in_windowed_mode(RefPtr config, String initial_locatio breadcrumbbar.set_selected_segment(breadcrumbbar.segment_count() - 1); breadcrumbbar.on_segment_click = [&](size_t segment_index) { - directory_view.open(breadcrumbbar.segment_data(segment_index)); + auto selected_path = breadcrumbbar.segment_data(segment_index); + if (Core::File::is_directory(selected_path)) { + directory_view.open(selected_path); + } else { + dbgln("Breadcrumb path '{}' doesn't exist", selected_path); + breadcrumbbar.remove_end_segments(segment_index); + auto existing_path_segment = breadcrumbbar.find_segment_with_data(directory_view.path()); + breadcrumbbar.set_selected_segment(existing_path_segment.value()); + } }; } } diff --git a/Userland/Libraries/LibGUI/FileSystemModel.cpp b/Userland/Libraries/LibGUI/FileSystemModel.cpp index 160a090ec4e..af465390596 100644 --- a/Userland/Libraries/LibGUI/FileSystemModel.cpp +++ b/Userland/Libraries/LibGUI/FileSystemModel.cpp @@ -272,11 +272,11 @@ FileSystemModel::FileSystemModel(String root_path, Mode mode) auto canonical_event_path = LexicalPath::canonicalized_path(event.event_path); if (m_root_path.starts_with(canonical_event_path)) { // Deleted directory contains our root, so navigate to our nearest parent. - auto new_path = LexicalPath(m_root_path).dirname(); - while (!Core::File::is_directory(new_path)) - new_path = LexicalPath(new_path).dirname(); + auto new_path = LexicalPath(m_root_path).parent(); + while (!Core::File::is_directory(new_path.string())) + new_path = new_path.parent(); - set_root_path(new_path); + set_root_path(new_path.string()); } else if (node.parent) { refresh_node(*node.parent); }