From 53569ece035b28f28b4a56427883f296ae5db8af Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 5 May 2023 14:23:17 -0700 Subject: [PATCH] WIP: Change RepositoryEntry representation to be keyed off of the work directory Removes branches button scaffolding --- crates/collab/src/db.rs | 16 +- crates/collab/src/db/worktree_repository.rs | 3 +- crates/collab_ui/src/branches_button.rs | 168 --------------- crates/collab_ui/src/collab_titlebar_item.rs | 2 +- crates/collab_ui/src/collab_ui.rs | 2 - crates/project/src/project.rs | 15 +- crates/project/src/worktree.rs | 206 +++++++++++-------- 7 files changed, 132 insertions(+), 280 deletions(-) delete mode 100644 crates/collab_ui/src/branches_button.rs diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 63fd59628c..8d5ad280ef 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1563,12 +1563,11 @@ impl Database { if db_repository.is_deleted { worktree .removed_repositories - .push(db_repository.dot_git_entry_id as u64); + .push(db_repository.work_directory_id as u64); } else { worktree.updated_repositories.push(proto::RepositoryEntry { - dot_git_entry_id: db_repository.dot_git_entry_id as u64, scan_id: db_repository.scan_id as u64, - work_directory: db_repository.work_directory_path, + work_directory_id: db_repository.work_directory_id as u64, branch: db_repository.branch, }); } @@ -2377,8 +2376,7 @@ impl Database { |repository| worktree_repository::ActiveModel { project_id: ActiveValue::set(project_id), worktree_id: ActiveValue::set(worktree_id), - dot_git_entry_id: ActiveValue::set(repository.dot_git_entry_id as i64), - work_directory_path: ActiveValue::set(repository.work_directory.clone()), + work_directory_id: ActiveValue::set(repository.work_directory_id as i64), scan_id: ActiveValue::set(update.scan_id as i64), branch: ActiveValue::set(repository.branch.clone()), is_deleted: ActiveValue::set(false), @@ -2388,11 +2386,10 @@ impl Database { OnConflict::columns([ worktree_repository::Column::ProjectId, worktree_repository::Column::WorktreeId, - worktree_repository::Column::DotGitEntryId, + worktree_repository::Column::WorkDirectoryId, ]) .update_columns([ worktree_repository::Column::ScanId, - worktree_repository::Column::WorkDirectoryPath, worktree_repository::Column::Branch, ]) .to_owned(), @@ -2408,7 +2405,7 @@ impl Database { .eq(project_id) .and(worktree_repository::Column::WorktreeId.eq(worktree_id)) .and( - worktree_repository::Column::DotGitEntryId + worktree_repository::Column::WorkDirectoryId .is_in(update.removed_repositories.iter().map(|id| *id as i64)), ), ) @@ -2650,9 +2647,8 @@ impl Database { worktrees.get_mut(&(db_repository_entry.worktree_id as u64)) { worktree.repository_entries.push(proto::RepositoryEntry { - dot_git_entry_id: db_repository_entry.dot_git_entry_id as u64, scan_id: db_repository_entry.scan_id as u64, - work_directory: db_repository_entry.work_directory_path, + work_directory_id: db_repository_entry.work_directory_id as u64, branch: db_repository_entry.branch, }); } diff --git a/crates/collab/src/db/worktree_repository.rs b/crates/collab/src/db/worktree_repository.rs index b281f2047a..116d7b3ed9 100644 --- a/crates/collab/src/db/worktree_repository.rs +++ b/crates/collab/src/db/worktree_repository.rs @@ -9,10 +9,9 @@ pub struct Model { #[sea_orm(primary_key)] pub worktree_id: i64, #[sea_orm(primary_key)] - pub dot_git_entry_id: i64, + pub work_directory_id: i64, pub scan_id: i64, pub branch: Option, - pub work_directory_path: String, pub is_deleted: bool, } diff --git a/crates/collab_ui/src/branches_button.rs b/crates/collab_ui/src/branches_button.rs deleted file mode 100644 index 7146e7f86b..0000000000 --- a/crates/collab_ui/src/branches_button.rs +++ /dev/null @@ -1,168 +0,0 @@ -use context_menu::{ContextMenu, ContextMenuItem}; -use gpui::{ - elements::*, - platform::{CursorStyle, MouseButton}, - AnyElement, Element, Entity, View, ViewContext, ViewHandle, WeakViewHandle, -}; -use settings::Settings; -use workspace::Workspace; - -///! TODO: This file will hold the branch switching UI once we build it. - -pub struct BranchesButton { - workspace: WeakViewHandle, - popup_menu: ViewHandle, -} - -impl Entity for BranchesButton { - type Event = (); -} - -impl View for BranchesButton { - fn ui_name() -> &'static str { - "BranchesButton" - } - - fn render(&mut self, cx: &mut ViewContext) -> AnyElement { - let Some(workspace) = self.workspace.upgrade(cx) else { - return Empty::new().into_any(); - }; - - let project = workspace.read(cx).project().read(cx); - let only_one_worktree = project.visible_worktrees(cx).count() == 1; - let branches_count: usize = project - .visible_worktrees(cx) - .map(|worktree_handle| worktree_handle.read(cx).snapshot().git_entries().count()) - .sum(); - let branch_caption: String = if only_one_worktree { - project - .visible_worktrees(cx) - .next() - .unwrap() - .read(cx) - .snapshot() - .root_git_entry() - .and_then(|entry| entry.branch()) - .map(|branch| branch.to_string()) - .unwrap_or_else(|| "".to_owned()) - } else { - branches_count.to_string() - }; - let is_popup_menu_visible = self.popup_menu.read(cx).visible(); - - let theme = cx.global::().theme.clone(); - - Stack::new() - .with_child( - MouseEventHandler::::new(0, cx, { - let theme = theme.clone(); - move |state, _cx| { - let style = theme - .workspace - .titlebar - .toggle_contacts_button - .style_for(state, is_popup_menu_visible); - - Flex::row() - .with_child( - Svg::new("icons/version_control_branch_12.svg") - .with_color(style.color) - .constrained() - .with_width(style.icon_width) - .aligned() - // .constrained() - // .with_width(style.button_width) - // .with_height(style.button_width) - // .contained() - // .with_style(style.container) - .into_any_named("version-control-branch-icon"), - ) - .with_child( - Label::new(branch_caption, theme.workspace.titlebar.title.clone()) - .contained() - .with_style(style.container) - .aligned(), - ) - .constrained() - .with_height(style.button_width) - .contained() - .with_style(style.container) - } - }) - .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, move |_, this, cx| { - this.deploy_branches_menu(cx); - }) - .with_tooltip::( - 0, - "Branches".into(), - None, - theme.tooltip.clone(), - cx, - ), - ) - .with_child( - ChildView::new(&self.popup_menu, cx) - .aligned() - .bottom() - .left(), - ) - .into_any_named("branches-button") - } -} - -impl BranchesButton { - pub fn new(workspace: ViewHandle, cx: &mut ViewContext) -> Self { - let parent_id = cx.view_id(); - cx.observe(&workspace, |_, _, cx| cx.notify()).detach(); - Self { - workspace: workspace.downgrade(), - popup_menu: cx.add_view(|cx| { - let mut menu = ContextMenu::new(parent_id, cx); - menu.set_position_mode(OverlayPositionMode::Local); - menu - }), - } - } - - pub fn deploy_branches_menu(&mut self, cx: &mut ViewContext) { - let mut menu_options = vec![]; - - if let Some(workspace) = self.workspace.upgrade(cx) { - let project = workspace.read(cx).project().read(cx); - - let worktrees_with_branches = project - .visible_worktrees(cx) - .map(|worktree_handle| { - worktree_handle - .read(cx) - .snapshot() - .git_entries() - .filter_map(|entry| { - entry.branch().map(|branch| { - let repo_name = entry.work_directory(); - if let Some(name) = repo_name.file_name() { - (name.to_string_lossy().to_string(), branch) - } else { - ("WORKTREE ROOT".into(), branch) - } - }) - }) - .collect::>() - }) - .flatten(); - - let context_menu_items = worktrees_with_branches.map(|(repo_name, branch_name)| { - let caption = format!("{} / {}", repo_name, branch_name); - ContextMenuItem::handler(caption.to_owned(), move |_| { - println!("{}", caption); - }) - }); - menu_options.extend(context_menu_items); - } - - self.popup_menu.update(cx, |menu, cx| { - menu.show(Default::default(), AnchorCorner::TopLeft, menu_options, cx); - }); - } -} diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index d74e2452e0..ec5eaab36c 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -1,6 +1,6 @@ use crate::{ contact_notification::ContactNotification, contacts_popover, face_pile::FacePile, - toggle_screen_sharing, BranchesButton, ToggleScreenSharing, + toggle_screen_sharing, ToggleScreenSharing, }; use call::{ActiveCall, ParticipantLocation, Room}; use client::{proto::PeerId, Client, ContactEventKind, SignIn, SignOut, User, UserStore}; diff --git a/crates/collab_ui/src/collab_ui.rs b/crates/collab_ui/src/collab_ui.rs index 21a864d4e8..c0734388b1 100644 --- a/crates/collab_ui/src/collab_ui.rs +++ b/crates/collab_ui/src/collab_ui.rs @@ -1,4 +1,3 @@ -mod branches_button; mod collab_titlebar_item; mod contact_finder; mod contact_list; @@ -10,7 +9,6 @@ mod notifications; mod project_shared_notification; mod sharing_status_indicator; -pub use branches_button::BranchesButton; use call::ActiveCall; pub use collab_titlebar_item::{CollabTitlebarItem, ToggleContactsMenu}; use gpui::{actions, AppContext, Task}; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 3971eafbdd..3f4c81afd1 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4696,11 +4696,11 @@ impl Project { fn update_local_worktree_buffers_git_repos( &mut self, - worktree: ModelHandle, + worktree_handle: ModelHandle, repos: &Vec, cx: &mut ModelContext, ) { - debug_assert!(worktree.read(cx).is_local()); + debug_assert!(worktree_handle.read(cx).is_local()); for (_, buffer) in &self.opened_buffers { if let Some(buffer) = buffer.upgrade(cx) { @@ -4708,28 +4708,31 @@ impl Project { Some(file) => file, None => continue, }; - if file.worktree != worktree { + if file.worktree != worktree_handle { continue; } let path = file.path().clone(); + let worktree = worktree_handle.read(cx); let repo = match repos .iter() - .find(|entry| entry.work_directory.contains(&path)) + .find(|repository| repository.work_directory.contains(worktree, &path)) { Some(repo) => repo.clone(), None => return, }; - let relative_repo = match repo.work_directory.relativize(&path) { + let relative_repo = match repo.work_directory.relativize(worktree, &path) { Some(relative_repo) => relative_repo.to_owned(), None => return, }; + drop(worktree); + let remote_id = self.remote_id(); let client = self.client.clone(); - let diff_base_task = worktree.update(cx, move |worktree, cx| { + let diff_base_task = worktree_handle.update(cx, move |worktree, cx| { worktree .as_local() .unwrap() diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index ea26a969d2..5b42257f0b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -120,7 +120,7 @@ pub struct Snapshot { #[derive(Clone, Debug, Eq, PartialEq)] pub struct RepositoryEntry { pub(crate) scan_id: usize, - pub(crate) work_directory_id: ProjectEntryId, + pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, } @@ -130,7 +130,17 @@ impl RepositoryEntry { } pub fn work_directory_id(&self) -> ProjectEntryId { - self.work_directory_id + *self.work_directory + } + + pub fn work_directory(&self, snapshot: &Snapshot) -> Option { + snapshot + .entry_for_id(self.work_directory_id()) + .map(|entry| RepositoryWorkDirectory(entry.path.clone())) + } + + pub(crate) fn contains(&self, snapshot: &Snapshot, path: &Path) -> bool { + self.work_directory.contains(snapshot, path) } } @@ -138,7 +148,7 @@ impl From<&RepositoryEntry> for proto::RepositoryEntry { fn from(value: &RepositoryEntry) -> Self { proto::RepositoryEntry { scan_id: value.scan_id as u64, - work_directory_id: value.work_directory_id.to_proto(), + work_directory_id: value.work_directory.to_proto(), branch: value.branch.as_ref().map(|str| str.to_string()), } } @@ -148,39 +158,47 @@ impl From<&RepositoryEntry> for proto::RepositoryEntry { #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] pub struct RepositoryWorkDirectory(Arc); -impl RepositoryWorkDirectory { - // Note that these paths should be relative to the worktree root. - pub(crate) fn contains(&self, path: &Path) -> bool { - path.starts_with(self.0.as_ref()) - } - - pub(crate) fn relativize(&self, path: &Path) -> Option { - path.strip_prefix(self.0.as_ref()) - .ok() - .map(move |path| RepoPath(path.to_owned())) - } -} - -impl Deref for RepositoryWorkDirectory { - type Target = Path; - - fn deref(&self) -> &Self::Target { - self.0.as_ref() - } -} - -impl<'a> From<&'a str> for RepositoryWorkDirectory { - fn from(value: &'a str) -> Self { - RepositoryWorkDirectory(Path::new(value).into()) - } -} - impl Default for RepositoryWorkDirectory { fn default() -> Self { RepositoryWorkDirectory(Arc::from(Path::new(""))) } } +#[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] +pub struct WorkDirectoryEntry(ProjectEntryId); + +impl WorkDirectoryEntry { + // Note that these paths should be relative to the worktree root. + pub(crate) fn contains(&self, snapshot: &Snapshot, path: &Path) -> bool { + snapshot + .entry_for_id(self.0) + .map(|entry| path.starts_with(&entry.path)) + .unwrap_or(false) + } + + pub(crate) fn relativize(&self, worktree: &Snapshot, path: &Path) -> Option { + worktree.entry_for_id(self.0).and_then(|entry| { + path.strip_prefix(&entry.path) + .ok() + .map(move |path| RepoPath(path.to_owned())) + }) + } +} + +impl Deref for WorkDirectoryEntry { + type Target = ProjectEntryId; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl<'a> From for WorkDirectoryEntry { + fn from(value: ProjectEntryId) -> Self { + WorkDirectoryEntry(value) + } +} + #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] pub struct RepoPath(PathBuf); @@ -698,12 +716,12 @@ impl LocalWorktree { ) { for a_repo in a { let matched = b.find(|b_repo| { - a_repo.work_directory_id == b_repo.work_directory_id + a_repo.work_directory == b_repo.work_directory && a_repo.scan_id == b_repo.scan_id }); if matched.is_none() { - updated.insert(a_repo.work_directory_id, a_repo.clone()); + updated.insert(*a_repo.work_directory, a_repo.clone()); } } } @@ -757,8 +775,8 @@ impl LocalWorktree { let mut index_task = None; if let Some(repo) = snapshot.repo_for(&path) { - let repo_path = repo.work_directory.relativize(&path).unwrap(); - if let Some(repo) = self.git_repositories.get(&repo.dot_git_entry_id) { + let repo_path = repo.work_directory.relativize(self, &path).unwrap(); + if let Some(repo) = self.git_repositories.get(&*repo.work_directory) { let repo = repo.repo_ptr.to_owned(); index_task = Some( cx.background() @@ -1155,7 +1173,7 @@ impl LocalWorktree { repo_path: RepoPath, cx: &mut ModelContext, ) -> Task> { - let Some(git_ptr) = self.git_repositories.get(&repo.work_directory_id).map(|git_ptr| git_ptr.to_owned()) else { + let Some(git_ptr) = self.git_repositories.get(&repo.work_directory).map(|git_ptr| git_ptr.to_owned()) else { return Task::Ready(Some(None)) }; let git_ptr = git_ptr.repo_ptr; @@ -1390,7 +1408,7 @@ impl Snapshot { self.repository_entries.retain(|_, entry| { if let Ok(_) = update .removed_repositories - .binary_search(&entry.dot_git_entry_id.to_proto()) + .binary_search(&entry.work_directory.to_proto()) { false } else { @@ -1400,15 +1418,15 @@ impl Snapshot { for repository in update.updated_repositories { let repository = RepositoryEntry { - dot_git_entry_id: ProjectEntryId::from_proto(repository.dot_git_entry_id), - work_directory: RepositoryWorkDirectory( - Path::new(&repository.work_directory).into(), - ), + work_directory: ProjectEntryId::from_proto(repository.work_directory_id).into(), scan_id: repository.scan_id as usize, branch: repository.branch.map(Into::into), }; - self.repository_entries - .insert(repository.work_directory.clone(), repository) + // TODO: Double check this logic + if let Some(entry) = self.entry_for_id(repository.work_directory_id()) { + self.repository_entries + .insert(RepositoryWorkDirectory(entry.path.clone()), repository) + } } self.scan_id = update.scan_id as usize; @@ -1509,7 +1527,7 @@ impl Snapshot { pub fn root_git_entry(&self) -> Option { self.repository_entries - .get(&"".into()) + .get(&RepositoryWorkDirectory(Path::new("").into())) .map(|entry| entry.to_owned()) } @@ -1549,7 +1567,7 @@ impl LocalSnapshot { let mut max_len = 0; let mut current_candidate = None; for (work_directory, repo) in (&self.repository_entries).iter() { - if work_directory.contains(path) { + if repo.contains(self, path) { if work_directory.0.as_os_str().len() >= max_len { current_candidate = Some(repo); max_len = work_directory.0.as_os_str().len(); @@ -1575,8 +1593,8 @@ impl LocalSnapshot { .snapshot .repository_entries .iter() - .find(|(_, entry)| entry.dot_git_entry_id == *entry_id) - .map(|(_, entry)| entry.work_directory.to_owned())?; + .find(|(_, entry)| *entry.work_directory == *entry_id) + .and_then(|(_, entry)| entry.work_directory(self))?; Some((work_dir, local_repo.repo_ptr.to_owned())) } @@ -1675,7 +1693,7 @@ impl LocalSnapshot { other_repos.next(); } Ordering::Greater => { - removed_repositories.push(other_repo.dot_git_entry_id.to_proto()); + removed_repositories.push(other_repo.work_directory.to_proto()); other_repos.next(); } } @@ -1685,7 +1703,7 @@ impl LocalSnapshot { self_repos.next(); } (None, Some(other_repo)) => { - removed_repositories.push(other_repo.dot_git_entry_id.to_proto()); + removed_repositories.push(other_repo.work_directory.to_proto()); other_repos.next(); } (None, None) => break, @@ -1794,28 +1812,32 @@ impl LocalSnapshot { let abs_path = self.abs_path.join(&parent_path); let content_path: Arc = parent_path.parent().unwrap().into(); - let key = RepositoryWorkDirectory(content_path.clone()); - if self.repository_entries.get(&key).is_none() { - if let Some(repo) = fs.open_repo(abs_path.as_path()) { - let repo_lock = repo.lock(); - self.repository_entries.insert( - key.clone(), - RepositoryEntry { - dot_git_entry_id: parent_entry.id, - work_directory: key, - scan_id: 0, - branch: repo_lock.branch_name().map(Into::into), - }, - ); - drop(repo_lock); + if let Some(work_dir_id) = self + .entry_for_path(content_path.clone()) + .map(|entry| entry.id) + { + let key = RepositoryWorkDirectory(content_path.clone()); + if self.repository_entries.get(&key).is_none() { + if let Some(repo) = fs.open_repo(abs_path.as_path()) { + let repo_lock = repo.lock(); + self.repository_entries.insert( + key.clone(), + RepositoryEntry { + work_directory: work_dir_id.into(), + scan_id: 0, + branch: repo_lock.branch_name().map(Into::into), + }, + ); + drop(repo_lock); - self.git_repositories.insert( - parent_entry.id, - LocalRepositoryEntry { - repo_ptr: repo, - git_dir_path: parent_path.clone(), - }, - ) + self.git_repositories.insert( + work_dir_id, + LocalRepositoryEntry { + repo_ptr: repo, + git_dir_path: parent_path.clone(), + }, + ) + } } } } @@ -2514,7 +2536,16 @@ impl BackgroundScanner { snapshot.git_repositories = git_repositories; let mut git_repository_entries = mem::take(&mut snapshot.snapshot.repository_entries); - git_repository_entries.retain(|_, entry| snapshot.contains_entry(entry.dot_git_entry_id)); + git_repository_entries.retain(|_, entry| { + entry + .work_directory(&snapshot) + .map(|directory| { + snapshot + .entry_for_path((directory.as_ref()).join(".git")) + .is_some() + }) + .unwrap_or(false) + }); snapshot.snapshot.repository_entries = git_repository_entries; snapshot.removed_entry_ids.clear(); @@ -3594,23 +3625,19 @@ mod tests { assert!(tree.repo_for("c.txt".as_ref()).is_none()); let entry = tree.repo_for("dir1/src/b.txt".as_ref()).unwrap(); - assert_eq!(entry.work_directory.0.as_ref(), Path::new("dir1")); assert_eq!( - tree.entry_for_id(entry.dot_git_entry_id) - .unwrap() - .path - .as_ref(), - Path::new("dir1/.git") + entry + .work_directory(tree) + .map(|directory| directory.as_ref().to_owned()), + Some(Path::new("dir1").to_owned()) ); let entry = tree.repo_for("dir1/deps/dep1/src/a.txt".as_ref()).unwrap(); - assert_eq!(entry.work_directory.deref(), Path::new("dir1/deps/dep1")); assert_eq!( - tree.entry_for_id(entry.dot_git_entry_id) - .unwrap() - .path - .as_ref(), - Path::new("dir1/deps/dep1/.git"), + entry + .work_directory(tree) + .map(|directory| directory.as_ref().to_owned()), + Some(Path::new("dir1/deps/dep1").to_owned()) ); }); @@ -3647,13 +3674,10 @@ mod tests { #[test] fn test_changed_repos() { - fn fake_entry(dot_git_id: usize, scan_id: usize) -> RepositoryEntry { + fn fake_entry(work_dir_id: usize, scan_id: usize) -> RepositoryEntry { RepositoryEntry { scan_id, - dot_git_entry_id: ProjectEntryId(dot_git_id), - work_directory: RepositoryWorkDirectory( - Path::new(&format!("don't-care-{}", scan_id)).into(), - ), + work_directory: ProjectEntryId(work_dir_id).into(), branch: None, } } @@ -3691,25 +3715,25 @@ mod tests { // Deletion retained assert!(res .iter() - .find(|repo| repo.dot_git_entry_id.0 == 1 && repo.scan_id == 0) + .find(|repo| repo.work_directory.0 .0 == 1 && repo.scan_id == 0) .is_some()); // Update retained assert!(res .iter() - .find(|repo| repo.dot_git_entry_id.0 == 2 && repo.scan_id == 1) + .find(|repo| repo.work_directory.0 .0 == 2 && repo.scan_id == 1) .is_some()); // Addition retained assert!(res .iter() - .find(|repo| repo.dot_git_entry_id.0 == 4 && repo.scan_id == 0) + .find(|repo| repo.work_directory.0 .0 == 4 && repo.scan_id == 0) .is_some()); // Nochange, not retained assert!(res .iter() - .find(|repo| repo.dot_git_entry_id.0 == 3 && repo.scan_id == 0) + .find(|repo| repo.work_directory.0 .0 == 3 && repo.scan_id == 0) .is_none()); }