From 067ec9f46f07c6f192f5d3120abb434d6bc838ff Mon Sep 17 00:00:00 2001 From: gnacho Date: Mon, 24 Aug 2026 00:21:22 +0200 Subject: [PATCH] feat(ui): group the mass-deletion review by top-level folder (closes #182) The review dialog listed up to 100 individual missing paths in a flat list. For a large cleanup (removed SDK, virtualenv, build caches) that wall of paths is unreadable in a small window. Missing paths are now grouped by their first path component: one expandable row per directory with its file count (folder icon, count subtitle), expanded children capped, and small groups plus loose files listed individually. Rows are sorted by group size. The grouping lives in a pure, unit-tested function. --- po/es.po | 8 ++ src/core/delete_guard.rs | 156 ++++++++++++++++++++++++++++++++++++ src/ui/main_window.rs | 77 +++++++++++++++--- src/util/translations/es.rs | 2 + 4 files changed, 231 insertions(+), 12 deletions(-) diff --git a/po/es.po b/po/es.po index 6457911..4a44e18 100644 --- a/po/es.po +++ b/po/es.po @@ -2630,6 +2630,14 @@ msgstr "{action} · {count} archivos" msgid "{count} files disappeared from the local folder." msgstr "{count} archivos desaparecieron de la carpeta local." +#: src/ui/main_window.rs +msgid "{count} files" +msgstr "{count} archivos" + +#: src/ui/main_window.rs +msgid "{count} more…" +msgstr "{count} más…" + #: src/ui/main_window.rs msgid "Showing {count} of {total} files." msgstr "Mostrando {count} de {total} archivos." diff --git a/src/core/delete_guard.rs b/src/core/delete_guard.rs index dee1504..6d09b40 100644 --- a/src/core/delete_guard.rs +++ b/src/core/delete_guard.rs @@ -391,11 +391,167 @@ fn absolute_root(path: &Path) -> PathBuf { std::path::absolute(&expanded).unwrap_or(expanded) } +/// Groups of missing paths for the deletion-review dialog (issue #182). +/// +/// A mass deletion (a removed vendored SDK, a virtualenv, build caches) +/// disappears as thousands of paths under a handful of top-level +/// directories; a flat list of individual paths is unreadable in a small +/// dialog. Grouping by the first path component surfaces *what* is being +/// deleted at a glance. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DeletionReviewRow { + /// A top-level directory with its missing-path count and the paths + /// themselves (the UI caps how many it expands). + Group { + /// First path component shared by every path in the group. + prefix: String, + /// Missing paths under the prefix. + count: usize, + /// The grouped paths, sorted. + paths: Vec, + }, + /// A single missing path shown on its own (loose root files and + /// directories too small to be worth a group). + File(String), +} + +/// Groups with at most this many paths are flattened to individual rows +/// instead of getting a group row (issue #182). +pub const DELETION_GROUP_MIN: usize = 5; + +/// Build the row model for the deletion-review dialog (issue #182): +/// one [`DeletionReviewRow::Group`] per top-level directory with more than +/// [`DELETION_GROUP_MIN`] missing paths, individual +/// [`DeletionReviewRow::File`] rows for everything else. Groups come first, +/// largest first (ties by name); files last, sorted. +pub fn deletion_review_rows(paths: &[String]) -> Vec { + let mut groups: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + let mut loose: Vec = Vec::new(); + for path in paths { + match path.split_once('/') { + Some((prefix, _)) if !prefix.is_empty() => groups + .entry(prefix.to_string()) + .or_default() + .push(path.clone()), + _ => loose.push(path.clone()), + } + } + let mut grouped: Vec = Vec::new(); + let mut small: Vec = Vec::new(); + for (prefix, mut members) in groups { + members.sort(); + if members.len() > DELETION_GROUP_MIN { + grouped.push(DeletionReviewRow::Group { + prefix, + count: members.len(), + paths: members, + }); + } else { + small.extend(members); + } + } + grouped.sort_by(|a, b| { + let ( + DeletionReviewRow::Group { + prefix: pa, + count: ca, + .. + }, + DeletionReviewRow::Group { + prefix: pb, + count: cb, + .. + }, + ) = (a, b) + else { + unreachable!("only groups are collected here"); + }; + cb.cmp(ca).then_with(|| pa.cmp(pb)) + }); + small.extend(loose); + small.sort(); + grouped.extend(small.into_iter().map(DeletionReviewRow::File)); + grouped +} + #[cfg(test)] mod tests { use super::*; use tempfile::{tempdir, TempDir}; + fn paths_of(row: &DeletionReviewRow) -> usize { + match row { + DeletionReviewRow::Group { count, .. } => *count, + DeletionReviewRow::File(_) => 1, + } + } + + #[test] + fn deletion_review_groups_by_top_level_directory() { + let mut paths: Vec = (0..10).map(|i| format!("sdk/src/file{i}.go")).collect(); + paths.extend((0..3).map(|i| format!("cache/build/obj{i}.o"))); + paths.push("notes.md".to_string()); + paths.push("cache/loose.txt".to_string()); + let rows = deletion_review_rows(&paths); + // sdk/ has 10 paths -> group; cache/ has 4 -> flattened; loose file. + let groups: Vec<&DeletionReviewRow> = rows + .iter() + .filter(|row| matches!(row, DeletionReviewRow::Group { .. })) + .collect(); + assert_eq!(groups.len(), 1); + let DeletionReviewRow::Group { + prefix, + count, + paths, + } = groups[0] + else { + unreachable!(); + }; + assert_eq!(prefix, "sdk"); + assert_eq!(count, &10); + assert!(paths.iter().all(|path| path.starts_with("sdk/"))); + // Small group and the loose file appear as individual rows, sorted. + let files: Vec<&String> = rows + .iter() + .filter_map(|row| match row { + DeletionReviewRow::File(path) => Some(path), + _ => None, + }) + .collect(); + assert_eq!(files.len(), 5); + assert!(files.contains(&&"notes.md".to_string())); + assert!(files.contains(&&"cache/loose.txt".to_string())); + // Totals are preserved. + assert_eq!(rows.iter().map(paths_of).sum::(), 15); + } + + #[test] + fn deletion_review_sorts_groups_by_count_then_name() { + let mut paths: Vec = (0..9).map(|i| format!("bbb/{i}")).collect(); + paths.extend((0..20).map(|i| format!("aaa/{i}"))); + paths.extend((0..9).map(|i| format!("ccc/{i}"))); + let rows = deletion_review_rows(&paths); + let prefixes: Vec<&str> = rows + .iter() + .filter_map(|row| match row { + DeletionReviewRow::Group { prefix, .. } => Some(prefix.as_str()), + _ => None, + }) + .collect(); + assert_eq!(prefixes, vec!["aaa", "bbb", "ccc"]); + } + + #[test] + fn deletion_review_handles_empty_and_deep_paths() { + assert!(deletion_review_rows(&[]).is_empty()); + let rows = deletion_review_rows(&["a/b/c/d/e.txt".to_string()]); + assert_eq!( + rows, + vec![DeletionReviewRow::File("a/b/c/d/e.txt".to_string())] + ); + } + fn root() -> TempDir { tempdir().expect("tempdir works") } diff --git a/src/ui/main_window.rs b/src/ui/main_window.rs index 2ecf602..85ac9d2 100644 --- a/src/ui/main_window.rs +++ b/src/ui/main_window.rs @@ -1278,24 +1278,77 @@ impl MainWindow { // List the files the guard detected as missing (issue #115), capped so // a very large deletion stays readable; the count above stays exact. + // Issue #182: paths are grouped by their top-level directory so a + // mass cleanup (a removed SDK, virtualenv or build cache) shows a + // handful of expandable groups instead of a wall of paths. if !missing.is_empty() { - const CAP: usize = 100; + const GROUP_ROW_CAP: usize = 100; + const GROUP_CHILD_CAP: usize = 25; + const TOTAL_CHILD_CAP: usize = 200; let list = gtk4::ListBox::builder() .css_classes(["boxed-list"]) .selection_mode(gtk4::SelectionMode::None) .build(); - for path in missing.iter().take(CAP) { - let row = libadwaita::ActionRow::builder() - .title(path) - .activatable(false) - .selectable(false) - .build(); - list.append(&row); + let review_rows = crate::core::delete_guard::deletion_review_rows(&missing); + let mut shown_rows = 0usize; + let mut shown_children = 0usize; + let mut truncated = false; + for review_row in &review_rows { + if shown_rows >= GROUP_ROW_CAP { + truncated = true; + break; + } + match review_row { + crate::core::delete_guard::DeletionReviewRow::Group { + prefix, + count, + paths, + } => { + let group_row = libadwaita::ExpanderRow::builder() + .title(prefix) + .subtitle(t("{count} files").replace("{count}", &count.to_string())) + .build(); + group_row.add_prefix(>k4::Image::from_icon_name("folder-symbolic")); + for path in paths.iter().take(GROUP_CHILD_CAP) { + if shown_children >= TOTAL_CHILD_CAP { + truncated = true; + break; + } + let child = libadwaita::ActionRow::builder() + .title(path) + .activatable(false) + .selectable(false) + .build(); + group_row.add_row(&child); + shown_children += 1; + } + if paths.len() > GROUP_CHILD_CAP { + let more = libadwaita::ActionRow::builder() + .title(t("{count} more…").replace( + "{count}", + &(paths.len() - GROUP_CHILD_CAP).to_string(), + )) + .activatable(false) + .selectable(false) + .build(); + group_row.add_row(&more); + } + list.append(&group_row); + shown_rows += 1; + } + crate::core::delete_guard::DeletionReviewRow::File(path) => { + let row = libadwaita::ActionRow::builder() + .title(path) + .activatable(false) + .selectable(false) + .build(); + list.append(&row); + shown_rows += 1; + } + } } - let note = if missing_len > CAP { - t("Showing {count} of {total} files.") - .replace("{count}", &CAP.to_string()) - .replace("{total}", &missing_len.to_string()) + let note = if truncated { + t("{count} more…").replace("{count}", &(review_rows.len() - shown_rows).to_string()) } else { t("These deletions will be propagated to the server when it synchronizes.") .to_string() diff --git a/src/util/translations/es.rs b/src/util/translations/es.rs index 2502471..a4b22ca 100644 --- a/src/util/translations/es.rs +++ b/src/util/translations/es.rs @@ -499,7 +499,9 @@ pub static CATALOG: &[(&str, &str)] = &[ ("{action} · {count} files", "{action} · {count} archivos"), ("{count} changes in this release", "{count} cambios en esta versión"), ("{count} conflicted copy(ies) found in {folder}.", "Se encontraron {count} copia(s) en conflicto en {folder}."), + ("{count} files", "{count} archivos"), ("{count} files disappeared from the local folder.", "{count} archivos desaparecieron de la carpeta local."), + ("{count} more…", "{count} más…"), ("{count} of {total} items were restored from the server trash.", "Se restauraron {count} de {total} elementos de la papelera del servidor."), ("{size} local", "{size} en local"), ("{used} used", "{used} usados"),