Merge pull request #652 from vangel76/fix-456-project-select-all

#456: Select All in the Project panel takes only the shown bin

crates/engine/src/project_panel.rs

@@ -17,9 +17,9 @@
 //! | `project.freeform.saveArrangement` / `.restoreArrangement` / `.arrangements` / `.deleteArrangement` | saved arrangements |
 //! | `project.freeform.options` | Freeform View Options… (grid, snap, names, durations) |
 //! | `project.renameBin` | rename a bin (inline rename, undoable) |
-//! | `project.selectAll` / `project.deselectAll` | select every item in the project / clear the Project panel selection (Cmd+A / Cmd+Shift+A with the panel focused) |
+//! | `project.selectAll` / `project.deselectAll` | select the items a bin shows (`bin`, plus the sub-bins open in List view in `expanded`; without `bin` every item in the project) / clear the Project panel selection (Cmd+A / Cmd+Shift+A with the panel focused) |
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use filmcraft_project::{Bin, BinEntry, BinId, ItemId, ItemKind, MediaRef, Project, ProjectItem, TrackKind};
 use filmcraft_time::{Tick, TimeDisplay, format_time};
@@ -1171,6 +1171,43 @@ fn rename_bin(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({"bin": bin.0, "name": name}))
 }
 
+/// Select All in the Project panel. With `bin`, only what a panel showing that bin lists: its
+/// items, plus the items of the sub-bins named in `expanded` (bins twirled open in List view),
+/// never those of a closed bin. Without `bin`, every item in the project.
+fn select_all(s: &mut Session, p: &Value) -> Result<Value> {
+    const CMD: &str = "project.selectAll";
+    // strict: a negative or fractional `bin` is an error, not bin 0
+    let id = match p.get("bin") {
+        None | Some(Value::Null) => {
+            s.state.project_selection = s.project.items.keys().copied().collect();
+            return Ok(json!({"selected": s.state.project_selection.len()}));
+        }
+        Some(b) => BinId(b.as_u64().ok_or_else(|| bad(CMD, "`bin` must be a bin id"))?),
+    };
+    let bin = s.project.root.find_bin(id).ok_or_else(|| bad(CMD, format!("no bin {}", id.0)))?;
+    let expanded: BTreeSet<BinId> = match p.get("expanded") {
+        None | Some(Value::Null) => Default::default(),
+        Some(Value::Array(a)) => {
+            a.iter().map(|v| v.as_u64().map(BinId).ok_or_else(|| bad(CMD, "`expanded` must be a list of bin ids"))).collect::<Result<_>>()?
+        }
+        Some(_) => return Err(bad(CMD, "`expanded` must be a list of bin ids")),
+    };
+    // iterative walk: a hostile project file cannot overflow the stack with deeply nested bins
+    let mut sel = Vec::new();
+    let mut stack = vec![bin];
+    while let Some(b) = stack.pop() {
+        for e in &b.children {
+            match e {
+                BinEntry::Item(i) if s.project.item(*i).is_some_and(|it| listed(&s.project, it)) => sel.push(*i),
+                BinEntry::Bin(sub) if expanded.contains(&sub.id) => stack.push(sub),
+                _ => {}
+            }
+        }
+    }
+    s.state.project_selection = sel;
+    Ok(json!({"bin": id.0, "selected": s.state.project_selection.len()}))
+}
+
 // ------------------------------------------------------------------------------------ registry
 
 pub(crate) fn commands() -> Vec<CommandSpec> {
@@ -1223,10 +1260,7 @@ pub(crate) fn commands() -> Vec<CommandSpec> {
             freeform_options,
         ),
         c("project.renameBin", "Rename Bin", r#"{"bin":binId,"name":str}"#, always, rename_bin),
-        c("project.selectAll", "Select All Project Items", "{}", always, |s, _| {
-            s.state.project_selection = s.project.items.keys().copied().collect();
-            Ok(json!({"selected": s.state.project_selection.len()}))
-        }),
+        c("project.selectAll", "Select All Project Items", r#"{"bin":binId?,"expanded":[binId]?}"#, always, select_all),
         c("project.deselectAll", "Deselect All Project Items", "{}", always, |s, _| {
             s.state.project_selection.clear();
             Ok(Value::Null)

crates/engine/src/project_panel_tests.rs

@@ -2,7 +2,7 @@
 //! sorting, view presets, Freeform positions / stacks / arrangements (undoable, saved with the
 //! project) and bin renames.
 
-use filmcraft_project::{ItemId, ItemKind};
+use filmcraft_project::{BinEntry, ItemId, ItemKind};
 use serde_json::{Value, json};
 
 use crate::Session;
@@ -305,3 +305,75 @@ fn select_all_selects_every_project_item() {
     assert!(!s.state.selection.is_empty());
     assert_eq!(s.state.project_selection, before);
 }
+
+#[test]
+fn select_all_in_a_bin_skips_closed_bins_and_other_bins() {
+    // #456: Cmd+A in the Project panel selected every item in the project, closed bins included
+    let mut s = demo();
+    let outer = s.execute("file.newBin", json!({"name": "Footage A"})).unwrap()["bin"].as_u64().unwrap();
+    let inner = s.execute("file.newBin", json!({"name": "Day 1", "parent": outer})).unwrap()["bin"].as_u64().unwrap();
+    let ids = media(&s);
+    assert!(ids.len() >= 2);
+    s.execute("project.moveToBin", json!({"items": [ids[0].0], "bin": outer})).unwrap();
+    s.execute("project.moveToBin", json!({"items": [ids[1].0], "bin": inner})).unwrap();
+    s.state.project_selection.clear();
+
+    // only the bin's own items: the closed sub-bin's item stays unselected
+    let r = s.execute("project.selectAll", json!({"bin": outer})).unwrap();
+    assert_eq!(r["selected"].as_u64(), Some(1));
+    assert_eq!(s.state.project_selection, vec![ids[0]]);
+
+    // a sub-bin twirled open in List view counts
+    s.execute("project.selectAll", json!({"bin": outer, "expanded": [inner]})).unwrap();
+    let mut sel = s.state.project_selection.clone();
+    sel.sort();
+    assert_eq!(sel, vec![ids[0], ids[1]]);
+
+    // the root: its direct items only, nothing from the (closed) bins
+    let root = s.project.root.id;
+    let mut want: Vec<ItemId> = s
+        .project
+        .root
+        .children
+        .iter()
+        .filter_map(|e| if let BinEntry::Item(i) = e { Some(*i) } else { None })
+        .filter(|i| s.project.item(*i).is_some_and(|it| project_panel::listed(&s.project, it)))
+        .collect();
+    want.sort();
+    s.execute("project.selectAll", json!({"bin": root.0})).unwrap();
+    let mut sel = s.state.project_selection.clone();
+    sel.sort();
+    assert_eq!(sel, want);
+    assert!(!sel.contains(&ids[0]) && !sel.contains(&ids[1]));
+    assert!(sel.len() < s.project.items.len());
+
+    // an open bin inside a closed one is not shown, so it does not count either
+    s.execute("project.selectAll", json!({"bin": root.0, "expanded": [inner]})).unwrap();
+    assert!(!s.state.project_selection.contains(&ids[1]));
+    // ... but open all the way down, it does
+    s.execute("project.selectAll", json!({"bin": root.0, "expanded": [outer, inner]})).unwrap();
+    assert!(s.state.project_selection.contains(&ids[0]) && s.state.project_selection.contains(&ids[1]));
+}
+
+#[test]
+fn select_all_rejects_bad_bins_without_touching_the_selection() {
+    let mut s = demo();
+    let ids = media(&s);
+    let root = s.project.root.id.0;
+    s.execute("project.select", json!({"items": [ids[0].0]})).unwrap();
+    for p in [
+        json!({"bin": 987_654_321u64}),
+        json!({"bin": "footage"}),
+        json!({"bin": -1}),
+        json!({"bin": root, "expanded": "all"}),
+        json!({"bin": root, "expanded": ["x"]}),
+    ] {
+        assert!(s.execute("project.selectAll", p.clone()).is_err(), "{p}");
+        assert_eq!(s.state.project_selection, vec![ids[0]], "{p}");
+    }
+    // unknown ids in `expanded` are simply not open bins
+    assert!(s.execute("project.selectAll", json!({"bin": root, "expanded": [987_654_321u64]})).is_ok());
+    // `bin: null` is the same as no bin: the whole project
+    let r = s.execute("project.selectAll", json!({"bin": null})).unwrap();
+    assert_eq!(r["selected"].as_u64(), Some(s.project.items.len() as u64));
+}

crates/ui-egui/src/lib.rs

@@ -1361,8 +1361,16 @@ impl FilmcraftApp {
         });
         for mut id in fire {
             // Select All / Deselect All act on the Project panel's items when it has focus (#168).
-            if self.ui.focused == PanelKind::Project && matches!(id.as_str(), "edit.selectAll" | "edit.deselectAll") {
-                id = id.replacen("edit.", "project.", 1);
+            // Select All takes only what the shown bin lists, not the whole project (#456).
+            if self.ui.focused == PanelKind::Project && id == "edit.selectAll" {
+                let (cmd, params) = panels::project::select_all_command(self);
+                if let Err(e) = menus::invoke(self, ctx, &cmd, params) {
+                    self.ui.status = e;
+                }
+                continue;
+            }
+            if self.ui.focused == PanelKind::Project && id == "edit.deselectAll" {
+                id = "project.deselectAll".into();
             }
             // Clear in the Project panel removes the selected bin when no item is selected (#587).
             let clear = if self.ui.focused == PanelKind::Project && id == "project.delete" {

crates/ui-egui/src/panels/project.rs

@@ -254,6 +254,21 @@ pub fn visible_items(app: &FilmcraftApp, inst: Inst) -> Vec<ItemId> {
     crate::panels::project_views::items_in_view(app, &v, &app.ui.project_search.to_ascii_lowercase())
 }
 
+/// The command Select All (Cmd+A) runs with the Project panel focused: the items of the shown
+/// bin, plus those of sub-bins twirled open in List view (`project.selectAll {bin, expanded}`).
+/// Items in closed bins and in bins of other tabs stay unselected, as in Premiere (#456). While a
+/// search filters the panel, exactly the rows it shows (`project.select {items}`).
+pub fn select_all_command(app: &FilmcraftApp) -> (String, Value) {
+    let inst = shown_inst(app);
+    if !app.ui.project_search.is_empty() {
+        let items: Vec<u64> = visible_items(app, inst).into_iter().map(|i| i.0).collect();
+        return ("project.select".into(), json!({"items": items}));
+    }
+    let v = view_of(app, inst);
+    let expanded: &[u64] = if v.mode == ViewMode::List { &app.ui.expanded_bins } else { &[] };
+    ("project.selectAll".into(), json!({"bin": v.bin.0, "expanded": expanded}))
+}
+
 // ------------------------------------------------------------------------------------ panel
 
 pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {

crates/ui-egui/tests/project_panel_ui.rs

@@ -447,6 +447,44 @@ fn bins_open_in_place_in_a_tab_or_in_a_window() {
     assert_eq!(name, "Camera A");
 }
 
+#[test]
+fn select_all_takes_only_the_shown_bin() {
+    // #456: Cmd+A selected every item in the project, closed bins and other tabs included
+    let mut d = project_driver();
+    d.exec("project.view.set", json!({"view": "list"}));
+    let (bin, items) = d.footage();
+    assert!(!items.is_empty());
+    let sorted = |v: &[ItemId]| {
+        let mut v: Vec<u64> = v.iter().map(|i| i.0).collect();
+        v.sort();
+        v
+    };
+    let mut footage = items.clone();
+    footage.sort();
+    // the root with the Footage bin closed: none of its items
+    d.app().ui.expanded_bins.clear();
+    d.frames(2);
+    d.key("Cmd+A");
+    let sel = d.app().session.state.project_selection.clone();
+    assert!(sel.iter().all(|i| !items.contains(&i.0)), "{sel:?}");
+    assert!(sel.iter().all(|i| d.app().session.project.root.children.contains(&BinEntry::Item(*i))), "{sel:?}");
+    // twirled open in List view, its items are shown and count
+    d.app().ui.expanded_bins.push(bin);
+    d.frames(2);
+    d.key("Cmd+A");
+    let sel = d.app().session.state.project_selection.clone();
+    assert!(items.iter().all(|i| sel.contains(&ItemId(*i))), "{sel:?}");
+    d.key("Cmd+Shift+A");
+    assert!(d.app().session.state.project_selection.is_empty());
+    // the bin opened in its own tab: exactly its items
+    d.app().ui.expanded_bins.clear();
+    d.app().session.prefs.general.bins_double_click = "openNewTab".into();
+    double_click_bin(&mut d, bin, false);
+    assert_eq!(d.app().ui.project_panel.active_tab, Some(0));
+    d.key("Cmd+A");
+    assert_eq!(sorted(&d.app().session.state.project_selection), footage);
+}
+
 #[test]
 fn footer_buttons_are_wired() {
     let mut d = project_driver();