diff --git a/crates/dv/src/automation/state.rs b/crates/dv/src/automation/state.rs index c8f355c..d4e0ff5 100644 --- a/crates/dv/src/automation/state.rs +++ b/crates/dv/src/automation/state.rs @@ -86,7 +86,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(); @@ -110,12 +110,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, }) } diff --git a/crates/dv/src/automation/tests.rs b/crates/dv/src/automation/tests.rs index ce9b0c3..c7b91af 100644 --- a/crates/dv/src/automation/tests.rs +++ b/crates/dv/src/automation/tests.rs @@ -236,7 +236,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"], diff --git a/crates/dv_ui/src/actions.rs b/crates/dv_ui/src/actions.rs index 75d7d78..64356d8 100644 --- a/crates/dv_ui/src/actions.rs +++ b/crates/dv_ui/src/actions.rs @@ -112,7 +112,7 @@ pub mod switcher { SelectPrevious, /// Open what the selected row stands for (`Enter`). Confirm, - /// Remove the selected row's repository from the list (`Shift Delete`). + /// Remove the selected row's repository from the list (`Ctrl Shift Backspace`). RemoveRepo, /// Close without opening anything (`Esc`). Dismiss, diff --git a/crates/dv_ui/src/keymap.rs b/crates/dv_ui/src/keymap.rs index b4d9f63..7e53884 100644 --- a/crates/dv_ui/src/keymap.rs +++ b/crates/dv_ui/src/keymap.rs @@ -150,9 +150,10 @@ pub const SHORTCUTS: &[Shortcut] = &[ shortcut!("down", SWITCHER, switcher::SelectNext, AheadOfInputs), shortcut!("up", SWITCHER, switcher::SelectPrevious, AheadOfInputs), shortcut!("enter", SWITCHER, switcher::Confirm), - // Ahead of the field too, which would otherwise cut its text. + // 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!( - "shift-delete", + "secondary-shift-backspace", SWITCHER, switcher::RemoveRepo, AheadOfInputs diff --git a/crates/dv_ui/src/services/reviews.rs b/crates/dv_ui/src/services/reviews.rs index bfc9ff0..44ad0c2 100644 --- a/crates/dv_ui/src/services/reviews.rs +++ b/crates/dv_ui/src/services/reviews.rs @@ -738,22 +738,6 @@ impl Reviews { }) } - /// Forgets the repository at `location` (the switcher's remove, DESIGN §4.1): its row - /// leaves the switcher and `known_repos.json`, and a refresh running for it is dropped. - /// Nothing in the repository changes: its reviews stay under `.git/dv`, and adding it again - /// (or opening a review in it) lists them as before. `false` when it wasn't known. - pub fn remove_repo(&mut self, location: &Location, cx: &mut Context) -> 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 - } - /// Knows `location` from now on, unless it is known already however its WSL distro is /// spelled (module docs); the location as known. fn add_known(&mut self, location: Location, cx: &mut Context) -> Location { @@ -771,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) -> 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 diff --git a/crates/dv_ui/src/services/reviews/tests.rs b/crates/dv_ui/src/services/reviews/tests.rs index 721c59b..c38356e 100644 --- a/crates/dv_ui/src/services/reviews/tests.rs +++ b/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) + ); + }); +} diff --git a/crates/dv_ui/src/shell/mod.rs b/crates/dv_ui/src/shell/mod.rs index 67837d0..b4efac2 100644 --- a/crates/dv_ui/src/shell/mod.rs +++ b/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 { diff --git a/crates/dv_ui/src/switcher/items.rs b/crates/dv_ui/src/switcher/items.rs index a8cfc2d..ce9ec49 100644 --- a/crates/dv_ui/src/switcher/items.rs +++ b/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 { diff --git a/crates/dv_ui/src/switcher/mod.rs b/crates/dv_ui/src/switcher/mod.rs index 9730762..c4758b3 100644 --- a/crates/dv_ui/src/switcher/mod.rs +++ b/crates/dv_ui/src/switcher/mod.rs @@ -5,12 +5,10 @@ //! stopped WSL distro's **Start** row starts it, and a failed connection's **Retry** row //! connects again, the switcher staying open while it lists the repository, which then has //! the selection. Meanwhile the Start or Retry row stays, inert, with the selection: picking -//! it again does nothing. A repository's header has a remove button (`Shift Delete` removes the -//! selected row's): it leaves the list, its reviews staying in the repository, and the -//! selection moves to the repository that took its place. +//! it again does nothing. //! //! A filter field has the keyboard while it is open: typing filters ([`items::build`]), `↑`/`↓` -//! move the selection (wrapping), `Enter` picks, `Shift Delete` removes, `Esc` closes. The switcher only reports what +//! move the selection (wrapping), `Enter` picks, `Esc` closes. The switcher only reports what //! was picked ([`SwitcherEvent`]); the shell opens it. Rows refresh as the reviews service //! recomputes them (it does when the switcher opens, and when a connection a row goes through //! starts reconnecting, stops or fails, or comes back), keeping the selected row. @@ -20,10 +18,17 @@ //! opening or resuming its `branch` review. There `Esc` steps back (bases → branches → the //! repositories, when the picker was entered from them) before it closes. //! +//! A repository's header has a menu (a right-click, or its `⋯` under the pointer) with **Remove +//! from list**, which `Ctrl Shift Backspace` does for the selected row's repository: the +//! repository leaves the known ones (`Reviews::remove_repo`), the selection moving to the next +//! one's first row. The open review's repository can't be removed (its menu item is disabled, +//! the key does nothing): the title bar's source picker lists its targets. +//! //! [`source`] is the title bar's other menu, the current repository's targets. //! -//! Automation targets: `switcher`, `switcher/filter`, `switcher/repo/` and its -//! `switcher/repo//remove`, `switcher/` for each row ([`items::Item::key`]). +//! Automation targets: `switcher`, `switcher/filter`, `switcher/repo/`, +//! `switcher/repo//more` (the header's `⋯`), `switcher/menu`, `switcher/menu/remove`, +//! `switcher/` for each row ([`items::Item::key`]). pub mod branches; pub mod items; @@ -64,12 +69,19 @@ pub enum SwitcherEvent { /// **Start** on the repository at `location`, whose WSL distro isn't running, or **Retry** /// on one whose connection failed: both connect explicitly (`Reviews::start`). Start { location: Location }, - /// Remove the repository at `location` from the list (`Reviews::remove_repo`). + /// **Remove from list** on the repository at `location` (`Reviews::remove_repo`). Remove { location: Location }, /// Closed without picking (`Esc`). Dismiss, } +/// A repository header's menu: whose, and where it opened. +#[derive(Clone, Debug, PartialEq)] +pub struct RepoMenu { + pub location: Location, + pub position: gpui::Point, +} + /// The switcher (see the module docs). pub struct Switcher { reviews: Entity, @@ -85,9 +97,12 @@ pub struct Switcher { /// The repository a **Start** or **Retry** was picked on: its first row takes the /// selection once it lists one. started: Option, - /// The repository that takes a removed one's place: its first row takes the selection - /// once the removed one has left the list. - after_remove: Option, + /// The open review's repository (the one it was opened from), which can't be removed. + in_use: Option, + /// A repository header's menu, while open. + menu: Option, + /// The row a removal moves the selection to, once the listing drops the repository. + after_remove: Option, scroll: ScrollHandle, _subscriptions: Vec, } @@ -97,10 +112,12 @@ impl EventEmitter for Switcher {} impl Switcher { /// A switcher over the known repositories, its selection on `focus`'s first row (the /// repository `dv open` named), else on the open review `current`, else on the first row. + /// `in_use` is the open review's repository, which it won't remove. pub fn new( reviews: Entity, focus: Option<&Location>, current: Option<&ReviewId>, + in_use: Option, window: &mut Window, cx: &mut Context, ) -> Self { @@ -124,6 +141,8 @@ impl Switcher { listing: Listing::default(), selected: 0, started: None, + in_use, + menu: None, after_remove: None, scroll: ScrollHandle::new(), _subscriptions: subscriptions, @@ -203,6 +222,68 @@ impl Switcher { .map(|item| item.key.as_str()) } + /// The open header menu. + pub fn menu(&self) -> Option<&RepoMenu> { + self.menu.as_ref() + } + + /// Whether the repository at `location` can be removed: it isn't the open review's. + pub fn can_remove(&self, location: &Location) -> bool { + self.in_use.as_ref() != Some(location) + } + + /// The repository `Ctrl Shift Backspace` would remove: the selected row's, when it can be. + /// Only the repositories' list removes: the branch picker's header is the repository it + /// compares in. + pub fn removable(&self) -> Option<&Location> { + if self.mode != Mode::Repos { + return None; + } + self.listing + .location_of(self.selected) + .filter(|location| self.can_remove(location)) + } + + /// Opens the header menu of the repository at `location`, at `position`. + pub fn open_menu( + &mut self, + location: Location, + position: gpui::Point, + cx: &mut Context, + ) { + self.menu = Some(RepoMenu { location, position }); + cx.notify(); + } + + pub fn close_menu(&mut self, cx: &mut Context) { + if self.menu.take().is_some() { + cx.notify(); + } + } + + /// Removes the repository at `location` from the list, when it can be (module docs). + pub fn remove(&mut self, location: &Location, cx: &mut Context) { + self.menu = None; + if self.mode != Mode::Repos || !self.can_remove(location) { + cx.notify(); + return; + } + self.after_remove = self + .listing + .row_after_removing(location) + .map(str::to_string); + cx.emit(SwitcherEvent::Remove { + location: location.clone(), + }); + cx.notify(); + } + + fn remove_selected(&mut self, _: &RemoveRepo, _: &mut Window, cx: &mut Context) { + if let Some(location) = self.removable().cloned() { + self.remove(&location, cx); + } + } + /// Selects the row `key`; `false` when it isn't listed. pub fn select_key(&mut self, key: &str, cx: &mut Context) -> bool { match self.listing.position(key) { @@ -245,15 +326,14 @@ impl Switcher { if started.is_some() { self.started = None; } - let kept = keep.and_then(|key| self.listing.position(&key)); - let replaced = match kept { - Some(_) => None, - None => self - .after_remove - .take() - .and_then(|location| self.listing.first_of(&location)), - }; - self.selected = started.or(kept).or(replaced).unwrap_or(0); + let removed_to = self + .after_remove + .take() + .and_then(|key| self.listing.position(&key)); + self.selected = started + .or(removed_to) + .or_else(|| keep.and_then(|key| self.listing.position(&key))) + .unwrap_or(0); self.scroll_to_selected(); cx.notify(); } @@ -300,37 +380,13 @@ impl Switcher { self.pick(self.selected, window, cx); } - fn remove_selected(&mut self, _: &RemoveRepo, _: &mut Window, cx: &mut Context) { - let repo = self - .listing - .items - .get(self.selected) - .and_then(|item| item.repo); - if let Some(repo) = repo { - self.remove(repo, cx); - } - } - - /// Asks for the repository `listing.repos[repo]` to be removed, the one after it (else the - /// one before) to have the selection once it has gone. Only the repositories' list removes: - /// the branch picker's header is the repository it compares in. - fn remove(&mut self, repo: usize, cx: &mut Context) { - if self.mode != Mode::Repos { + /// `Esc`: closes the header menu when it is open, else a step back in the branch picker + /// (module docs), else closes. + fn dismiss(&mut self, _: &Dismiss, window: &mut Window, cx: &mut Context) { + if self.menu.take().is_some() { + cx.notify(); return; } - let repos = &self.listing.repos; - let Some(location) = repos.get(repo).map(|header| header.location.clone()) else { - return; - }; - self.after_remove = repos - .get(repo + 1) - .or_else(|| repo.checked_sub(1).and_then(|before| repos.get(before))) - .map(|header| header.location.clone()); - cx.emit(SwitcherEvent::Remove { location }); - } - - /// `Esc`: a step back in the branch picker (module docs), else closes. - fn dismiss(&mut self, _: &Dismiss, window: &mut Window, cx: &mut Context) { let repo_key = self.listing.repos.first().map(|repo| repo.key.clone()); match self.mode.clone() { Mode::Bases { location, branch } => { diff --git a/crates/dv_ui/src/switcher/render.rs b/crates/dv_ui/src/switcher/render.rs index 4a4e723..4846732 100644 --- a/crates/dv_ui/src/switcher/render.rs +++ b/crates/dv_ui/src/switcher/render.rs @@ -1,20 +1,23 @@ //! The switcher's panel (DESIGN §3: `overlay` over the scrim, radius 12, the overlay shadow): //! the filter field, the repositories and their rows, and a footer with "Open folder…" and the -//! keys. A repository's header shows its remove button while the pointer is over it. +//! keys. use gpui::{ - AnyElement, Context, ElementId, FontWeight, InteractiveElement as _, IntoElement, - ParentElement as _, Render, SharedString, StatefulInteractiveElement as _, Styled as _, Window, - div, prelude::FluentBuilder as _, px, + AnyElement, Context, ElementId, FontWeight, InteractiveElement as _, IntoElement, MouseButton, + MouseDownEvent, ParentElement as _, Render, SharedString, StatefulInteractiveElement as _, + Styled as _, Window, anchored, deferred, div, prelude::FluentBuilder as _, px, }; use gpui_component::input::Input; use super::branches::Mode; use super::items::{Entry, Item, ItemKind, RepoHeader, RepoSummary, TargetKind}; -use super::{SWITCHER_WIDTH, Switcher}; +use super::{RepoMenu, SWITCHER_WIDTH, Switcher}; + +/// The header menu's layer: above the switcher, itself at the shell's menu priority (4). +const REPO_MENU_PRIORITY: usize = 5; use crate::automation::AutomationTargetExt as _; use crate::components::{ - count_badge, icon, kbd, location_chip, skeleton, status_chip, tooltip, totals, + count_badge, icon, kbd, location_chip, menu_item, menu_panel, skeleton, status_chip, totals, }; use crate::icons::DvIcon; use crate::keymap; @@ -31,8 +34,8 @@ impl Render for Switcher { .on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_previous)) .on_action(cx.listener(Self::confirm)) - .on_action(cx.listener(Self::remove_selected)) .on_action(cx.listener(Self::dismiss)) + .on_action(cx.listener(Self::remove_selected)) .w(SWITCHER_WIDTH) .max_h(max_height) .flex() @@ -49,6 +52,11 @@ impl Render for Switcher { .child(self.render_filter(&theme)) .child(self.render_list(&theme, cx)) .child(self.render_footer(&theme, cx)) + .children( + self.menu + .clone() + .map(|menu| self.render_menu(menu, &theme, cx)), + ) .automation_target("switcher") } } @@ -97,7 +105,7 @@ impl Switcher { format!("No branches match “{}”", self.query.trim()) }; return list - .child(self.repo_header(0, &self.listing.repos[0], true, theme, cx)) + .child(repo_header(&self.listing.repos[0], true, None, theme)) .child( div() .flex() @@ -149,7 +157,7 @@ impl Switcher { .listing .repos .get(repo) - .map(|header| self.repo_header(repo, header, line == 0, theme, cx)), + .map(|header| self.repo_header(header, line == 0, theme, cx)), Entry::Item(ix) => self .listing .items @@ -307,6 +315,9 @@ impl Switcher { .pr(theme.spacing.sm) .text_spec(&theme.text.secondary) .text_color(c.fg3) + .when(self.removable().is_some(), |this| { + this.child(hint(vec!["secondary-shift-backspace"], "remove")) + }) .child(hint(vec!["up", "down"], "move")) .child(hint( vec!["enter"], @@ -328,140 +339,222 @@ impl Switcher { .into_any_element() } - /// A repository's header: its name, its path dimmed, its location chip, its remove button - /// while the pointer is over it, and a line on its uncommitted changes or why it can't be - /// opened. + /// A repository header's menu at the pointer: **Remove from list**, disabled for the open + /// review's repository with the reason under it. + fn render_menu(&self, menu: RepoMenu, theme: &DvTheme, cx: &mut Context) -> AnyElement { + let c = &theme.colors; + let removable = self.can_remove(&menu.location); + let location = menu.location.clone(); + let remove = menu_item( + "switcher-menu-remove", + "Remove from list", + DvIcon::X, + removable, + theme, + ) + .when(removable, |this| { + this.on_click(cx.listener(move |this, _, _, cx| this.remove(&location, cx))) + }) + .automation_target("switcher/menu/remove"); + let panel = menu_panel("switcher-menu", theme) + .on_mouse_down_out(cx.listener(|this, _, _, cx| this.close_menu(cx))) + .child(remove) + .when(!removable, |this| { + this.child( + div() + .px(theme.spacing.md) + .pb(theme.spacing.xs) + .text_spec(&theme.text.secondary) + .text_color(c.fg3) + .child("Its review is open"), + ) + }) + .automation_target("switcher/menu"); + deferred( + anchored() + .position(menu.position) + .snap_to_window_with_margin(px(8.)) + .child(panel), + ) + .with_priority(REPO_MENU_PRIORITY) + .into_any_element() + } + + /// A repository's header: its name, its path dimmed, its location chip, and a line on its + /// uncommitted changes or why it can't be opened; in the repositories' list a right-click, + /// or its `⋯` under the pointer, opens its menu (the branch picker's header has none). fn repo_header( &self, - repo: usize, header: &RepoHeader, first: bool, theme: &DvTheme, cx: &mut Context, ) -> AnyElement { + if self.mode != Mode::Repos { + return repo_header(header, first, None, theme); + } + let group = SharedString::from(format!("switcher-repo-{}", header.key)); + let menu_open = self + .menu + .as_ref() + .is_some_and(|menu| menu.location == header.location); let c = &theme.colors; - let summary: AnyElement = match &header.summary { - RepoSummary::Loading => skeleton(px(160.), px(10.), theme), - RepoSummary::Unreachable(reason) => { - let text = match &header.distro { - Some(distro) => format!("{distro} · {reason}"), - None => reason.clone(), - }; - problem(&text, c.yellow, theme) - } - RepoSummary::Stopped(distro) => { - problem(&format!("{distro} isn't running"), c.yellow, theme) - } - RepoSummary::Starting(line) => div() - .text_color(c.fg3) - .child(SharedString::from(line.clone())) - .into_any_element(), - RepoSummary::Error(message) => problem(message, c.red, theme), - RepoSummary::Clean => div() - .text_color(c.fg3) - .child("Working tree clean") - .into_any_element(), - RepoSummary::Changes(working) => div() - .flex() - .items_center() - .gap(theme.spacing.md) - .child(div().text_color(c.fg2).child(format!( - "uncommitted changes · {}", - if working.files == 1 { - "1 file".to_string() - } else { - format!("{} files", working.files) - } - ))) - .child(totals( - working.additions, - working.deletions, - px(11.5), - theme, - )) - .into_any_element(), - }; - let group: SharedString = format!("switcher-repo-{repo}").into(); - let remove = div() - .id(ElementId::NamedInteger( - "switcher-remove".into(), - repo as u64, + let hover = c.control_hover; + let more = div() + .id(ElementId::Name( + format!("switcher-more-{}", header.key).into(), )) - .debug_selector({ - let key = header.key.clone(); - move || format!("switcher-remove-{key}") - }) .flex_none() + .size(px(22.)) .flex() .items_center() .justify_center() - .size(px(20.)) .rounded(theme.radii.control) .cursor_pointer() - .invisible() - .group_hover(group.clone(), |style| style.visible()) - .hover({ - let hover = c.control_hover; - move |style| style.bg(hover) + .hover(move |style| style.bg(hover)) + .when(!menu_open, |this| { + this.opacity(0.) + .group_hover(group.clone(), |style| style.opacity(1.)) }) - .tooltip(tooltip( - "Remove from the list (Shift Delete); its reviews stay in the repository", - )) - .on_click(cx.listener(move |this, _, _, cx| this.remove(repo, cx))) - .child(icon(DvIcon::X, px(13.), c.fg2)) - .automation_target(format!("switcher/repo/{}/remove", header.key)); + .when(menu_open, |this| this.bg(hover)) + .child(icon(DvIcon::Ellipsis, px(14.), c.fg2)) + .on_mouse_down(MouseButton::Left, { + let location = header.location.clone(); + cx.listener(move |this, event: &MouseDownEvent, _, cx| { + cx.stop_propagation(); + if this + .menu + .as_ref() + .is_some_and(|menu| menu.location == location) + { + this.close_menu(cx); + } else { + this.open_menu(location.clone(), event.position, cx); + } + }) + }) + .automation_target(format!("switcher/repo/{}/more", header.key)); + let location = header.location.clone(); div() + .id(ElementId::Name(group.clone())) .group(group) - .flex() - .flex_col() - .gap(px(3.)) - .pt(if first { theme.spacing.xs } else { px(10.) }) - .pb(theme.spacing.xs) - .px(theme.spacing.md) - .child( - div() - .flex() - .items_center() - .gap(theme.spacing.md) - .child(icon(DvIcon::Folder, px(14.), c.fg2)) - .child( - div() - .flex_none() - .font_weight(FontWeight::SEMIBOLD) - .child(SharedString::from(header.name.clone())), - ) - .children( - header - .distro - .as_deref() - .map(|distro| location_chip(distro, theme)), - ) - .child( - div() - .flex_1() - .min_w_0() - .overflow_hidden() - .whitespace_nowrap() - .text_ellipsis_start() - .text_spec(&theme.text.secondary) - .text_color(c.fg3) - .child(SharedString::from(header.shown_path.clone())), - ) - .children((self.mode == Mode::Repos).then_some(remove)), - ) - .child( - div() - .pl(px(22.)) - .h(px(16.)) - .flex() - .items_center() - .text_spec(&theme.text.secondary) - .child(summary), + .child(repo_header( + header, + first, + Some(more.into_any_element()), + theme, + )) + .on_mouse_down( + MouseButton::Right, + cx.listener(move |this, event: &MouseDownEvent, _, cx| { + cx.stop_propagation(); + this.open_menu(location.clone(), event.position, cx); + }), ) - .automation_target(format!("switcher/repo/{}", header.key)) + .into_any_element() } } +/// A repository's header: its name, its path dimmed, its location chip, `more` at the end of +/// its first line, and a line on its uncommitted changes or why it can't be opened. +fn repo_header( + header: &RepoHeader, + first: bool, + more: Option, + theme: &DvTheme, +) -> AnyElement { + let c = &theme.colors; + let summary: AnyElement = match &header.summary { + RepoSummary::Loading => skeleton(px(160.), px(10.), theme), + RepoSummary::Unreachable(reason) => { + let text = match &header.distro { + Some(distro) => format!("{distro} · {reason}"), + None => reason.clone(), + }; + problem(&text, c.yellow, theme) + } + RepoSummary::Stopped(distro) => { + problem(&format!("{distro} isn't running"), c.yellow, theme) + } + RepoSummary::Starting(line) => div() + .text_color(c.fg3) + .child(SharedString::from(line.clone())) + .into_any_element(), + RepoSummary::Error(message) => problem(message, c.red, theme), + RepoSummary::Clean => div() + .text_color(c.fg3) + .child("Working tree clean") + .into_any_element(), + RepoSummary::Changes(working) => div() + .flex() + .items_center() + .gap(theme.spacing.md) + .child(div().text_color(c.fg2).child(format!( + "uncommitted changes · {}", + if working.files == 1 { + "1 file".to_string() + } else { + format!("{} files", working.files) + } + ))) + .child(totals( + working.additions, + working.deletions, + px(11.5), + theme, + )) + .into_any_element(), + }; + div() + .flex() + .flex_col() + .gap(px(3.)) + .pt(if first { theme.spacing.xs } else { px(10.) }) + .pb(theme.spacing.xs) + .px(theme.spacing.md) + .child( + div() + .flex() + .items_center() + .gap(theme.spacing.md) + .child(icon(DvIcon::Folder, px(14.), c.fg2)) + .child( + div() + .flex_none() + .font_weight(FontWeight::SEMIBOLD) + .child(SharedString::from(header.name.clone())), + ) + .children( + header + .distro + .as_deref() + .map(|distro| location_chip(distro, theme)), + ) + .child( + div() + .flex_1() + .min_w_0() + .overflow_hidden() + .whitespace_nowrap() + .text_ellipsis_start() + .text_spec(&theme.text.secondary) + .text_color(c.fg3) + .child(SharedString::from(header.shown_path.clone())), + ) + .children(more), + ) + .child( + div() + .pl(px(22.)) + .h(px(16.)) + .flex() + .items_center() + .text_spec(&theme.text.secondary) + .child(summary), + ) + .automation_target(format!("switcher/repo/{}", header.key)) +} + /// A one-line problem with its glyph. fn problem(text: &str, hue: gpui::Hsla, theme: &DvTheme) -> AnyElement { div() diff --git a/crates/dv_ui/src/switcher/tests.rs b/crates/dv_ui/src/switcher/tests.rs index 277bf10..558c9b6 100644 --- a/crates/dv_ui/src/switcher/tests.rs +++ b/crates/dv_ui/src/switcher/tests.rs @@ -972,8 +972,8 @@ fn the_branch_picker_opens_a_stacked_branch_against_its_base(cx: &mut TestAppCon selected(&shell, cx).as_deref(), Some("api-server/pick/feat-b") ); - // Shift Delete removes repositories from their own list only. - cx.simulate_keystrokes("shift-delete"); + // Ctrl Shift Backspace removes repositories from their own list only. + cx.simulate_keystrokes("secondary-shift-backspace"); settle(cx, &shell, "nothing removed", |_, _| true); let rows = cx.update(|_, cx| { crate::ActiveServices::services(cx) @@ -1075,51 +1075,112 @@ fn the_source_picker_opens_the_branch_picker_and_esc_closes_it(cx: &mut TestAppC } #[gpui::test] -fn shift_delete_removes_the_selected_rows_repository_and_its_neighbour_takes_the_selection( - cx: &mut TestAppContext, -) { - let (dir, shell, cx) = open_shell(cx); +fn a_repo_is_removed_by_key_or_menu_but_not_the_open_reviews(cx: &mut TestAppContext) { + use crate::services::ActiveServices as _; + let (_dir, shell, cx) = open_shell(cx); let repos = tempfile::tempdir().unwrap(); let api = fixture_at(&repos.path().join("api-server")); let web = fixture_at(&repos.path().join("web")); - add_repo(&api, &shell, cx); - add_repo(&web, &shell, cx); + let docs = fixture_at(&repos.path().join("docs")); + for repo in [&api, &web, &docs] { + add_repo(repo, &shell, cx); + } + let api_location = Location::Local(api.path().to_path_buf()); + let task = cx.update(|window, cx| { + shell.update(cx, |shell, cx| { + shell.open( + api_location.clone(), + OpenRequest::Target(Target::Working), + window, + cx, + ) + }) + }); + cx.run_until_parked(); + settle(cx, &shell, "the review", |shell, _| { + shell.session().is_some() + }); + cx.foreground_executor().block_test(task).unwrap(); + let known = |shell: &Entity, cx: &mut VisualTestContext| { + shell.read_with(cx, |_, cx| { + let mut names: Vec = cx + .services() + .reviews + .read(cx) + .rows() + .iter() + .map(|row| row.name.clone()) + .collect(); + names.sort(); + names + }) + }; - // The most recently added first: web's row is selected; Shift Delete removes web. cx.simulate_keystrokes("secondary-o"); settle(cx, &shell, "the rows", |_, _| true); - assert_eq!(selected(&shell, cx).as_deref(), Some("web/working")); - cx.simulate_keystrokes("shift-delete"); - settle(cx, &shell, "the removal", |_, _| true); - let view = switcher(&shell, cx).expect("the switcher stays open"); - view.read_with(cx, |view, _| { - assert!(view.listing().repos.iter().all(|repo| repo.name != "web")); - assert_eq!(view.query(), "", "the filter keeps its text"); - }); + let view = switcher(&shell, cx).expect("Ctrl O opens the switcher"); + // The open review's repository: the key does nothing, and its menu's item is disabled. assert_eq!(selected(&shell, cx).as_deref(), Some("api-server/working")); - let file = std::fs::read_to_string(dir.path().join("known_repos.json")).unwrap(); - let known: serde_json::Value = serde_json::from_str(&file).unwrap(); - assert_eq!(known["repos"].as_array().map(Vec::len), Some(1), "{file}"); - assert!(file.contains("api-server"), "{file}"); - - // Its reviews stay in the repository: it lists again once added back. - cx.simulate_keystrokes("escape"); - add_repo(&web, &shell, cx); - let rows = cx.update(|_, cx| { - crate::ActiveServices::services(cx) - .reviews - .read(cx) - .rows() - .len() + cx.simulate_keystrokes("secondary-shift-backspace"); + assert_eq!(known(&shell, cx), ["api-server", "docs", "web"]); + view.update(cx, |view, cx| { + assert_eq!(view.removable(), None); + view.open_menu( + api_location.clone(), + gpui::point(gpui::px(10.), gpui::px(10.)), + cx, + ); + view.remove(&api_location, cx); + assert!(view.menu().is_none(), "the menu closes"); }); - assert_eq!(rows, 2); + cx.run_until_parked(); + assert_eq!(known(&shell, cx), ["api-server", "docs", "web"]); - // The last repository goes too: the list says there are none. - cx.simulate_keystrokes("secondary-o"); - settle(cx, &shell, "the rows", |_, _| true); - cx.simulate_keystrokes("shift-delete"); + // From the filter field, Ctrl Shift Backspace removes the selected row's repository; the + // selection moves to the next repository's first row. + let order: Vec = view.read_with(cx, |view, _| { + view.listing() + .repos + .iter() + .map(|repo| repo.key.clone()) + .collect() + }); + let (middle, after) = (order[1].clone(), order[2].clone()); + assert!(view.update(cx, |view, cx| { + view.select_key(&format!("{middle}/staged"), cx) + })); + assert!(cx.update(|window, cx| view.read(cx).is_focused(window, cx))); + cx.simulate_keystrokes("secondary-shift-backspace"); settle(cx, &shell, "the removal", |_, _| true); - cx.simulate_keystrokes("shift-delete"); + let mut left = vec!["api-server".to_string(), after.clone()]; + left.sort(); + assert_eq!(known(&shell, cx), left); + assert_eq!(selected(&shell, cx), Some(format!("{after}/working"))); + + // The menu's Remove from list does the same; Esc closes an open menu first. + let after_location = view.read_with(cx, |view, _| { + view.listing() + .repos + .iter() + .find(|repo| repo.key == after) + .unwrap() + .location + .clone() + }); + view.update(cx, |view, cx| { + view.open_menu( + after_location.clone(), + gpui::point(gpui::px(10.), gpui::px(10.)), + cx, + ) + }); + cx.simulate_keystrokes("escape"); + assert!(view.read_with(cx, |view, _| view.menu().is_none())); + assert!( + switcher(&shell, cx).is_some(), + "Esc closed the menu, not the switcher" + ); + view.update(cx, |view, cx| view.remove(&after_location, cx)); settle(cx, &shell, "the removal", |_, _| true); - view.read_with(cx, |view, _| assert!(view.listing().repos.is_empty())); + assert_eq!(known(&shell, cx), ["api-server"]); } diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 4eb4efa..a63298f 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -215,13 +215,18 @@ inactive: material falls back to its inactive appearance (OS-managed); dv dims n beside its open review (status chip, "round n", open-comment badge; "New review" on the selected row without one), then the repo's other open reviews by their target label, then, when it has a branch other than the default one, **Compare branches…** (arrows glyph). Picking a row opens or - resumes that review (PLAN §2.4). A repo that can't be opened offers no rows. While the pointer is - over a header, an `x` (13, `fg2`, a 20-square hover target at its right) removes the repo from - the list (tooltip "Remove from the list (Shift Delete); its reviews stay in the repository"); - `Shift Delete` removes the selected row's repo. Nothing in the repo changes, so adding it again - lists its reviews as before; the switcher stays open, the selection kept, or on the first row - of the repo that took the removed one's place. The branch picker's header has no `x`, and - `Shift Delete` removes nothing there. + resumes that review (PLAN §2.4). A repo that can't be opened offers no rows. A header's menu + (a right-click on it, or its `⋯`, a 22-square ghost button at the end of its first line shown + under the pointer and while its menu is open; the menu panel at the pointer, as the comments + row menu's) holds **Remove from list** (x glyph): the repo leaves the switcher and + `known_repos.json`, nothing in it changing (its reviews show again once it is opened again), and + the selection moves to the next repo's first row, else the previous one's, else **Open + folder…**. For the open review's repo the item is disabled, "Its review is open" in `fg3` under + it. `Ctrl Shift Backspace` removes the selected row's repo the same way, from the filter field + too (which deletes text with Backspace's other forms), and does nothing on the open review's; + the footer's hints start with it ("remove") while the selected row's repo can be removed. `Esc` + closes an open menu before the switcher. The branch picker's header has + no menu, and `Ctrl Shift Backspace` removes nothing there. **Compare branches…** turns the switcher into the branch picker for that repo, in the same panel: the field cleared, its placeholder "Pick a branch to review", the repo's header over one row per local branch but the default one (branch glyph, the most recently committed first): the name, then @@ -901,6 +906,7 @@ Setup unchanged. | `Ctrl Enter` | finish review / submit: the "Finish round" sheet, while no text field has the keyboard (in the composer: post comment) | | `Ctrl K` | the palette: commands, reviews and, as you type, the review's files (§4.1; again, or `Esc`, closes it) | | `Ctrl O` | switch repo / review (the switcher; again, or `Esc`, closes it) | +| `Ctrl Shift Backspace` | in the switcher: remove the selected row's repo from the list (not the open review's, §4.1) | | `Ctrl G` | the PR picker (§4.5) | | `Ctrl R` | on a PR review: refresh it from GitHub (fetch, threads, checks, Viewed; PLAN §9.4) | | `Ctrl B` | toggle panel | @@ -917,7 +923,6 @@ Setup unchanged. | `Ctrl F` | the find bar, its field focused (the selected text in it when that is one line); again, the field focused with its text selected (§4.1 "Find") | | `Enter` / `Shift Enter`, `Esc` | in the find bar: the next / previous match, wrapping; close it (the keyboard back to the stream) | | `↑` / `↓`, `Enter` | in the switcher, the source picker, the file finder and the palette: move (wrapping), open | -| `Shift Delete` | in the switcher: remove the selected row's repo from the list (its reviews stay in the repo) | | `Enter` / `Esc` | in the file filter: show the first file left / clear the filter (both return to the diff) | | `Ctrl Enter` / `Esc` | in the composer: post / discard (both return to the diff; `Enter` is a newline) | | `↑` / `↓`, `1` / `2`, `Tab` | in the "Finish round" sheet: choose the verdict (Approve, Request changes) / move between the verdicts and the summary; `Ctrl Enter` finishes the round, `Esc` closes | diff --git a/docs/PLAN.md b/docs/PLAN.md index 4bcb0b6..adca7ef 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -2083,7 +2083,9 @@ at all and the worktree root its reviews name; a known repo is a worktree root, spelled WSL's way, and one added before it could be resolved (a subfolder added while its connection was down, so its row could say why) or under another spelling of its distro becomes that root, row and `known_repos.json` re-keyed (or dropped when the root is known already), the -first time a listing or an open resolves it; repos compare with the distro folded +first time a listing or an open resolves it; "Remove from list" (`remove_repo`, DESIGN §4.1) forgets +one, its row, any refresh of it and its entry in `known_repos.json` going, nothing in the repo +touched, and never the open review's, whose targets the source picker lists; repos compare with the distro folded (`Location::same_place`), so a spelling WSL couldn't confirm never makes a second row; so the switcher shows each target beside its open review by the rule `open_review` resumes by (`review::ops::is_review_of`); it watches nothing itself), `ReviewUi` (what the reviewer chose per review: split or unified), `GitHub` (gh calls, the diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 3d44c6e..3f32db8 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -226,6 +226,7 @@ Ubuntu, from the Linux GUI target dir (`~/.cache/dv-target-gui`), whose | `setup` | Windows only. Setup (PLAN §12, DESIGN §4.3) on a fresh data dir with `DV_FIRST_RUN=1` and `mock_gh` signed in (`tests/fixtures/gh/auth_status.json`): the first-run screen over this machine's checks; a fake Claude Code folder (`home_file`), Check again, its skill's Install; Add to PATH writing the user `Path`'s stand-in; the git command copied through the clipboard seam; WSL's stand-in naming Ubuntu (listed, not reached), Debian (stopped; Start refused: the run reaches no distro but its sandbox's, why under its name) and `docker-desktop` and `podman-machine-default` (container tools' VMs, not listed); Continue (`Enter`) writing the marker and a review opening; Settings (`Ctrl ,`) showing a skill edited since as out of date and Update fixing it, `Esc` closing it; after a `restart`, no first run and the status bar's notice "dv-review skill out of date on Windows", whose Update fixes it; screenshots of the first-run screen, with the distros, Settings and the notice | | `setup_linux` | Linux only, `setup`'s sibling: the first-run screen over this machine's checks alone (no distros on Linux; the header empties `WSL_DISTRO_NAME`, so a run inside WSL names the machine `Linux` as a desktop would), in the scratch home with `mock_gh` signed in; a fake Claude Code folder, Check again, Install; Add to PATH linking `~/.local/bin/dv` to this dv and putting `~/.local/bin` on the shell's `PATH` through its startup file, the shell then asked again with the scratch home; the git command copied through the clipboard seam; Continue and a review opening; after a `restart` the notice "dv-review skill out of date on Linux" and its Update; screenshots of the first-run screen (dark, and set up in dark and light) and the notice | | `switcher_branches` | the branch picker (DESIGN §4.1 "Compare branches…", PLAN §2.2): a stacked repo (`main → feat-a → feat-b`, and `side` off `main`) offers **Compare branches…** after its targets; it lists the branches newest first, each against its stack base (`feat-b` vs `feat-a`, the others vs `main`), a click on `feat-b` lists its bases (`feat-a` "stack base", `main` "default branch", then `side`), `Esc` steps back with the selection on `feat-b`, and `Enter` twice opens `feat-b` vs `feat-a` with only `b.txt`; the source picker's **Compare branches…** opens the picker on the repo, the branch's open review beside it, and `Esc` closes it; screenshots of the branches and the bases in dark, the stacked review, and the branches in light | +| `switcher_remove_repo` | Remove from list (DESIGN §4.1): with two seeded repos and `api`'s review open, `web`'s `⋯` (`switcher/repo/web/more`) opens its header menu and **Remove from list** (`switcher/menu/remove`) drops it from the switcher and `repos` (`known_repos.json`), the switcher staying open; a right-click on `api`'s header opens its menu with the item disabled ("Its review is open", `menu.removable` `false`), `Esc` closes the menu and leaves the switcher open, and `Ctrl Shift Backspace` on `api` removes nothing; screenshots of both menus | | `update` | dv's own update (PLAN D9, DESIGN §4.1): nothing shows in an automation run (it checks no feed); `update_ready` through the seam shows "dv is ready" and Restart to update at the status bar's right beside an open review; with two other dv processes running from the install (the stand-in's `others`) its click opens the confirmation (`update.confirm`, `status/update/confirm`) and Not now keeps everything; a restart the stand-in fails reads "Update failed · details" (`update.failure`, its tooltip) with Retry; with an unsent comment (`J`, `C`, typed) the confirmation names it and Restart anyway records the restart (`update.restart_requested`) while dv keeps running, the draft intact; screenshots of the item in dark and light, both confirmations and the failure | | `backend_loopback` | a repo reached through the loopback `dv backend --stdio` (this `dv`'s own server, a child of the app), as a WSL repo is through its distro's (PLAN §5.2): `seed_repo` with `"backend": "loopback"`, its review opening and a comment posting through the connection (`state.backends`), the remote watch following the agent's edit, a `backend_kill` that reconnects by itself (the status bar's "loopback · reconnecting…", no banner) and a file written while it was down listed after the synthetic rescan, then `backend_block` and a kill that runs out of respawns (attempts 2 and 3: a death soon after a reconnect continues its count): `failed`, "loopback · disconnected" and the backend-down banner (Details, Copy diagnostics); meanwhile the open switcher's row reads "reconnecting…" keeping its rows, then offers **Retry** once the connection failed; unblocked, the banner's Retry connects again and the watch follows the agent once more; then `backend_stop` (the host stopped, as `wsl --shutdown` stops a distro): the respawn starts nothing, `stopped`, "loopback · stopped", the banner's **Start loopback** and the switcher's Start row, and the banner's Start boots it again; stopped once more, the switcher's Start row started with the start held (`backend_hold`): the row stays, inert (`starting`), with the selection, and `Enter` again does nothing, until the repo lists and its first row takes the selection; a review picked from the switcher while the connection reconnects opens once it is back, with no banner; one picked while the reconnect then fails waits as `shell.pending` (the backend banner the only banner, `shell.banner` `null`); with a round summary typed in the sheet meanwhile it waits on after the switcher's Retry row brings the connection back (the review under the sheet stays, the summary kept), and opens by itself once the sheet is cancelled, no banner left; and after a `restart` with `app_args: ["open", ]` the repo is reached through the loopback again, where a review picked with no review open and its server killed at once (under the open) opens once the connection is back (generation 2, no banner); screenshots of the review over the loopback, while reconnecting, the switcher reconnecting and offering Retry, the banner and status bar in dark Mica, dark Opaque and light, the review reconnected, the stopped banner and switcher row, the switcher while its Start runs, and the pick waiting under the backend banner and then for the sheet | | `review_screen` | the diff stream (DESIGN §4.1, §5): the board's `api-server` with a CRLF README under `core.autocrlf`, a binary, a file too large to show, a rename, an untracked file, a deletion and a long file with gaps, opened from the switcher by keyboard; `J`/`K`, `N`/`P`, the cursor (`↑`/`↓`, `←`/`→`), a `Shift` selection and `Esc`, `S` to unified and back, gap rows and `E`/`Shift E`, `PageDown` with the pinned header, the agent staging everything under the reader (the gaps stay open, the place stays), the sideways scroll, line-number clicks and shift-clicks, the file menu's Copy path, `V` collapsing a viewed file and its chevron, "Load anyway"; screenshots of each state in dark Mica, dark Opaque and light | @@ -248,7 +249,6 @@ Ubuntu, from the Linux GUI target dir (`~/.cache/dv-target-gui`), whose | `review_intel_places` | where code intelligence's places go and what they read (PLAN §18.1, §18.2, DESIGN §4.1): references asked on a changed line's old side list that side's text, marked changed (`intel.references.rows[].side` "old") and a click goes to the old line; 700 references (mock_lsp's answer) show their count as soon as the server answers (`wait_for` on `intel.references.count`), the rows' text once read, the review's file first; a hover at the window's bottom hangs above its token, inside the main area (`bounds` of `hover`); `Ctrl`+click into a library's declarations in another repository beside this one (`${parent}` in the fixture) opens a read-only file view of it (`file_view.outside`), where hover works and `C` does nothing; `Esc` in the code after a press to quote keeps a composer's draft, `C` gives it the keyboard and `Esc` in the composer discards it; screenshots of each | | `review_file_view` | the file view (PLAN §18.1, DESIGN §4.1 "File view"): the header menu's View file (`stream/menu/view_file`) opens the stream's file whole at the cursor's line (`file_view`: its new side's 60 lines, `changed_lines` the review's), the cursor moving in it; `Esc` returns to the stream at the line it left, `F` opens it again, `Alt ←` / `Alt →` step between the views (`history`), a draft survives `Ctrl P`, `Alt ←` and the close button (the composer keeps the keyboard) until `Esc` discards it, `Ctrl F` searches that file only (from the cursor on), `Esc` closes the bar first, then its close button the view; screenshots of the file view and its find bar in dark Mica, the file view in light | | `review_palette` | `Ctrl K` (DESIGN §4.1 "Palette"): a click on the title bar's command field opens the palette (`palette`), its field focused and its first command selected; typing filters by every word, `Enter` runs Next file in the main area (`file_list.current_file` moves), a typed name lists the review's file and a click on its row opens it in the file view, a repository's targets list by name, and `Esc` and `Ctrl K` close it; screenshots of the palette empty, filtered to a file and in light mode | -| `switcher_remove` | removing repos from the switcher (DESIGN §4.1): three seeded repos, the pointer over one's header shows its remove button, a click removes it with the switcher staying open and the selection kept, `Shift Delete` removes the selected row's repo and the next one takes the selection, and with the last one gone the list reads "No repositories yet"; screenshots of the button under the pointer and the empty list | | `review_themes` | the themes (DESIGN §1.3): Settings › Appearance's Theme row switches to Claude, which is opaque over the Mica setting (its Mica and Acrylic disabled, the note keeping Mica for Aura), then light; Dracula asked for light shows dark, the mode row saying so; back to Aura the light mode and Mica return; screenshots of a review with an annotation in Aura Dark, Claude Light, Claude Dark and Dracula, and of Settings in Claude Dark, Claude Light and Dracula | | `review_file_finder` | `Ctrl P` and comments on any line (PLAN §18.1, §8): the file finder (`file_finder`) lists every file of the working tree, the changed one first, filters (a name match before a directory match), moves and opens an unchanged file in the file view; `C` on a line of it posts a thread placed there (`session.threads[].in_diff` `false`, `placement` `unchanged`), its card inline; back in the stream it heads the "Not in the diff" section with its "Unchanged file" chip and View file (`thread//view_file`), which opens the file view at its line; `dv comment list --json` (the automation host's `cli`) says `unchanged` and `in_diff: false` with `anchor.current`, `--oneline` `(not in the diff)`; the agent then edits the file, which joins the review, and the thread follows its code, `moved`; screenshots of the finder, the file view with the card and the section | | `workflow_a_agent` | PLAN §3 workflow A end to end, the agent played by the real CLI (`cli`, `cli_spawn`, `cli_join`: this `dv.exe` as a child process) and the reviewer by keyboard alone: before any review the agent's `dv status` has none, and `cli` refuses `open`; `Enter` opens the working review from the switcher and `dv status` names it; the agent's `dv review wait` blocks (still running after the reviewer's comment) while `V`, `J J`, `Shift ↓`, `C` and `Ctrl Enter` comment and `Ctrl Enter`, `Tab`, the summary and `Ctrl Enter` request changes; the wait wakes with `changes_requested`, the round's summary and one open thread; `dv comment list --status open` gives the thread and its lines (the comments list's location agrees); the agent's fix inserts lines above the thread (`repo_edit`) and `dv comment show` finds it moved; `dv comment reply --body-file -` over stdin (the review goes `in_progress`), `dv comment address --note`, `dv review done --note`; the GUI shows the ready banner, the interdiff since round 1 with one file and the thread addressed at its new lines (`moved`, its reply's recorded anchor); a second wait (by id) blocks while `]`, `X` and `Ctrl Enter`, `Tab`, a summary, `Ctrl Enter` approve, then wakes with `approved`; `dv comment list` (`$DV_REVIEW`), `dv review list --all` and `dv status` see the finished review, and a bare `dv review wait` is exit 2 `no_review`; a wait left blocked on a staged review (`dv review new --staged`) outlives `cli_join`'s wait and is killed as the app quits; screenshots of the comment, both sheets, the ready interdiff, the moved card and the resolved one in dark Mica, the approved review in light | @@ -283,7 +283,7 @@ assert on them rather than on screenshots, which are for style. | `system.requests` | what the app asked the OS to do, oldest first, recorded instead of performed: `{"kind": "open_url", "url"}`, `{"kind": "reveal", "path"}`, `{"kind": "pick_folder", "prompt"}`, `{"kind": "clipboard", "text"}` (nothing reaches the real clipboard) | | `system.answers_left` | folder-picker answers still queued | | `repos` | the known repositories (`/known_repos.json`), most recently used first: `{path, name, location, loading, error, stopped, retry, reconnecting, starting, working, reviews}`. `location` is the stored shape (`{"local": "D:\\…"}`, `{"wsl": {"distro", "path"}}`); `error` is `null` or `{message, detail}` (a WSL location's says why it can't be opened: `"Ubuntu isn't running"`, or its connection's failure, kept when the distro still isn't running after a failed start); `stopped` is the distro (or connection) of a repo whose host isn't running, whatever its connection did before (its row offers **Start**), else `null`; `retry` whether its connection failed with its host running (its row offers **Retry**); `reconnecting` whether its connection is reconnecting (the row keeps `working` and `reviews` from its last listing); `starting` whether a Start or Retry is connecting; `working` is `{files, additions, deletions}` of the uncommitted changes, `null` until computed (on `open`, and when the switcher opens) or when it can't be; `reviews` newest first, each `{id, target, target_label, status, round, threads: {open, addressed, resolved, annotation}}`, or `{id, error}` for a log that doesn't load | -| `switcher` | the switcher (`Ctrl O`): `{open, mode, query, selected, items}`; closed it is `{open: false, mode: null, query: null, selected: null, items: []}`. `mode` is `"repos"`, or the branch picker's `"branches"` (a repo's branches) and `"bases"` (the bases the picked branch can be compared with, DESIGN §4.1). `items` are its pickable rows in order, each `{key, kind, target, label, detail, starting, repo, review}`: `key` names the row (`/working`, `/staged`, `/branch`, `/review/`, `/start`, `/retry`, `/compare`, `open_folder`; in the branch picker `/pick/` and `/base/`; `` is the repo's name, its full path when two known repos share a name), `kind` is `"target"`, `"review"`, `"start"` (**Start Ubuntu**, a stopped distro's one row), `"retry"` (**Retry**, a failed connection's one row), `"open_folder"`, `"compare"` (**Compare branches…**), `"branch"` or `"base"`, `starting` whether it is a Start or Retry whose start runs (drawn inert; picking it does nothing), `target` the target or base row's `"working"`/`"staged"`/`"branch"` (else `null`), `detail` a branch row's `"vs "` or a base row's `"stack base"` / `"default branch"` (else `null`), `repo` its path, `review` its open review `{id, status, round, open_threads}` or `null`; `selected` is a `key` | +| `switcher` | the switcher (`Ctrl O`): `{open, mode, query, selected, items, menu}`; closed it is `{open: false, mode: null, query: null, selected: null, items: [], menu: null}`. `mode` is `"repos"`, or the branch picker's `"branches"` (a repo's branches) and `"bases"` (the bases the picked branch can be compared with, DESIGN §4.1). `menu` is a repo header's open menu (a right-click on `switcher/repo/`, or a click on its `⋯`, `switcher/repo//more`; the repositories' list only), `{repo, removable}` (`repo` its path; `removable` `false` for the open review's repo, whose **Remove from list**, `switcher/menu/remove`, is disabled), else `null`. `items` are its pickable rows in order, each `{key, kind, target, label, detail, starting, repo, review}`: `key` names the row (`/working`, `/staged`, `/branch`, `/review/`, `/start`, `/retry`, `/compare`, `open_folder`; in the branch picker `/pick/` and `/base/`; `` is the repo's name, its full path when two known repos share a name), `kind` is `"target"`, `"review"`, `"start"` (**Start Ubuntu**, a stopped distro's one row), `"retry"` (**Retry**, a failed connection's one row), `"open_folder"`, `"compare"` (**Compare branches…**), `"branch"` or `"base"`, `starting` whether it is a Start or Retry whose start runs (drawn inert; picking it does nothing), `target` the target or base row's `"working"`/`"staged"`/`"branch"` (else `null`), `detail` a branch row's `"vs "` or a base row's `"stack base"` / `"default branch"` (else `null`), `repo` its path, `review` its open review `{id, status, round, open_threads}` or `null`; `selected` is a `key` | | `source_picker` | the source picker (the title bar's `⇄ ▾`): `{open, selected, items: [{kind, label, review}], compare}`, `selected` a `kind` or `"compare"`, `compare` whether it offers **Compare branches…** (`source_picker/compare`) after the items | | `file_list` | the panel's file list: `{query, shown, current_file}`; `shown` the paths the filter leaves, in list order; `current_file` the stream's current file (the highlighted row) | | `session` | the open review, or `null`: `review_id`, `repo` (the root of the review's home worktree, where its target resolves: PLAN §2.2; another worktree's than the row it was opened from for a working review made in a linked worktree, or a branch review of a branch checked out in one), `target` (stored shape: `"working"`, `"staged"`, `{"branch": {…}}`, …), `target_label` (`"working tree vs main"`), `status`, `round` (the one in progress; for a finished review the one that finished it, as the status bar says), `read_only` (approved, closed or submitted), `loading` (the file list is loading), `error` (`null` or `{message, detail}`), `viewed_error` (why the last viewed toggle failed, shown as `banner/viewed`; `null` once one lands or it is dismissed), `watch_notice` (why the worktree isn't watched, or `null`), `generation` (the load generation), `warnings` (problems in the log), `viewed` / `total` (files viewed this round / listed), `current_file` (the stream's current file: the cursor's while the cursor is on screen, else the file at the top), `view_mode` (`"split"` or `"unified"`, remembered per review in `/review_ui.json`), `pane` (`"stream"` or `"file_view"`: the pane `cursor`, `selection`, `composer` and `focused_thread` describe, the one the main area shows), `cursor` (`null` or `{path, side, line}`: `side` `"old"`/`"new"`, `line` its number on that side), `selection` (`null` or `{path, side, start_line, end_line}`, one file and one side) | @@ -389,7 +389,7 @@ edge); `shell/main` (280,40 1160x832) with `shell/main/empty`, `shell/main/loading` and `stream`; `shell/scrim` under the top overlay (over the sheet too while a menu is open over it); `statusbar` (0,872 1440x28) with `statusbar/location`, `statusbar/review` and `statusbar/finish` (the last two with a review open). The switcher: `switcher` (560 wide, hung under its button), `switcher/filter`, -`switcher/repo/` and its remove button `switcher/repo//remove` (shown while the pointer is over the header: a `mouse_move` onto the header first), `switcher/` per row (`state.switcher.items`, `switcher//start` included), and +`switcher/repo/`, `switcher/` per row (`state.switcher.items`, `switcher//start` included), and `switcher/empty`; the source picker: `source_picker` and `source_picker/`. The file list: `file_list/filter`, `file_list/progress`, `file_list/` (a 28-high row), `file_list//viewed` diff --git a/tests/e2e/switcher_remove.jsonl b/tests/e2e/switcher_remove.jsonl deleted file mode 100644 index 1a1fc67..0000000 --- a/tests/e2e/switcher_remove.jsonl +++ /dev/null @@ -1,42 +0,0 @@ -{"scenario": {"window_size": "1440x900", "description": "Removing repositories from the switcher (DESIGN §4.1): a header's remove button shows under the pointer and removes its repository, the next one taking the selection; Shift Delete removes the selected row's; the last one gone, the list says there are none. Screenshots of the button under the pointer and the empty list."}} -{"cmd": "wait_ready", "timeout_ms": 20000} -{"cmd": "set_setting", "key": "mode", "value": "dark"} -{"cmd": "settle"} - -// Three repositories, each with a change; dv open on the last opens the switcher on it. -{"cmd": "seed_repo", "name": "api-server", "steps": [{"write": "a.txt", "text": "one\n"}, {"commit": "base"}, {"write": "a.txt", "text": "two\n"}], "save": {"api": "/path"}} -{"cmd": "seed_repo", "name": "web", "steps": [{"write": "b.txt", "text": "one\n"}, {"commit": "base"}, {"write": "b.txt", "text": "two\n"}], "save": {"web": "/path"}} -{"cmd": "seed_repo", "name": "docs", "steps": [{"write": "c.txt", "text": "one\n"}, {"commit": "base"}, {"write": "c.txt", "text": "two\n"}], "save": {"docs": "/path"}} -{"cmd": "open", "path": "${api}"} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "key", "keystroke": "escape"} -{"cmd": "open", "path": "${web}"} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "key", "keystroke": "escape"} -{"cmd": "open", "path": "${docs}"} -{"cmd": "wait_for", "pointer": "/repos/2/working/files", "equals": 1, "timeout_ms": 20000} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "state", "expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/selected", "equals": "docs/working"}, {"pointer": "/repos", "len": 3}]} - -// The pointer over web's header shows its remove button; a click removes web, the switcher -// staying open with the selection where it was. -{"cmd": "mouse_move", "target": "switcher/repo/web"} -{"cmd": "settle"} -{"cmd": "screenshot", "name": "remove-button"} -{"cmd": "click", "target": "switcher/repo/web/remove"} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "state", "expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/selected", "equals": "docs/working"}, {"pointer": "/repos", "len": 2}]} -{"cmd": "query", "prefix": "switcher/repo/web", "expect_result": {"pointer": "", "len": 0}} - -// Shift Delete removes the selected row's repository, docs; api-server takes the selection. -{"cmd": "key", "keystroke": "shift-delete"} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "state", "expect": [{"pointer": "/switcher/selected", "equals": "api-server/working"}, {"pointer": "/repos", "len": 1}]} - -// The last one: the list says there are none, "Open folder…" still there. -{"cmd": "key", "keystroke": "shift-delete"} -{"cmd": "settle", "timeout_ms": 20000} -{"cmd": "state", "expect": [{"pointer": "/switcher/selected", "equals": "open_folder"}, {"pointer": "/repos", "len": 0}]} -{"cmd": "bounds", "target": "switcher/empty", "expect_result": {"pointer": "/visible", "not_equals": null}} -{"cmd": "screenshot", "name": "no-repositories"} -{"cmd": "quit"} diff --git a/tests/e2e/switcher_remove_repo.jsonl b/tests/e2e/switcher_remove_repo.jsonl new file mode 100644 index 0000000..cb08f8a --- /dev/null +++ b/tests/e2e/switcher_remove_repo.jsonl @@ -0,0 +1,30 @@ +{"scenario": {"window_size": "1440x900", "description": "Remove from list (DESIGN §4.1): a repo header's `⋯` opens its menu, and Remove from list drops the repo from the switcher and `known_repos.json`, the selection moving on; the open review's repo has the item disabled (\"Its review is open\") and Ctrl Shift Backspace does nothing on it; Esc closes the menu before the switcher; screenshots of both menus."}} +{"cmd": "wait_ready", "timeout_ms": 20000} +{"cmd": "set_setting", "key": "mode", "value": "dark"} +{"cmd": "settle"} +{"cmd": "seed_repo", "name": "api", "steps": [{"write": "app.ts", "text": "export const app = 1;\n"}, {"commit": "base"}, {"write": "app.ts", "text": "export const app = 2;\n"}], "save": {"api_repo": "/path"}} +{"cmd": "seed_repo", "name": "web", "steps": [{"write": "index.ts", "text": "export const web = 1;\n"}, {"commit": "base"}], "save": {"web_repo": "/path"}} +{"cmd": "open", "path": "${web_repo}"} +{"cmd": "open_review", "path": "${api_repo}"} +{"cmd": "settle", "expect": [{"pointer": "/session/total", "equals": 1}, {"pointer": "/repos", "len": 2}]} + +// The switcher; web's `⋯` opens its menu, Remove from list enabled. +{"cmd": "key", "keystroke": "secondary-o"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/menu", "equals": null}, {"pointer": "/switcher/selected", "equals": "api/working"}]} +{"cmd": "click", "target": "switcher/repo/web/more"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/menu/repo", "equals": "${web_repo}"}, {"pointer": "/switcher/menu/removable", "equals": true}]} +{"cmd": "screenshot", "name": "menu"} +{"cmd": "click", "target": "switcher/menu/remove"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/menu", "equals": null}, {"pointer": "/repos", "len": 1}, {"pointer": "/repos/0/name", "equals": "api"}, {"pointer": "/switcher/items", "contains": {"key": "api/working"}}, {"pointer": "/switcher/selected", "equals": "api/working"}]} + +// The open review's repo: a right-click opens its menu with the item disabled, Esc closes the +// menu and leaves the switcher open, and Ctrl Shift Backspace does nothing. +{"cmd": "click", "target": "switcher/repo/api", "button": "right"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/menu/repo", "equals": "${api_repo}"}, {"pointer": "/switcher/menu/removable", "equals": false}]} +{"cmd": "screenshot", "name": "menu-open-review"} +{"cmd": "key", "keystroke": "escape"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/menu", "equals": null}, {"pointer": "/switcher/open", "equals": true}]} +{"cmd": "key", "keystroke": "secondary-shift-backspace"} +{"cmd": "settle", "expect": [{"pointer": "/repos", "len": 1}, {"pointer": "/switcher/open", "equals": true}]} +{"cmd": "key", "keystroke": "escape"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/open", "equals": false}]}