Remove from list: a repository header's menu and Ctrl Shift Backspace in the switcher

crates/dv/src/automation/state.rs

@@ -84,7 +84,7 @@ fn badge(badge: Option<&ReviewBadge>) -> Value {
 /// `state.switcher`.
 pub fn switcher(shell: &Shell, cx: &App) -> Value {
     let Some(switcher) = shell.switcher() else {
-        return json!({"open": false, "mode": null, "query": null, "selected": null, "items": []});
+        return json!({"open": false, "mode": null, "query": null, "selected": null, "items": [], "menu": null});
     };
     let switcher = switcher.read(cx);
     let listing = switcher.listing();
@@ -108,12 +108,19 @@ pub fn switcher(shell: &Shell, cx: &App) -> Value {
             })
         })
         .collect();
+    let menu = switcher.menu().map(|menu| {
+        json!({
+            "repo": menu.location.display_name(),
+            "removable": switcher.can_remove(&menu.location),
+        })
+    });
     json!({
         "open": true,
         "mode": switcher.mode().key(),
         "query": switcher.query(),
         "selected": switcher.selected_key(),
         "items": items,
+        "menu": menu,
     })
 }
 

crates/dv/src/automation/tests.rs

@@ -228,7 +228,7 @@ fn state_describes_the_empty_shell(cx: &mut TestAppContext) {
     assert_eq!(state["backends"], json!([]));
     assert_eq!(
         state["switcher"],
-        json!({"open": false, "mode": null, "query": null, "selected": null, "items": []})
+        json!({"open": false, "mode": null, "query": null, "selected": null, "items": [], "menu": null})
     );
     assert_eq!(
         state["source_picker"],

crates/dv_ui/src/actions.rs

@@ -112,6 +112,8 @@ pub mod switcher {
             SelectPrevious,
             /// Open what the selected row stands for (`Enter`).
             Confirm,
+            /// Remove the selected row's repository from the list (`Ctrl Shift Backspace`).
+            RemoveRepo,
             /// Close without opening anything (`Esc`).
             Dismiss,
         ]

crates/dv_ui/src/keymap.rs

@@ -150,6 +150,14 @@ pub const SHORTCUTS: &[Shortcut] = &[
     shortcut!("down", SWITCHER, switcher::SelectNext, AheadOfInputs),
     shortcut!("up", SWITCHER, switcher::SelectPrevious, AheadOfInputs),
     shortcut!("enter", SWITCHER, switcher::Confirm),
+    // The filter field deletes text with Backspace and its Ctrl, Alt and Shift forms, never
+    // with Ctrl Shift Backspace, which the field would otherwise swallow.
+    shortcut!(
+        "secondary-shift-backspace",
+        SWITCHER,
+        switcher::RemoveRepo,
+        AheadOfInputs
+    ),
     shortcut!("escape", SWITCHER, switcher::Dismiss),
     // The file finder: as the switcher, its filter field has the keyboard.
     shortcut!("down", FILE_FINDER, file_finder::SelectNext, AheadOfInputs),

crates/dv_ui/src/services/reviews.rs

@@ -755,6 +755,22 @@ impl Reviews {
         location
     }
 
+    /// Forgets the known repository at `location` ("Remove from list", DESIGN §4.1): its row
+    /// and any refresh of it go, and `known_repos.json` is saved without it. Nothing in the
+    /// repository changes, its reviews included; opening it again adds it back with them. A
+    /// refresh still running for it finds no row and is dropped. `false` when it wasn't known.
+    pub fn remove_repo(&mut self, location: &Location, cx: &mut Context<Self>) -> bool {
+        if !self.known.remove(location) {
+            return false;
+        }
+        self.rows.remove(location);
+        self.refreshing.remove(location);
+        self.save(cx);
+        cx.emit(ReviewsEvent::Changed);
+        cx.notify();
+        true
+    }
+
     /// A listing or an open of `given` reached the repository whose worktree root is `root`
     /// (`RepoInfo::location`, its distro spelled WSL's way): every known location that names
     /// it otherwise — `given` (a subfolder, another spelling of the distro) and any other

crates/dv_ui/src/services/reviews/tests.rs

@@ -875,3 +875,73 @@ fn a_known_repo_spelled_otherwise_becomes_the_spelling_wsl_resolves(cx: &mut Tes
     });
     assert_eq!(reviews.read_with(cx, |reviews, _| reviews.rows().len()), 1);
 }
+
+#[gpui::test]
+async fn a_removed_repo_leaves_the_list_and_the_file_keeping_its_reviews(cx: &mut TestAppContext) {
+    let data = tempfile::tempdir().unwrap();
+    let repo = fixture();
+    let other = fixture();
+    let (locations, reviews) = services(cx, data.path());
+    locations.update(cx, |locations, _| {
+        locations.register_env(repo.path().to_path_buf(), repo.env_overlay());
+        locations.register_env(other.path().to_path_buf(), other.env_overlay());
+    });
+    let location = Location::Local(repo.path().to_path_buf());
+    let kept = Location::Local(other.path().to_path_buf());
+    for added in [&location, &kept] {
+        reviews
+            .update(cx, |reviews, cx| reviews.add_repo(added.clone(), cx))
+            .await
+            .expect("a repo");
+    }
+    let opened = reviews
+        .update(cx, |reviews, cx| {
+            reviews.open_review(location.clone(), Target::Working, cx)
+        })
+        .await
+        .expect("a review");
+    settle(cx, &reviews);
+
+    // Removed while a refresh of it runs: the row goes at once, and the refresh lands nowhere.
+    let removed = reviews.update(cx, |reviews, cx| {
+        reviews.refresh(location.clone(), cx);
+        reviews.remove_repo(&location, cx)
+    });
+    assert!(removed);
+    settle(cx, &reviews);
+    reviews.read_with(cx, |reviews, _| {
+        assert!(reviews.row(&location).is_none());
+        assert!(reviews.known().get(&location).is_none());
+        assert_eq!(reviews.rows().len(), 1);
+        assert_eq!(reviews.rows()[0].location, kept);
+    });
+    let saved: Value =
+        serde_json::from_str(&std::fs::read_to_string(data.path().join(KNOWN_REPOS_FILE)).unwrap())
+            .unwrap();
+    assert_eq!(
+        saved["repos"].as_array().unwrap().len(),
+        1,
+        "saved without it"
+    );
+    assert_eq!(
+        saved["repos"][0]["location"],
+        serde_json::to_value(&kept).unwrap()
+    );
+    // Not known any more, so nothing to remove.
+    assert!(!reviews.update(cx, |reviews, cx| reviews.remove_repo(&location, cx)));
+
+    // Added again, it lists its review: nothing in the repository was touched.
+    reviews
+        .update(cx, |reviews, cx| reviews.add_repo(location.clone(), cx))
+        .await
+        .expect("added again");
+    settle(cx, &reviews);
+    reviews.read_with(cx, |reviews, _| {
+        let row = reviews.row(&location).expect("listed again");
+        assert_eq!(
+            row.open_review_of(&Target::Working)
+                .map(|summary| &summary.id),
+            Some(&opened.loaded.review.id)
+        );
+    });
+}

crates/dv_ui/src/shell/mod.rs

@@ -841,7 +841,11 @@ impl Shell {
             .session
             .as_ref()
             .map(|session| session.read(cx).review_id().clone());
-        let view = cx.new(|cx| Switcher::new(reviews, focus, current.as_ref(), window, cx));
+        let in_use = self
+            .session
+            .as_ref()
+            .map(|session| session.read(cx).origin().clone());
+        let view = cx.new(|cx| Switcher::new(reviews, focus, current.as_ref(), in_use, window, cx));
         let subscription = cx.subscribe_in(&view, window, Self::on_switcher_event);
         view.update(cx, |switcher, cx| switcher.focus(window, cx));
         self.switcher = Some(Overlay {
@@ -1162,6 +1166,10 @@ impl Shell {
                     .update(cx, |reviews, cx| reviews.start(location.clone(), cx))
                     .detach();
             }
+            SwitcherEvent::Remove { location } => {
+                let reviews = cx.services().reviews.clone();
+                reviews.update(cx, |reviews, cx| reviews.remove_repo(location, cx));
+            }
             SwitcherEvent::Dismiss => {
                 self.close_menu(window, cx);
             }

crates/dv_ui/src/switcher/items.rs

@@ -300,12 +300,26 @@ impl Listing {
         Some((location, request))
     }
 
-    /// The repository of `item`.
+    /// The repository of `item`; `None` for "Open folder…".
     pub fn location_of(&self, item: usize) -> Option<&Location> {
         let item = self.items.get(item)?;
         Some(&self.repos.get(item.repo?)?.location)
     }
 
+    /// The first row of the repository listed after `location`'s, else of the one before it,
+    /// else "Open folder…": where the selection goes when `location` is removed.
+    pub fn row_after_removing(&self, location: &Location) -> Option<&str> {
+        let repo = self
+            .repos
+            .iter()
+            .position(|repo| &repo.location == location)?;
+        let first_of = |repo: usize| self.items.iter().find(|item| item.repo == Some(repo));
+        first_of(repo + 1)
+            .or_else(|| repo.checked_sub(1).and_then(first_of))
+            .or_else(|| self.items.last())
+            .map(|item| item.key.as_str())
+    }
+
     /// The repository picking `item` connects to, when it is a **Start** or **Retry** row
     /// (whose start doesn't run already).
     pub fn start(&self, item: usize) -> Option<Location> {