Skip to content

Commit a137982

Browse files
authored
Merge pull request #2 from Method-es/feat/remove-known-repo
Remove from list: a repository header's menu and Ctrl Shift Backspace in the switcher
2 parents 9edec64 + 2de41a4 commit a137982

16 files changed

Lines changed: 585 additions & 284 deletions

File tree

‎crates/dv/src/automation/state.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ fn badge(badge: Option<&ReviewBadge>) -> Value {
8686
/// `state.switcher`.
8787
pub fn switcher(shell: &Shell, cx: &App) -> Value {
8888
let Some(switcher) = shell.switcher() else {
89-
return json!({"open": false, "mode": null, "query": null, "selected": null, "items": []});
89+
return json!({"open": false, "mode": null, "query": null, "selected": null, "items": [], "menu": null});
9090
};
9191
let switcher = switcher.read(cx);
9292
let listing = switcher.listing();
@@ -110,12 +110,19 @@ pub fn switcher(shell: &Shell, cx: &App) -> Value {
110110
})
111111
})
112112
.collect();
113+
let menu = switcher.menu().map(|menu| {
114+
json!({
115+
"repo": menu.location.display_name(),
116+
"removable": switcher.can_remove(&menu.location),
117+
})
118+
});
113119
json!({
114120
"open": true,
115121
"mode": switcher.mode().key(),
116122
"query": switcher.query(),
117123
"selected": switcher.selected_key(),
118124
"items": items,
125+
"menu": menu,
119126
})
120127
}
121128

‎crates/dv/src/automation/tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ fn state_describes_the_empty_shell(cx: &mut TestAppContext) {
236236
assert_eq!(state["backends"], json!([]));
237237
assert_eq!(
238238
state["switcher"],
239-
json!({"open": false, "mode": null, "query": null, "selected": null, "items": []})
239+
json!({"open": false, "mode": null, "query": null, "selected": null, "items": [], "menu": null})
240240
);
241241
assert_eq!(
242242
state["source_picker"],

‎crates/dv_ui/src/actions.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ pub mod switcher {
112112
SelectPrevious,
113113
/// Open what the selected row stands for (`Enter`).
114114
Confirm,
115-
/// Remove the selected row's repository from the list (`Shift Delete`).
115+
/// Remove the selected row's repository from the list (`Ctrl Shift Backspace`).
116116
RemoveRepo,
117117
/// Close without opening anything (`Esc`).
118118
Dismiss,

‎crates/dv_ui/src/keymap.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,10 @@ pub const SHORTCUTS: &[Shortcut] = &[
150150
shortcut!("down", SWITCHER, switcher::SelectNext, AheadOfInputs),
151151
shortcut!("up", SWITCHER, switcher::SelectPrevious, AheadOfInputs),
152152
shortcut!("enter", SWITCHER, switcher::Confirm),
153-
// Ahead of the field too, which would otherwise cut its text.
153+
// The filter field deletes text with Backspace and its Ctrl, Alt and Shift forms, never
154+
// with Ctrl Shift Backspace, which the field would otherwise swallow.
154155
shortcut!(
155-
"shift-delete",
156+
"secondary-shift-backspace",
156157
SWITCHER,
157158
switcher::RemoveRepo,
158159
AheadOfInputs

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -738,22 +738,6 @@ impl Reviews {
738738
})
739739
}
740740

741-
/// Forgets the repository at `location` (the switcher's remove, DESIGN §4.1): its row
742-
/// leaves the switcher and `known_repos.json`, and a refresh running for it is dropped.
743-
/// Nothing in the repository changes: its reviews stay under `.git/dv`, and adding it again
744-
/// (or opening a review in it) lists them as before. `false` when it wasn't known.
745-
pub fn remove_repo(&mut self, location: &Location, cx: &mut Context<Self>) -> bool {
746-
if !self.known.remove(location) {
747-
return false;
748-
}
749-
self.rows.remove(location);
750-
self.refreshing.remove(location);
751-
self.save(cx);
752-
cx.emit(ReviewsEvent::Changed);
753-
cx.notify();
754-
true
755-
}
756-
757741
/// Knows `location` from now on, unless it is known already however its WSL distro is
758742
/// spelled (module docs); the location as known.
759743
fn add_known(&mut self, location: Location, cx: &mut Context<Self>) -> Location {
@@ -771,6 +755,22 @@ impl Reviews {
771755
location
772756
}
773757

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

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,3 +875,73 @@ fn a_known_repo_spelled_otherwise_becomes_the_spelling_wsl_resolves(cx: &mut Tes
875875
});
876876
assert_eq!(reviews.read_with(cx, |reviews, _| reviews.rows().len()), 1);
877877
}
878+
879+
#[gpui::test]
880+
async fn a_removed_repo_leaves_the_list_and_the_file_keeping_its_reviews(cx: &mut TestAppContext) {
881+
let data = tempfile::tempdir().unwrap();
882+
let repo = fixture();
883+
let other = fixture();
884+
let (locations, reviews) = services(cx, data.path());
885+
locations.update(cx, |locations, _| {
886+
locations.register_env(repo.path().to_path_buf(), repo.env_overlay());
887+
locations.register_env(other.path().to_path_buf(), other.env_overlay());
888+
});
889+
let location = Location::Local(repo.path().to_path_buf());
890+
let kept = Location::Local(other.path().to_path_buf());
891+
for added in [&location, &kept] {
892+
reviews
893+
.update(cx, |reviews, cx| reviews.add_repo(added.clone(), cx))
894+
.await
895+
.expect("a repo");
896+
}
897+
let opened = reviews
898+
.update(cx, |reviews, cx| {
899+
reviews.open_review(location.clone(), Target::Working, cx)
900+
})
901+
.await
902+
.expect("a review");
903+
settle(cx, &reviews);
904+
905+
// Removed while a refresh of it runs: the row goes at once, and the refresh lands nowhere.
906+
let removed = reviews.update(cx, |reviews, cx| {
907+
reviews.refresh(location.clone(), cx);
908+
reviews.remove_repo(&location, cx)
909+
});
910+
assert!(removed);
911+
settle(cx, &reviews);
912+
reviews.read_with(cx, |reviews, _| {
913+
assert!(reviews.row(&location).is_none());
914+
assert!(reviews.known().get(&location).is_none());
915+
assert_eq!(reviews.rows().len(), 1);
916+
assert_eq!(reviews.rows()[0].location, kept);
917+
});
918+
let saved: Value =
919+
serde_json::from_str(&std::fs::read_to_string(data.path().join(KNOWN_REPOS_FILE)).unwrap())
920+
.unwrap();
921+
assert_eq!(
922+
saved["repos"].as_array().unwrap().len(),
923+
1,
924+
"saved without it"
925+
);
926+
assert_eq!(
927+
saved["repos"][0]["location"],
928+
serde_json::to_value(&kept).unwrap()
929+
);
930+
// Not known any more, so nothing to remove.
931+
assert!(!reviews.update(cx, |reviews, cx| reviews.remove_repo(&location, cx)));
932+
933+
// Added again, it lists its review: nothing in the repository was touched.
934+
reviews
935+
.update(cx, |reviews, cx| reviews.add_repo(location.clone(), cx))
936+
.await
937+
.expect("added again");
938+
settle(cx, &reviews);
939+
reviews.read_with(cx, |reviews, _| {
940+
let row = reviews.row(&location).expect("listed again");
941+
assert_eq!(
942+
row.open_review_of(&Target::Working)
943+
.map(|summary| &summary.id),
944+
Some(&opened.loaded.review.id)
945+
);
946+
});
947+
}

‎crates/dv_ui/src/shell/mod.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,7 +841,11 @@ impl Shell {
841841
.session
842842
.as_ref()
843843
.map(|session| session.read(cx).review_id().clone());
844-
let view = cx.new(|cx| Switcher::new(reviews, focus, current.as_ref(), window, cx));
844+
let in_use = self
845+
.session
846+
.as_ref()
847+
.map(|session| session.read(cx).origin().clone());
848+
let view = cx.new(|cx| Switcher::new(reviews, focus, current.as_ref(), in_use, window, cx));
845849
let subscription = cx.subscribe_in(&view, window, Self::on_switcher_event);
846850
view.update(cx, |switcher, cx| switcher.focus(window, cx));
847851
self.switcher = Some(Overlay {

‎crates/dv_ui/src/switcher/items.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,12 +300,26 @@ impl Listing {
300300
Some((location, request))
301301
}
302302

303-
/// The repository of `item`.
303+
/// The repository of `item`; `None` for "Open folder…".
304304
pub fn location_of(&self, item: usize) -> Option<&Location> {
305305
let item = self.items.get(item)?;
306306
Some(&self.repos.get(item.repo?)?.location)
307307
}
308308

309+
/// The first row of the repository listed after `location`'s, else of the one before it,
310+
/// else "Open folder…": where the selection goes when `location` is removed.
311+
pub fn row_after_removing(&self, location: &Location) -> Option<&str> {
312+
let repo = self
313+
.repos
314+
.iter()
315+
.position(|repo| &repo.location == location)?;
316+
let first_of = |repo: usize| self.items.iter().find(|item| item.repo == Some(repo));
317+
first_of(repo + 1)
318+
.or_else(|| repo.checked_sub(1).and_then(first_of))
319+
.or_else(|| self.items.last())
320+
.map(|item| item.key.as_str())
321+
}
322+
309323
/// The repository picking `item` connects to, when it is a **Start** or **Retry** row
310324
/// (whose start doesn't run already).
311325
pub fn start(&self, item: usize) -> Option<Location> {

0 commit comments

Comments
 (0)