From 22e4086658365324c7984ecf7a44ff74240ebfc4 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 31 May 2023 11:02:59 -0700 Subject: [PATCH 01/29] WIP: Move statuses to be on their associated file entries in worktree co-authored-by: Julia --- Cargo.lock | 1 + crates/collab/src/db.rs | 143 +-------- crates/collab/src/tests/integration_tests.rs | 3 +- crates/fs/Cargo.toml | 2 + crates/fs/src/repository.rs | 21 ++ crates/project/src/project.rs | 12 +- crates/project/src/worktree.rs | 309 +++++-------------- crates/project_panel/src/project_panel.rs | 11 +- crates/rpc/proto/zed.proto | 3 +- crates/rpc/src/rpc.rs | 2 +- 10 files changed, 124 insertions(+), 383 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3751b34d31..4bea44ffee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2438,6 +2438,7 @@ dependencies = [ "parking_lot 0.11.2", "regex", "rope", + "rpc", "serde", "serde_derive", "serde_json", diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index fd28fb9101..e3d553c606 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1537,6 +1537,8 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, + // TODO stream statuses + git_status: None, }); } } @@ -1571,54 +1573,6 @@ impl Database { worktree.updated_repositories.push(proto::RepositoryEntry { work_directory_id: db_repository.work_directory_id as u64, branch: db_repository.branch, - removed_repo_paths: Default::default(), - updated_statuses: Default::default(), - }); - } - } - } - - // Repository Status Entries - for repository in worktree.updated_repositories.iter_mut() { - let repository_status_entry_filter = - if let Some(rejoined_worktree) = rejoined_worktree { - worktree_repository_statuses::Column::ScanId - .gt(rejoined_worktree.scan_id) - } else { - worktree_repository_statuses::Column::IsDeleted.eq(false) - }; - - let mut db_repository_statuses = - worktree_repository_statuses::Entity::find() - .filter( - Condition::all() - .add( - worktree_repository_statuses::Column::ProjectId - .eq(project.id), - ) - .add( - worktree_repository_statuses::Column::WorktreeId - .eq(worktree.id), - ) - .add( - worktree_repository_statuses::Column::WorkDirectoryId - .eq(repository.work_directory_id), - ) - .add(repository_status_entry_filter), - ) - .stream(&*tx) - .await?; - - while let Some(db_status_entry) = db_repository_statuses.next().await { - let db_status_entry = db_status_entry?; - if db_status_entry.is_deleted { - repository - .removed_repo_paths - .push(db_status_entry.repo_path); - } else { - repository.updated_statuses.push(proto::StatusEntry { - repo_path: db_status_entry.repo_path, - status: db_status_entry.status as i32, }); } } @@ -2446,68 +2400,6 @@ impl Database { ) .exec(&*tx) .await?; - - for repository in update.updated_repositories.iter() { - if !repository.updated_statuses.is_empty() { - worktree_repository_statuses::Entity::insert_many( - repository.updated_statuses.iter().map(|status_entry| { - worktree_repository_statuses::ActiveModel { - project_id: ActiveValue::set(project_id), - worktree_id: ActiveValue::set(worktree_id), - work_directory_id: ActiveValue::set( - repository.work_directory_id as i64, - ), - repo_path: ActiveValue::set(status_entry.repo_path.clone()), - status: ActiveValue::set(status_entry.status as i64), - scan_id: ActiveValue::set(update.scan_id as i64), - is_deleted: ActiveValue::set(false), - } - }), - ) - .on_conflict( - OnConflict::columns([ - worktree_repository_statuses::Column::ProjectId, - worktree_repository_statuses::Column::WorktreeId, - worktree_repository_statuses::Column::WorkDirectoryId, - worktree_repository_statuses::Column::RepoPath, - ]) - .update_columns([ - worktree_repository_statuses::Column::ScanId, - worktree_repository_statuses::Column::Status, - worktree_repository_statuses::Column::IsDeleted, - ]) - .to_owned(), - ) - .exec(&*tx) - .await?; - } - - if !repository.removed_repo_paths.is_empty() { - worktree_repository_statuses::Entity::update_many() - .filter( - worktree_repository_statuses::Column::ProjectId - .eq(project_id) - .and( - worktree_repository_statuses::Column::WorktreeId - .eq(worktree_id), - ) - .and( - worktree_repository_statuses::Column::WorkDirectoryId - .eq(repository.work_directory_id as i64), - ) - .and(worktree_repository_statuses::Column::RepoPath.is_in( - repository.removed_repo_paths.iter().map(String::as_str), - )), - ) - .set(worktree_repository_statuses::ActiveModel { - is_deleted: ActiveValue::Set(true), - scan_id: ActiveValue::Set(update.scan_id as i64), - ..Default::default() - }) - .exec(&*tx) - .await?; - } - } } if !update.removed_repositories.is_empty() { @@ -2738,6 +2630,8 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, + // TODO stream statuses + git_status: None, }); } } @@ -2763,41 +2657,12 @@ impl Database { proto::RepositoryEntry { work_directory_id: db_repository_entry.work_directory_id as u64, branch: db_repository_entry.branch, - removed_repo_paths: Default::default(), - updated_statuses: Default::default(), }, ); } } } - { - let mut db_status_entries = worktree_repository_statuses::Entity::find() - .filter( - Condition::all() - .add(worktree_repository_statuses::Column::ProjectId.eq(project_id)) - .add(worktree_repository_statuses::Column::IsDeleted.eq(false)), - ) - .stream(&*tx) - .await?; - - while let Some(db_status_entry) = db_status_entries.next().await { - let db_status_entry = db_status_entry?; - if let Some(worktree) = worktrees.get_mut(&(db_status_entry.worktree_id as u64)) - { - if let Some(repository_entry) = worktree - .repository_entries - .get_mut(&(db_status_entry.work_directory_id as u64)) - { - repository_entry.updated_statuses.push(proto::StatusEntry { - repo_path: db_status_entry.repo_path, - status: db_status_entry.status as i32, - }); - } - } - } - } - // Populate worktree diagnostic summaries. { let mut db_summaries = worktree_diagnostic_summary::Entity::find() diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 53726113db..5f5057478e 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2763,8 +2763,7 @@ async fn test_git_status_sync( assert_eq!(worktrees.len(), 1); let worktree = worktrees[0].clone(); let snapshot = worktree.read(cx).snapshot(); - let root_entry = snapshot.root_git_entry().unwrap(); - assert_eq!(root_entry.status_for_file(&snapshot, file), status); + assert_eq!(snapshot.status_for_file(file), status); } // Smoke test status reading diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 54c6ce362a..7dda3f7273 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -14,6 +14,8 @@ lsp = { path = "../lsp" } rope = { path = "../rope" } util = { path = "../util" } sum_tree = { path = "../sum_tree" } +rpc = { path = "../rpc" } + anyhow.workspace = true async-trait.workspace = true futures.workspace = true diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 2c309351fc..8305d11bb4 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,6 +1,7 @@ use anyhow::Result; use collections::HashMap; use parking_lot::Mutex; +use rpc::proto; use serde_derive::{Deserialize, Serialize}; use std::{ cmp::Ordering, @@ -197,6 +198,26 @@ pub enum GitFileStatus { Conflict, } +impl GitFileStatus { + pub fn from_proto(git_status: Option) -> Option { + git_status.and_then(|status| { + proto::GitStatus::from_i32(status).map(|status| match status { + proto::GitStatus::Added => GitFileStatus::Added, + proto::GitStatus::Modified => GitFileStatus::Modified, + proto::GitStatus::Conflict => GitFileStatus::Conflict, + }) + }) + } + + pub fn to_proto(self) -> i32 { + match self { + GitFileStatus::Added => proto::GitStatus::Added as i32, + GitFileStatus::Modified => proto::GitStatus::Modified as i32, + GitFileStatus::Conflict => proto::GitStatus::Conflict as i32, + } + } +} + #[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] pub struct RepoPath(PathBuf); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fd522c5061..ed30c0ecd4 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5095,9 +5095,9 @@ impl Project { return None; } let path = &project_path.path; - changed_repos.iter().find(|(work_dir, change)| { - path.starts_with(work_dir) && change.git_dir_changed - })?; + changed_repos + .iter() + .find(|(work_dir, _)| path.starts_with(work_dir))?; let receiver = receiver.clone(); let path = path.clone(); Some(async move { @@ -5120,9 +5120,9 @@ impl Project { return None; } let path = file.path(); - changed_repos.iter().find(|(work_dir, change)| { - path.starts_with(work_dir) && change.git_dir_changed - })?; + changed_repos + .iter() + .find(|(work_dir, _)| path.starts_with(work_dir))?; Some((buffer, path.clone())) }) .collect::>(); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index dc3c172775..678b34ebed 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -7,7 +7,7 @@ use client::{proto, Client}; use clock::ReplicaId; use collections::{HashMap, VecDeque}; use fs::{ - repository::{GitFileStatus, GitRepository, RepoPath, RepoPathDescendants}, + repository::{GitFileStatus, GitRepository, RepoPath}, Fs, LineEnding, }; use futures::{ @@ -55,7 +55,7 @@ use std::{ time::{Duration, SystemTime}, }; use sum_tree::{Bias, Edit, SeekTarget, SumTree, TreeMap, TreeSet}; -use util::{paths::HOME, ResultExt, TakeUntilExt}; +use util::{paths::HOME, ResultExt}; #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] pub struct WorktreeId(usize); @@ -124,15 +124,6 @@ pub struct Snapshot { pub struct RepositoryEntry { pub(crate) work_directory: WorkDirectoryEntry, pub(crate) branch: Option>, - pub(crate) statuses: TreeMap, -} - -fn read_git_status(git_status: i32) -> Option { - proto::GitStatus::from_i32(git_status).map(|status| match status { - proto::GitStatus::Added => GitFileStatus::Added, - proto::GitStatus::Modified => GitFileStatus::Modified, - proto::GitStatus::Conflict => GitFileStatus::Conflict, - }) } impl RepositoryEntry { @@ -150,115 +141,19 @@ impl RepositoryEntry { .map(|entry| RepositoryWorkDirectory(entry.path.clone())) } - pub fn status_for_path(&self, snapshot: &Snapshot, path: &Path) -> Option { - self.work_directory - .relativize(snapshot, path) - .and_then(|repo_path| { - self.statuses - .iter_from(&repo_path) - .take_while(|(key, _)| key.starts_with(&repo_path)) - // Short circut once we've found the highest level - .take_until(|(_, status)| status == &&GitFileStatus::Conflict) - .map(|(_, status)| status) - .reduce( - |status_first, status_second| match (status_first, status_second) { - (GitFileStatus::Conflict, _) | (_, GitFileStatus::Conflict) => { - &GitFileStatus::Conflict - } - (GitFileStatus::Modified, _) | (_, GitFileStatus::Modified) => { - &GitFileStatus::Modified - } - _ => &GitFileStatus::Added, - }, - ) - .copied() - }) - } - - #[cfg(any(test, feature = "test-support"))] - pub fn status_for_file(&self, snapshot: &Snapshot, path: &Path) -> Option { - self.work_directory - .relativize(snapshot, path) - .and_then(|repo_path| (&self.statuses).get(&repo_path)) - .cloned() - } - - pub fn build_update(&self, other: &Self) -> proto::RepositoryEntry { - let mut updated_statuses: Vec = Vec::new(); - let mut removed_statuses: Vec = Vec::new(); - - let mut self_statuses = self.statuses.iter().peekable(); - let mut other_statuses = other.statuses.iter().peekable(); - loop { - match (self_statuses.peek(), other_statuses.peek()) { - (Some((self_repo_path, self_status)), Some((other_repo_path, other_status))) => { - match Ord::cmp(self_repo_path, other_repo_path) { - Ordering::Less => { - updated_statuses.push(make_status_entry(self_repo_path, self_status)); - self_statuses.next(); - } - Ordering::Equal => { - if self_status != other_status { - updated_statuses - .push(make_status_entry(self_repo_path, self_status)); - } - - self_statuses.next(); - other_statuses.next(); - } - Ordering::Greater => { - removed_statuses.push(make_repo_path(other_repo_path)); - other_statuses.next(); - } - } - } - (Some((self_repo_path, self_status)), None) => { - updated_statuses.push(make_status_entry(self_repo_path, self_status)); - self_statuses.next(); - } - (None, Some((other_repo_path, _))) => { - removed_statuses.push(make_repo_path(other_repo_path)); - other_statuses.next(); - } - (None, None) => break, - } - } - + pub fn build_update(&self, _: &Self) -> proto::RepositoryEntry { proto::RepositoryEntry { work_directory_id: self.work_directory_id().to_proto(), branch: self.branch.as_ref().map(|str| str.to_string()), - removed_repo_paths: removed_statuses, - updated_statuses, } } } -fn make_repo_path(path: &RepoPath) -> String { - path.as_os_str().to_string_lossy().to_string() -} - -fn make_status_entry(path: &RepoPath, status: &GitFileStatus) -> proto::StatusEntry { - proto::StatusEntry { - repo_path: make_repo_path(path), - status: match status { - GitFileStatus::Added => proto::GitStatus::Added.into(), - GitFileStatus::Modified => proto::GitStatus::Modified.into(), - GitFileStatus::Conflict => proto::GitStatus::Conflict.into(), - }, - } -} - impl From<&RepositoryEntry> for proto::RepositoryEntry { fn from(value: &RepositoryEntry) -> Self { proto::RepositoryEntry { work_directory_id: value.work_directory.to_proto(), branch: value.branch.as_ref().map(|str| str.to_string()), - updated_statuses: value - .statuses - .iter() - .map(|(repo_path, status)| make_status_entry(repo_path, status)) - .collect(), - removed_repo_paths: Default::default(), } } } @@ -330,7 +225,6 @@ pub struct BackgroundScannerState { #[derive(Debug, Clone)] pub struct LocalRepositoryEntry { - pub(crate) work_dir_scan_id: usize, pub(crate) git_dir_scan_id: usize, pub(crate) repo_ptr: Arc>, /// Path to the actual .git folder. @@ -867,18 +761,13 @@ impl LocalWorktree { entry.path.clone(), GitRepositoryChange { old_repository: None, - git_dir_changed: true, }, )); } new_repos.next(); } Ordering::Equal => { - let git_dir_changed = - new_repo.git_dir_scan_id != old_repo.git_dir_scan_id; - let work_dir_changed = - new_repo.work_dir_scan_id != old_repo.work_dir_scan_id; - if git_dir_changed || work_dir_changed { + if new_repo.git_dir_scan_id != old_repo.git_dir_scan_id { if let Some(entry) = new_snapshot.entry_for_id(new_entry_id) { let old_repo = old_snapshot .repository_entries @@ -888,7 +777,6 @@ impl LocalWorktree { entry.path.clone(), GitRepositoryChange { old_repository: old_repo, - git_dir_changed, }, )); } @@ -906,7 +794,6 @@ impl LocalWorktree { entry.path.clone(), GitRepositoryChange { old_repository: old_repo, - git_dir_changed: true, }, )); } @@ -920,7 +807,6 @@ impl LocalWorktree { entry.path.clone(), GitRepositoryChange { old_repository: None, - git_dir_changed: true, }, )); } @@ -936,7 +822,6 @@ impl LocalWorktree { entry.path.clone(), GitRepositoryChange { old_repository: old_repo, - git_dir_changed: true, }, )); } @@ -1587,6 +1472,13 @@ impl Snapshot { Some(removed_entry.path) } + #[cfg(any(test, feature = "test-support"))] + pub fn status_for_file(&self, path: impl Into) -> Option { + self.entries_by_path + .get(&PathKey(Arc::from(path.into())), &()) + .and_then(|entry| entry.git_status) + } + pub(crate) fn apply_remote_update(&mut self, mut update: proto::UpdateWorktree) -> Result<()> { let mut entries_by_path_edits = Vec::new(); let mut entries_by_id_edits = Vec::new(); @@ -1638,26 +1530,10 @@ impl Snapshot { ProjectEntryId::from_proto(repository.work_directory_id).into(); if let Some(entry) = self.entry_for_id(*work_directory_entry) { - let mut statuses = TreeMap::default(); - for status_entry in repository.updated_statuses { - let Some(git_file_status) = read_git_status(status_entry.status) else { - continue; - }; - - let repo_path = RepoPath::new(status_entry.repo_path.into()); - statuses.insert(repo_path, git_file_status); - } - let work_directory = RepositoryWorkDirectory(entry.path.clone()); if self.repository_entries.get(&work_directory).is_some() { self.repository_entries.update(&work_directory, |repo| { repo.branch = repository.branch.map(Into::into); - repo.statuses.insert_tree(statuses); - - for repo_path in repository.removed_repo_paths { - let repo_path = RepoPath::new(repo_path.into()); - repo.statuses.remove(&repo_path); - } }); } else { self.repository_entries.insert( @@ -1665,7 +1541,6 @@ impl Snapshot { RepositoryEntry { work_directory: work_directory_entry, branch: repository.branch.map(Into::into), - statuses, }, ) } @@ -2063,7 +1938,8 @@ impl LocalSnapshot { RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), - statuses: repo_lock.statuses().unwrap_or_default(), + // TODO: statuses + // statuses: repo_lock.statuses().unwrap_or_default(), }, ); drop(repo_lock); @@ -2071,7 +1947,6 @@ impl LocalSnapshot { self.git_repositories.insert( work_dir_id, LocalRepositoryEntry { - work_dir_scan_id: scan_id, git_dir_scan_id: scan_id, repo_ptr: repo, git_dir_path: parent_path.clone(), @@ -2498,6 +2373,7 @@ pub struct Entry { pub mtime: SystemTime, pub is_symlink: bool, pub is_ignored: bool, + pub git_status: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -2526,9 +2402,6 @@ pub enum PathChange { pub struct GitRepositoryChange { /// The previous state of the repository, if it already existed. pub old_repository: Option, - /// Whether the content of the .git directory changed. This will be false - /// if only the repository's work directory changed. - pub git_dir_changed: bool, } pub type UpdatedEntriesSet = Arc<[(Arc, ProjectEntryId, PathChange)]>; @@ -2553,6 +2426,7 @@ impl Entry { mtime: metadata.mtime, is_symlink: metadata.is_symlink, is_ignored: false, + git_status: None, } } @@ -3199,8 +3073,6 @@ impl BackgroundScanner { .components() .any(|component| component.as_os_str() == *DOT_GIT) { - let scan_id = snapshot.scan_id; - if let Some(repository) = snapshot.repository_for_work_directory(path) { let entry = repository.work_directory.0; snapshot.git_repositories.remove(&entry); @@ -3210,23 +3082,11 @@ impl BackgroundScanner { .remove(&RepositoryWorkDirectory(path.into())); return Some(()); } - - let repo = snapshot.repository_for_path(&path)?; - let repo_path = repo.work_directory.relativize(&snapshot, &path)?; - let work_dir = repo.work_directory(snapshot)?; - let work_dir_id = repo.work_directory; - - snapshot - .git_repositories - .update(&work_dir_id, |entry| entry.work_dir_scan_id = scan_id); - - snapshot.repository_entries.update(&work_dir, |entry| { - entry - .statuses - .remove_range(&repo_path, &RepoPathDescendants(&repo_path)) - }); } + // TODO statuses + // Track when a .git is removed and iterate over the file system there + Some(()) } @@ -3264,35 +3124,46 @@ impl BackgroundScanner { let repo = repo_ptr.lock(); repo.reload_index(); let branch = repo.branch_name(); - let statuses = repo.statuses().unwrap_or_default(); snapshot.git_repositories.update(&entry_id, |entry| { - entry.work_dir_scan_id = scan_id; entry.git_dir_scan_id = scan_id; }); - snapshot.repository_entries.update(&work_dir, |entry| { - entry.branch = branch.map(Into::into); - entry.statuses = statuses; - }); + snapshot + .snapshot + .repository_entries + .update(&work_dir, |entry| { + entry.branch = branch.map(Into::into); + }); + + let statuses = repo.statuses().unwrap_or_default(); + for (repo_path, status) in statuses.iter() { + let Some(entry) = snapshot.entry_for_path(&work_dir.0.join(repo_path)) else { + continue; + }; + + let mut entry = entry.clone(); + entry.git_status = Some(*status); + + // TODO statuses + // Bubble + + snapshot.entries_by_path.insert_or_replace(entry, &()); + } } else { if snapshot .entry_for_path(&path) .map(|entry| entry.is_ignored) .unwrap_or(false) { - self.remove_repo_path(&path, snapshot); return None; } let repo = snapshot.repository_for_path(&path)?; - let work_dir = repo.work_directory(snapshot)?; let work_dir_id = repo.work_directory.clone(); - let (local_repo, git_dir_scan_id) = snapshot.git_repositories.update(&work_dir_id, |entry| { - entry.work_dir_scan_id = scan_id; (entry.repo_ptr.clone(), entry.git_dir_scan_id) })?; @@ -3301,22 +3172,25 @@ impl BackgroundScanner { return None; } - let mut repository = snapshot.repository_entries.remove(&work_dir)?; - - for entry in snapshot.descendent_entries(false, false, path) { + for mut entry in snapshot + .descendent_entries(false, false, path) + .cloned() + .collect::>() + .into_iter() + { let Some(repo_path) = repo.work_directory.relativize(snapshot, &entry.path) else { continue; }; - let status = local_repo.lock().status(&repo_path); - if let Some(status) = status { - repository.statuses.insert(repo_path.clone(), status); - } else { - repository.statuses.remove(&repo_path); - } - } + let Some(status) = local_repo.lock().status(&repo_path) else { + continue; + }; - snapshot.repository_entries.insert(work_dir, repository) + entry.git_status = Some(status); + snapshot.entries_by_path.insert_or_replace(entry, &()); + // TODO statuses + // Bubble + } } Some(()) @@ -3831,6 +3705,7 @@ impl<'a> From<&'a Entry> for proto::Entry { mtime: Some(entry.mtime.into()), is_symlink: entry.is_symlink, is_ignored: entry.is_ignored, + git_status: entry.git_status.map(|status| status.to_proto()), } } } @@ -3856,6 +3731,7 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry { mtime: mtime.into(), is_symlink: entry.is_symlink, is_ignored: entry.is_ignored, + git_status: GitFileStatus::from_proto(entry.git_status), }) } else { Err(anyhow!( @@ -4911,14 +4787,14 @@ mod tests { cx.read(|cx| { let tree = tree.read(cx); - let (work_dir, repo) = tree.repositories().next().unwrap(); + let (work_dir, _) = tree.repositories().next().unwrap(); assert_eq!(work_dir.as_ref(), Path::new("projects/project1")); assert_eq!( - repo.status_for_file(tree, Path::new("projects/project1/a")), + tree.status_for_file(Path::new("projects/project1/a")), Some(GitFileStatus::Modified) ); assert_eq!( - repo.status_for_file(tree, Path::new("projects/project1/b")), + tree.status_for_file(Path::new("projects/project1/b")), Some(GitFileStatus::Added) ); }); @@ -4932,14 +4808,14 @@ mod tests { cx.read(|cx| { let tree = tree.read(cx); - let (work_dir, repo) = tree.repositories().next().unwrap(); + let (work_dir, _) = tree.repositories().next().unwrap(); assert_eq!(work_dir.as_ref(), Path::new("projects/project2")); assert_eq!( - repo.status_for_file(tree, Path::new("projects/project2/a")), + tree.status_for_file(Path::new("projects/project2/a")), Some(GitFileStatus::Modified) ); assert_eq!( - repo.status_for_file(tree, Path::new("projects/project2/b")), + tree.status_for_file(Path::new("projects/project2/b")), Some(GitFileStatus::Added) ); }); @@ -5067,6 +4943,7 @@ mod tests { }); } + // TODO: Stream statuses UPDATE THIS TO CHECK BUBBLIBG BEHAVIOR #[gpui::test] async fn test_git_status(cx: &mut TestAppContext) { const IGNORE_RULE: &'static str = "**/target"; @@ -5111,6 +4988,7 @@ mod tests { const F_TXT: &'static str = "f.txt"; const DOTGITIGNORE: &'static str = ".gitignore"; const BUILD_FILE: &'static str = "target/build_file"; + let project_path: &Path = &Path::new("project"); let work_dir = root.path().join("project"); let mut repo = git_init(work_dir.as_path()); @@ -5128,21 +5006,20 @@ mod tests { tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); assert_eq!(snapshot.repository_entries.iter().count(), 1); - let (dir, repo) = snapshot.repository_entries.iter().next().unwrap(); + let (dir, _) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); - assert_eq!(repo.statuses.iter().count(), 3); assert_eq!( - repo.statuses.get(&Path::new(A_TXT).into()), - Some(&GitFileStatus::Modified) + snapshot.status_for_file(project_path.join(A_TXT)), + Some(GitFileStatus::Modified) ); assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(project_path.join(B_TXT)), + Some(GitFileStatus::Added) ); assert_eq!( - repo.statuses.get(&Path::new(F_TXT).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(project_path.join(F_TXT)), + Some(GitFileStatus::Added) ); }); @@ -5154,12 +5031,10 @@ mod tests { // Check that repo only changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.statuses.iter().count(), 1); assert_eq!( - repo.statuses.get(&Path::new(F_TXT).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(project_path.join(F_TXT)), + Some(GitFileStatus::Added) ); }); @@ -5173,21 +5048,19 @@ mod tests { // Check that more complex repo changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.statuses.iter().count(), 3); - assert_eq!(repo.statuses.get(&Path::new(A_TXT).into()), None); + assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None); assert_eq!( - repo.statuses.get(&Path::new(B_TXT).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(project_path.join(B_TXT)), + Some(GitFileStatus::Added) ); assert_eq!( - repo.statuses.get(&Path::new(E_TXT).into()), - Some(&GitFileStatus::Modified) + snapshot.status_for_file(project_path.join(E_TXT)), + Some(GitFileStatus::Modified) ); assert_eq!( - repo.statuses.get(&Path::new(F_TXT).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(project_path.join(F_TXT)), + Some(GitFileStatus::Added) ); }); @@ -5204,14 +5077,6 @@ mod tests { tree.flush_fs_events(cx).await; - // Check that non-repo behavior is tracked - tree.read_with(cx, |tree, _cx| { - let snapshot = tree.snapshot(); - let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - - assert_eq!(repo.statuses.iter().count(), 0); - }); - let mut renamed_dir_name = "first_directory/second_directory"; const RENAMED_FILE: &'static str = "rf.txt"; @@ -5226,13 +5091,10 @@ mod tests { tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - - assert_eq!(repo.statuses.iter().count(), 1); assert_eq!( - repo.statuses - .get(&Path::new(renamed_dir_name).join(RENAMED_FILE).into()), - Some(&GitFileStatus::Added) + snapshot + .status_for_file(&project_path.join(renamed_dir_name).join(RENAMED_FILE)), + Some(GitFileStatus::Added) ); }); @@ -5248,13 +5110,10 @@ mod tests { tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); - let (_, repo) = snapshot.repository_entries.iter().next().unwrap(); - assert_eq!(repo.statuses.iter().count(), 1); assert_eq!( - repo.statuses - .get(&Path::new(renamed_dir_name).join(RENAMED_FILE).into()), - Some(&GitFileStatus::Added) + snapshot.status_for_file(Path::new(renamed_dir_name).join(RENAMED_FILE)), + Some(GitFileStatus::Added) ); }); } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 60af95174f..f6f3d67f3a 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1002,6 +1002,7 @@ impl ProjectPanel { mtime: entry.mtime, is_symlink: false, is_ignored: false, + git_status: entry.git_status, }); } if expanded_dir_ids.binary_search(&entry.id).is_err() @@ -1108,14 +1109,8 @@ impl ProjectPanel { .unwrap_or(&[]); let entry_range = range.start.saturating_sub(ix)..end_ix - ix; - for (entry, repo) in - snapshot.entries_with_repositories(visible_worktree_entries[entry_range].iter()) - { - let status = (git_status_setting - && entry.path.parent().is_some() - && !entry.is_ignored) - .then(|| repo.and_then(|repo| repo.status_for_path(&snapshot, &entry.path))) - .flatten(); + for entry in visible_worktree_entries[entry_range].iter() { + let status = git_status_setting.then(|| entry.git_status).flatten(); let mut details = EntryDetails { filename: entry diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 848cc1c2fa..001d503875 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -996,13 +996,12 @@ message Entry { Timestamp mtime = 5; bool is_symlink = 6; bool is_ignored = 7; + optional GitStatus git_status = 8; } message RepositoryEntry { uint64 work_directory_id = 1; optional string branch = 2; - repeated string removed_repo_paths = 3; - repeated StatusEntry updated_statuses = 4; } message StatusEntry { diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index b929de9596..bef6efa529 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 56; +pub const PROTOCOL_VERSION: u32 = 57; From 4717ce1da380c4b17c25dc637730d393c36b8a50 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 31 May 2023 12:55:31 -0700 Subject: [PATCH 02/29] WIP: Move statuses to entries co-authored-by: julia --- crates/project/src/worktree.rs | 60 ++++++++++++++++++++++------------ crates/rpc/src/proto.rs | 5 +++ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 678b34ebed..3f4df1328b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1474,8 +1474,9 @@ impl Snapshot { #[cfg(any(test, feature = "test-support"))] pub fn status_for_file(&self, path: impl Into) -> Option { + let path = path.into(); self.entries_by_path - .get(&PathKey(Arc::from(path.into())), &()) + .get(&PathKey(Arc::from(path)), &()) .and_then(|entry| entry.git_status) } @@ -1929,25 +1930,25 @@ impl LocalSnapshot { if self.git_repositories.get(&work_dir_id).is_none() { let repo = fs.open_repo(abs_path.as_path())?; let work_directory = RepositoryWorkDirectory(work_dir.clone()); - let scan_id = self.scan_id; let repo_lock = repo.lock(); self.repository_entries.insert( - work_directory, + work_directory.clone(), RepositoryEntry { work_directory: work_dir_id.into(), branch: repo_lock.branch_name().map(Into::into), - // TODO: statuses - // statuses: repo_lock.statuses().unwrap_or_default(), }, ); + + self.scan_statuses(repo_lock.deref(), &work_directory); + drop(repo_lock); self.git_repositories.insert( work_dir_id, LocalRepositoryEntry { - git_dir_scan_id: scan_id, + git_dir_scan_id: 0, repo_ptr: repo, git_dir_path: parent_path.clone(), }, @@ -1957,6 +1958,23 @@ impl LocalSnapshot { Some(()) } + fn scan_statuses(&mut self, repo_ptr: &dyn GitRepository, work_directory: &RepositoryWorkDirectory) { + let statuses = repo_ptr.statuses().unwrap_or_default(); + for (repo_path, status) in statuses.iter() { + let Some(entry) = self.entry_for_path(&work_directory.0.join(repo_path)) else { + continue; + }; + + let mut entry = entry.clone(); + entry.git_status = Some(*status); + + // TODO statuses + // Bubble + + self.entries_by_path.insert_or_replace(entry, &()); + } + } + fn ancestor_inodes_for_path(&self, path: &Path) -> TreeSet { let mut inodes = TreeSet::default(); for ancestor in path.ancestors().skip(1) { @@ -3114,8 +3132,15 @@ impl BackgroundScanner { if repo.git_dir_scan_id == scan_id { return None; } + (*entry_id, repo.repo_ptr.to_owned()) }; + /* + 1. Populate dir, initializes the git repo + 2. Sometimes, we get a file event inside the .git repo, before it's initializaed + In both cases, we should end up with an initialized repo and a full status scan + + */ let work_dir = snapshot .entry_for_id(entry_id) @@ -3136,20 +3161,8 @@ impl BackgroundScanner { entry.branch = branch.map(Into::into); }); - let statuses = repo.statuses().unwrap_or_default(); - for (repo_path, status) in statuses.iter() { - let Some(entry) = snapshot.entry_for_path(&work_dir.0.join(repo_path)) else { - continue; - }; - let mut entry = entry.clone(); - entry.git_status = Some(*status); - - // TODO statuses - // Bubble - - snapshot.entries_by_path.insert_or_replace(entry, &()); - } + snapshot.scan_statuses(repo.deref(), &work_dir); } else { if snapshot .entry_for_path(&path) @@ -4945,7 +4958,7 @@ mod tests { // TODO: Stream statuses UPDATE THIS TO CHECK BUBBLIBG BEHAVIOR #[gpui::test] - async fn test_git_status(cx: &mut TestAppContext) { + async fn test_git_status(deterministic: Arc, cx: &mut TestAppContext) { const IGNORE_RULE: &'static str = "**/target"; let root = temp_tree(json!({ @@ -4982,6 +4995,7 @@ mod tests { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; + const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; const E_TXT: &'static str = "c/d/e.txt"; @@ -5001,6 +5015,7 @@ mod tests { std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); // Check that the right git state is observed on startup tree.read_with(cx, |tree, _cx| { @@ -5027,6 +5042,7 @@ mod tests { git_add(Path::new(B_TXT), &repo); git_commit("Committing modified and added", &repo); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); // Check that repo only changes are tracked tree.read_with(cx, |tree, _cx| { @@ -5044,6 +5060,7 @@ mod tests { std::fs::write(work_dir.join(E_TXT), "eeee").unwrap(); std::fs::write(work_dir.join(BUILD_FILE), "this should be ignored").unwrap(); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); // Check that more complex repo changes are tracked tree.read_with(cx, |tree, _cx| { @@ -5076,6 +5093,7 @@ mod tests { git_commit("Committing modified git ignore", &repo); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); let mut renamed_dir_name = "first_directory/second_directory"; const RENAMED_FILE: &'static str = "rf.txt"; @@ -5088,6 +5106,7 @@ mod tests { .unwrap(); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); @@ -5107,6 +5126,7 @@ mod tests { .unwrap(); tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 07925a0486..814b40403f 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -497,6 +497,11 @@ pub fn split_worktree_update( .map(|repo| (repo.work_directory_id, repo)) .collect::>(); + // Maintain a list of inflight repositories + // Every time we send a repository's work directory, stick it in the list of in-flight repositories + // Every go of the loop, drain each in-flight repository's statuses + // Until we have no more data + iter::from_fn(move || { if done_files { return None; From 5e43dcaab8abaf206364bd2ba873d6c83c070f14 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 1 Jun 2023 16:51:34 -0700 Subject: [PATCH 03/29] WIP: Add stateful status bubbling to worktree --- crates/fs/src/repository.rs | 18 +++++ crates/project/src/worktree.rs | 119 +++++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 27 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 8305d11bb4..2169baa724 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -199,6 +199,24 @@ pub enum GitFileStatus { } impl GitFileStatus { + pub fn merge( + this: Option, + other: Option, + ) -> Option { + match (this, other) { + (Some(GitFileStatus::Conflict), _) | (_, Some(GitFileStatus::Conflict)) => { + Some(GitFileStatus::Conflict) + } + (Some(GitFileStatus::Modified), _) | (_, Some(GitFileStatus::Modified)) => { + Some(GitFileStatus::Modified) + } + (Some(GitFileStatus::Added), _) | (_, Some(GitFileStatus::Added)) => { + Some(GitFileStatus::Added) + } + _ => None, + } + } + pub fn from_proto(git_status: Option) -> Option { git_status.and_then(|status| { proto::GitStatus::from_i32(status).map(|status| match status { diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 3f4df1328b..104e428602 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1958,20 +1958,32 @@ impl LocalSnapshot { Some(()) } - fn scan_statuses(&mut self, repo_ptr: &dyn GitRepository, work_directory: &RepositoryWorkDirectory) { + fn scan_statuses( + &mut self, + repo_ptr: &dyn GitRepository, + work_directory: &RepositoryWorkDirectory, + ) { let statuses = repo_ptr.statuses().unwrap_or_default(); + let mut edits = vec![]; for (repo_path, status) in statuses.iter() { - let Some(entry) = self.entry_for_path(&work_directory.0.join(repo_path)) else { + self.set_git_status(&work_directory.0.join(repo_path), Some(*status), &mut edits); + } + self.entries_by_path.edit(edits, &()); + } + + fn set_git_status( + &self, + path: &Path, + status: Option, + edits: &mut Vec>, + ) { + for path in path.ancestors() { + let Some(entry) = self.entry_for_path(path) else { continue; }; - let mut entry = entry.clone(); - entry.git_status = Some(*status); - - // TODO statuses - // Bubble - - self.entries_by_path.insert_or_replace(entry, &()); + entry.git_status = GitFileStatus::merge(entry.git_status, status); + edits.push(Edit::Insert(entry)) } } @@ -3161,7 +3173,6 @@ impl BackgroundScanner { entry.branch = branch.map(Into::into); }); - snapshot.scan_statuses(repo.deref(), &work_dir); } else { if snapshot @@ -3185,13 +3196,13 @@ impl BackgroundScanner { return None; } - for mut entry in snapshot + let mut edits = vec![]; + + for path in snapshot .descendent_entries(false, false, path) - .cloned() - .collect::>() - .into_iter() + .map(|entry| &entry.path) { - let Some(repo_path) = repo.work_directory.relativize(snapshot, &entry.path) else { + let Some(repo_path) = repo.work_directory.relativize(snapshot, &path) else { continue; }; @@ -3199,11 +3210,10 @@ impl BackgroundScanner { continue; }; - entry.git_status = Some(status); - snapshot.entries_by_path.insert_or_replace(entry, &()); - // TODO statuses - // Bubble + snapshot.set_git_status(&path, Some(status), &mut edits); } + + snapshot.entries_by_path.edit(edits, &()); } Some(()) @@ -4995,7 +5005,6 @@ mod tests { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; - const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; const E_TXT: &'static str = "c/d/e.txt"; @@ -5012,8 +5021,6 @@ mod tests { git_add(Path::new(DOTGITIGNORE), &repo); git_commit("Initial commit", &repo); - std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); - tree.flush_fs_events(cx).await; deterministic.run_until_parked(); @@ -5024,10 +5031,6 @@ mod tests { let (dir, _) = snapshot.repository_entries.iter().next().unwrap(); assert_eq!(dir.0.as_ref(), Path::new("project")); - assert_eq!( - snapshot.status_for_file(project_path.join(A_TXT)), - Some(GitFileStatus::Modified) - ); assert_eq!( snapshot.status_for_file(project_path.join(B_TXT)), Some(GitFileStatus::Added) @@ -5036,6 +5039,36 @@ mod tests { snapshot.status_for_file(project_path.join(F_TXT)), Some(GitFileStatus::Added) ); + + // Check stateful bubbling works + assert_eq!( + snapshot.status_for_file(project_path), + Some(GitFileStatus::Added) + ); + + assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Added)); + }); + + std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); + + tree.flush_fs_events(cx).await; + deterministic.run_until_parked(); + + tree.read_with(cx, |tree, _cx| { + let snapshot = tree.snapshot(); + + assert_eq!( + snapshot.status_for_file(project_path.join(A_TXT)), + Some(GitFileStatus::Modified) + ); + + // Check stateful bubbling works, modified overrules added + assert_eq!( + snapshot.status_for_file(project_path), + Some(GitFileStatus::Modified) + ); + + assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Modified)); }); git_add(Path::new(A_TXT), &repo); @@ -5052,6 +5085,17 @@ mod tests { snapshot.status_for_file(project_path.join(F_TXT)), Some(GitFileStatus::Added) ); + + assert_eq!(snapshot.status_for_file(project_path.join(B_TXT)), None); + assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None); + + // Check bubbling + assert_eq!( + snapshot.status_for_file(project_path), + Some(GitFileStatus::Added) + ); + + assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Added)); }); git_reset(0, &repo); @@ -5075,6 +5119,23 @@ mod tests { snapshot.status_for_file(project_path.join(E_TXT)), Some(GitFileStatus::Modified) ); + + // Check status bubbling + assert_eq!( + snapshot.status_for_file(project_path.join(Path::new(E_TXT).parent().unwrap())), + Some(GitFileStatus::Modified) + ); + assert_eq!( + snapshot.status_for_file( + project_path.join(Path::new(E_TXT).parent().unwrap().parent().unwrap()) + ), + Some(GitFileStatus::Modified) + ); + assert_eq!( + snapshot.status_for_file(project_path), + Some(GitFileStatus::Modified) + ); + assert_eq!( snapshot.status_for_file(project_path.join(F_TXT)), Some(GitFileStatus::Added) @@ -5132,7 +5193,11 @@ mod tests { let snapshot = tree.snapshot(); assert_eq!( - snapshot.status_for_file(Path::new(renamed_dir_name).join(RENAMED_FILE)), + snapshot.status_for_file( + project_path + .join(Path::new(renamed_dir_name)) + .join(RENAMED_FILE) + ), Some(GitFileStatus::Added) ); }); From 3768851799689ceff1c1858735ae18229a406954 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 1 Jun 2023 23:27:49 -0700 Subject: [PATCH 04/29] WIP: Git statuses --- crates/fs/src/repository.rs | 2 +- crates/project/src/worktree.rs | 83 ++++++++++++++++++++++------------ 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 2169baa724..63005a9974 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -237,7 +237,7 @@ impl GitFileStatus { } #[derive(Clone, Debug, Ord, Hash, PartialOrd, Eq, PartialEq)] -pub struct RepoPath(PathBuf); +pub struct RepoPath(pub PathBuf); impl RepoPath { pub fn new(path: PathBuf) -> Self { diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 104e428602..6297d16857 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1941,7 +1941,7 @@ impl LocalSnapshot { }, ); - self.scan_statuses(repo_lock.deref(), &work_directory); + self.scan_statuses(repo_lock.deref(), &work_directory, &work_directory.0); drop(repo_lock); @@ -1962,12 +1962,28 @@ impl LocalSnapshot { &mut self, repo_ptr: &dyn GitRepository, work_directory: &RepositoryWorkDirectory, + path: &Path, ) { - let statuses = repo_ptr.statuses().unwrap_or_default(); let mut edits = vec![]; - for (repo_path, status) in statuses.iter() { - self.set_git_status(&work_directory.0.join(repo_path), Some(*status), &mut edits); + + for path in self + .descendent_entries(false, false, path) + .map(|entry| &entry.path) + { + dbg!("******Starting scan*******"); + dbg!(path); + let Ok(repo_path) = path.strip_prefix(&work_directory.0) else { + continue; + }; + + self.set_git_status( + &path, + dbg!(repo_ptr.status(&RepoPath(repo_path.into()))), + &mut edits, + ); } + + // let statuses = repo_ptr.statuses().unwrap_or_default(); self.entries_by_path.edit(edits, &()); } @@ -1978,12 +1994,31 @@ impl LocalSnapshot { edits: &mut Vec>, ) { for path in path.ancestors() { - let Some(entry) = self.entry_for_path(path) else { - continue; - }; - let mut entry = entry.clone(); - entry.git_status = GitFileStatus::merge(entry.git_status, status); - edits.push(Edit::Insert(entry)) + dbg!(path); + + let search = edits.binary_search_by_key(&path, |edit| match edit { + Edit::Insert(item) => &item.path, + _ => unreachable!(), + }); + + match search { + Ok(idx) => match &mut edits[idx] { + Edit::Insert(item) => { + item.git_status = GitFileStatus::merge(item.git_status, status) + } + _ => unreachable!(), + }, + Err(idx) => { + let Some(entry) = self.entry_for_path(path) else { + continue; + }; + + let mut entry = entry.clone(); + entry.git_status = dbg!(GitFileStatus::merge(dbg!(entry.git_status), status)); + + edits.insert(idx, Edit::Insert(entry)) + } + } } } @@ -3173,7 +3208,7 @@ impl BackgroundScanner { entry.branch = branch.map(Into::into); }); - snapshot.scan_statuses(repo.deref(), &work_dir); + snapshot.scan_statuses(repo.deref(), &work_dir, &work_dir.0); } else { if snapshot .entry_for_path(&path) @@ -3184,7 +3219,7 @@ impl BackgroundScanner { } let repo = snapshot.repository_for_path(&path)?; - + let work_directory = &repo.work_directory(snapshot)?; let work_dir_id = repo.work_directory.clone(); let (local_repo, git_dir_scan_id) = snapshot.git_repositories.update(&work_dir_id, |entry| { @@ -3196,24 +3231,9 @@ impl BackgroundScanner { return None; } - let mut edits = vec![]; + let repo_ptr = local_repo.lock(); - for path in snapshot - .descendent_entries(false, false, path) - .map(|entry| &entry.path) - { - let Some(repo_path) = repo.work_directory.relativize(snapshot, &path) else { - continue; - }; - - let Some(status) = local_repo.lock().status(&repo_path) else { - continue; - }; - - snapshot.set_git_status(&path, Some(status), &mut edits); - } - - snapshot.entries_by_path.edit(edits, &()); + snapshot.scan_statuses(repo_ptr.deref(), &work_directory, path); } Some(()) @@ -5040,9 +5060,11 @@ mod tests { Some(GitFileStatus::Added) ); + dbg!(snapshot.entries(false).collect::>()); + // Check stateful bubbling works assert_eq!( - snapshot.status_for_file(project_path), + snapshot.status_for_file(dbg!(project_path)), Some(GitFileStatus::Added) ); @@ -5077,6 +5099,7 @@ mod tests { tree.flush_fs_events(cx).await; deterministic.run_until_parked(); + dbg!(git_status(&repo)); // Check that repo only changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); From 99a0e11e70b5c2eadcce6665bd3129ea5cfd2a43 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 2 Jun 2023 14:51:40 -0700 Subject: [PATCH 05/29] Abandoning stateful bubbling approach co-authored-by: max --- crates/fs/src/repository.rs | 25 +++++++++------ crates/project/src/worktree.rs | 58 +++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 63005a9974..70f2916681 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -202,18 +202,23 @@ impl GitFileStatus { pub fn merge( this: Option, other: Option, + prefer_other: bool, ) -> Option { - match (this, other) { - (Some(GitFileStatus::Conflict), _) | (_, Some(GitFileStatus::Conflict)) => { - Some(GitFileStatus::Conflict) + if prefer_other { + return other; + } else { + match (this, other) { + (Some(GitFileStatus::Conflict), _) | (_, Some(GitFileStatus::Conflict)) => { + Some(GitFileStatus::Conflict) + } + (Some(GitFileStatus::Modified), _) | (_, Some(GitFileStatus::Modified)) => { + Some(GitFileStatus::Modified) + } + (Some(GitFileStatus::Added), _) | (_, Some(GitFileStatus::Added)) => { + Some(GitFileStatus::Added) + } + _ => None, } - (Some(GitFileStatus::Modified), _) | (_, Some(GitFileStatus::Modified)) => { - Some(GitFileStatus::Modified) - } - (Some(GitFileStatus::Added), _) | (_, Some(GitFileStatus::Added)) => { - Some(GitFileStatus::Added) - } - _ => None, } } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 6297d16857..3fbac1fa5d 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1670,6 +1670,17 @@ impl Snapshot { }) } + pub fn statuses_for_directories(&self, paths: &[&Path]) -> Vec { + // ["/a/b", "a/b/c", "a/b/d", "j"] + + // Path stack: + // If path has descendents following it, push to stack: ["a/b"] + // Figure out a/b/c + // Figure out a/b/d + // Once no more descendants, pop the stack: + // Figure out a/b + } + pub fn paths(&self) -> impl Iterator> { let empty_path = Path::new(""); self.entries_by_path @@ -1965,20 +1976,20 @@ impl LocalSnapshot { path: &Path, ) { let mut edits = vec![]; + let prefer_repo = work_directory.0.deref() == path; for path in self .descendent_entries(false, false, path) .map(|entry| &entry.path) { - dbg!("******Starting scan*******"); - dbg!(path); let Ok(repo_path) = path.strip_prefix(&work_directory.0) else { continue; }; self.set_git_status( &path, - dbg!(repo_ptr.status(&RepoPath(repo_path.into()))), + repo_ptr.status(&RepoPath(repo_path.into())), + prefer_repo, &mut edits, ); } @@ -1991,6 +2002,7 @@ impl LocalSnapshot { &self, path: &Path, status: Option, + prefer_repo: bool, edits: &mut Vec>, ) { for path in path.ancestors() { @@ -2004,7 +2016,7 @@ impl LocalSnapshot { match search { Ok(idx) => match &mut edits[idx] { Edit::Insert(item) => { - item.git_status = GitFileStatus::merge(item.git_status, status) + item.git_status = GitFileStatus::merge(item.git_status, status, prefer_repo) } _ => unreachable!(), }, @@ -2014,7 +2026,11 @@ impl LocalSnapshot { }; let mut entry = entry.clone(); - entry.git_status = dbg!(GitFileStatus::merge(dbg!(entry.git_status), status)); + entry.git_status = dbg!(GitFileStatus::merge( + dbg!(entry.git_status), + status, + prefer_repo + )); edits.insert(idx, Edit::Insert(entry)) } @@ -2429,6 +2445,29 @@ impl File { } } +#[derive(Clone, Debug, PartialEq, Eq, Default)] +struct GitStatuses { + added: usize, + modified: usize, + conflict: usize, +} + +impl GitStatuses { + fn status(&self) -> Option { + if self.conflict > 0 { + Some(GitFileStatus::Conflict) + } else if self.modified > 0 { + Some(GitFileStatus::Modified) + } else if self.added > 0 { + Some(GitFileStatus::Added) + } else { + None + } + } + + fn add_status(&self, status: Option) {} +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Entry { pub id: ProjectEntryId, @@ -2502,6 +2541,10 @@ impl Entry { pub fn is_file(&self) -> bool { matches!(self.kind, EntryKind::File(_)) } + + pub fn git_status(&self) -> Option { + self.git_status /*.status() */ + } } impl sum_tree::Item for Entry { @@ -2544,6 +2587,9 @@ pub struct EntrySummary { visible_count: usize, file_count: usize, visible_file_count: usize, + // git_modified_count: usize, + // git_added_count: usize, + // git_conflict_count: usize, } impl Default for EntrySummary { @@ -5109,6 +5155,8 @@ mod tests { Some(GitFileStatus::Added) ); + dbg!(snapshot.entries(false).collect::>()); + assert_eq!(snapshot.status_for_file(project_path.join(B_TXT)), None); assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None); From e377459948d99bef92e87ca7c484cc330980d997 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 2 Jun 2023 15:07:49 -0700 Subject: [PATCH 06/29] Remove stateful bubbling co-authored-by: max --- crates/project/src/worktree.rs | 144 +++------------------------------ 1 file changed, 12 insertions(+), 132 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 3fbac1fa5d..9b7efd56d1 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -36,6 +36,7 @@ use postage::{ prelude::{Sink as _, Stream as _}, watch, }; +use sha2::digest::typenum::private::IsLessPrivate; use smol::channel::{self, Sender}; use std::{ any::Any, @@ -1671,6 +1672,7 @@ impl Snapshot { } pub fn statuses_for_directories(&self, paths: &[&Path]) -> Vec { + todo!(); // ["/a/b", "a/b/c", "a/b/d", "j"] // Path stack: @@ -1976,68 +1978,19 @@ impl LocalSnapshot { path: &Path, ) { let mut edits = vec![]; - let prefer_repo = work_directory.0.deref() == path; - - for path in self - .descendent_entries(false, false, path) - .map(|entry| &entry.path) - { - let Ok(repo_path) = path.strip_prefix(&work_directory.0) else { + for mut entry in self.descendent_entries(false, false, path).cloned() { + let Ok(repo_path) = entry.path.strip_prefix(&work_directory.0) else { continue; }; - - self.set_git_status( - &path, - repo_ptr.status(&RepoPath(repo_path.into())), - prefer_repo, - &mut edits, - ); + let git_file_status = repo_ptr.status(&RepoPath(repo_path.into())); + let status = git_file_status; + entry.git_status = status; + edits.push(Edit::Insert(entry)); } - // let statuses = repo_ptr.statuses().unwrap_or_default(); self.entries_by_path.edit(edits, &()); } - fn set_git_status( - &self, - path: &Path, - status: Option, - prefer_repo: bool, - edits: &mut Vec>, - ) { - for path in path.ancestors() { - dbg!(path); - - let search = edits.binary_search_by_key(&path, |edit| match edit { - Edit::Insert(item) => &item.path, - _ => unreachable!(), - }); - - match search { - Ok(idx) => match &mut edits[idx] { - Edit::Insert(item) => { - item.git_status = GitFileStatus::merge(item.git_status, status, prefer_repo) - } - _ => unreachable!(), - }, - Err(idx) => { - let Some(entry) = self.entry_for_path(path) else { - continue; - }; - - let mut entry = entry.clone(); - entry.git_status = dbg!(GitFileStatus::merge( - dbg!(entry.git_status), - status, - prefer_repo - )); - - edits.insert(idx, Edit::Insert(entry)) - } - } - } - } - fn ancestor_inodes_for_path(&self, path: &Path) -> TreeSet { let mut inodes = TreeSet::default(); for ancestor in path.ancestors().skip(1) { @@ -2126,10 +2079,6 @@ impl BackgroundScannerState { .insert(abs_parent_path, (ignore, false)); } - if parent_path.file_name() == Some(&DOT_GIT) { - self.snapshot.build_repo(parent_path, fs); - } - let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; let mut entries_by_id_edits = Vec::new(); @@ -2148,6 +2097,10 @@ impl BackgroundScannerState { .entries_by_path .edit(entries_by_path_edits, &()); self.snapshot.entries_by_id.edit(entries_by_id_edits, &()); + + if parent_path.file_name() == Some(&DOT_GIT) { + self.snapshot.build_repo(parent_path, fs); + } } fn remove_path(&mut self, path: &Path) { @@ -2445,29 +2398,6 @@ impl File { } } -#[derive(Clone, Debug, PartialEq, Eq, Default)] -struct GitStatuses { - added: usize, - modified: usize, - conflict: usize, -} - -impl GitStatuses { - fn status(&self) -> Option { - if self.conflict > 0 { - Some(GitFileStatus::Conflict) - } else if self.modified > 0 { - Some(GitFileStatus::Modified) - } else if self.added > 0 { - Some(GitFileStatus::Added) - } else { - None - } - } - - fn add_status(&self, status: Option) {} -} - #[derive(Clone, Debug, PartialEq, Eq)] pub struct Entry { pub id: ProjectEntryId, @@ -5105,16 +5035,6 @@ mod tests { snapshot.status_for_file(project_path.join(F_TXT)), Some(GitFileStatus::Added) ); - - dbg!(snapshot.entries(false).collect::>()); - - // Check stateful bubbling works - assert_eq!( - snapshot.status_for_file(dbg!(project_path)), - Some(GitFileStatus::Added) - ); - - assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Added)); }); std::fs::write(work_dir.join(A_TXT), "aa").unwrap(); @@ -5129,14 +5049,6 @@ mod tests { snapshot.status_for_file(project_path.join(A_TXT)), Some(GitFileStatus::Modified) ); - - // Check stateful bubbling works, modified overrules added - assert_eq!( - snapshot.status_for_file(project_path), - Some(GitFileStatus::Modified) - ); - - assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Modified)); }); git_add(Path::new(A_TXT), &repo); @@ -5145,7 +5057,6 @@ mod tests { tree.flush_fs_events(cx).await; deterministic.run_until_parked(); - dbg!(git_status(&repo)); // Check that repo only changes are tracked tree.read_with(cx, |tree, _cx| { let snapshot = tree.snapshot(); @@ -5155,18 +5066,8 @@ mod tests { Some(GitFileStatus::Added) ); - dbg!(snapshot.entries(false).collect::>()); - assert_eq!(snapshot.status_for_file(project_path.join(B_TXT)), None); assert_eq!(snapshot.status_for_file(project_path.join(A_TXT)), None); - - // Check bubbling - assert_eq!( - snapshot.status_for_file(project_path), - Some(GitFileStatus::Added) - ); - - assert_eq!(snapshot.status_for_file(""), Some(GitFileStatus::Added)); }); git_reset(0, &repo); @@ -5190,27 +5091,6 @@ mod tests { snapshot.status_for_file(project_path.join(E_TXT)), Some(GitFileStatus::Modified) ); - - // Check status bubbling - assert_eq!( - snapshot.status_for_file(project_path.join(Path::new(E_TXT).parent().unwrap())), - Some(GitFileStatus::Modified) - ); - assert_eq!( - snapshot.status_for_file( - project_path.join(Path::new(E_TXT).parent().unwrap().parent().unwrap()) - ), - Some(GitFileStatus::Modified) - ); - assert_eq!( - snapshot.status_for_file(project_path), - Some(GitFileStatus::Modified) - ); - - assert_eq!( - snapshot.status_for_file(project_path.join(F_TXT)), - Some(GitFileStatus::Added) - ); }); std::fs::remove_file(work_dir.join(B_TXT)).unwrap(); From 2f97c7a4f1d8e2ad333b1cfab33e5479684ab8f6 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 2 Jun 2023 16:41:01 -0700 Subject: [PATCH 07/29] Remove stale comments Implement status bubbling query with sum tree traversals co-authored-by: max --- crates/collab/src/tests/integration_tests.rs | 90 +++----- .../src/tests/randomized_integration_tests.rs | 7 +- crates/fs/src/fs.rs | 6 +- crates/project/src/worktree.rs | 216 +++++++++++++++--- crates/rpc/src/proto.rs | 5 - 5 files changed, 228 insertions(+), 96 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 5f5057478e..f8504c1905 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2415,14 +2415,10 @@ async fn test_git_diff_base_change( " .unindent(); - client_a - .fs - .as_fake() - .set_index_for_repo( - Path::new("/dir/.git"), - &[(Path::new("a.txt"), diff_base.clone())], - ) - .await; + client_a.fs.as_fake().set_index_for_repo( + Path::new("/dir/.git"), + &[(Path::new("a.txt"), diff_base.clone())], + ); // Create the buffer let buffer_local_a = project_local @@ -2464,14 +2460,10 @@ async fn test_git_diff_base_change( ); }); - client_a - .fs - .as_fake() - .set_index_for_repo( - Path::new("/dir/.git"), - &[(Path::new("a.txt"), new_diff_base.clone())], - ) - .await; + client_a.fs.as_fake().set_index_for_repo( + Path::new("/dir/.git"), + &[(Path::new("a.txt"), new_diff_base.clone())], + ); // Wait for buffer_local_a to receive it deterministic.run_until_parked(); @@ -2513,14 +2505,10 @@ async fn test_git_diff_base_change( " .unindent(); - client_a - .fs - .as_fake() - .set_index_for_repo( - Path::new("/dir/sub/.git"), - &[(Path::new("b.txt"), diff_base.clone())], - ) - .await; + client_a.fs.as_fake().set_index_for_repo( + Path::new("/dir/sub/.git"), + &[(Path::new("b.txt"), diff_base.clone())], + ); // Create the buffer let buffer_local_b = project_local @@ -2562,14 +2550,10 @@ async fn test_git_diff_base_change( ); }); - client_a - .fs - .as_fake() - .set_index_for_repo( - Path::new("/dir/sub/.git"), - &[(Path::new("b.txt"), new_diff_base.clone())], - ) - .await; + client_a.fs.as_fake().set_index_for_repo( + Path::new("/dir/sub/.git"), + &[(Path::new("b.txt"), new_diff_base.clone())], + ); // Wait for buffer_local_b to receive it deterministic.run_until_parked(); @@ -2646,8 +2630,7 @@ async fn test_git_branch_name( client_a .fs .as_fake() - .set_branch_name(Path::new("/dir/.git"), Some("branch-1")) - .await; + .set_branch_name(Path::new("/dir/.git"), Some("branch-1")); // Wait for it to catch up to the new branch deterministic.run_until_parked(); @@ -2673,8 +2656,7 @@ async fn test_git_branch_name( client_a .fs .as_fake() - .set_branch_name(Path::new("/dir/.git"), Some("branch-2")) - .await; + .set_branch_name(Path::new("/dir/.git"), Some("branch-2")); // Wait for buffer_local_a to receive it deterministic.run_until_parked(); @@ -2726,17 +2708,13 @@ async fn test_git_status_sync( const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; - client_a - .fs - .as_fake() - .set_status_for_repo( - Path::new("/dir/.git"), - &[ - (&Path::new(A_TXT), GitFileStatus::Added), - (&Path::new(B_TXT), GitFileStatus::Added), - ], - ) - .await; + client_a.fs.as_fake().set_status_for_repo( + Path::new("/dir/.git"), + &[ + (&Path::new(A_TXT), GitFileStatus::Added), + (&Path::new(B_TXT), GitFileStatus::Added), + ], + ); let (project_local, _worktree_id) = client_a.build_local_project("/dir", cx_a).await; let project_id = active_call_a @@ -2776,17 +2754,13 @@ async fn test_git_status_sync( assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); - client_a - .fs - .as_fake() - .set_status_for_repo( - Path::new("/dir/.git"), - &[ - (&Path::new(A_TXT), GitFileStatus::Modified), - (&Path::new(B_TXT), GitFileStatus::Modified), - ], - ) - .await; + client_a.fs.as_fake().set_status_for_repo( + Path::new("/dir/.git"), + &[ + (&Path::new(A_TXT), GitFileStatus::Modified), + (&Path::new(B_TXT), GitFileStatus::Modified), + ], + ); // Wait for buffer_local_a to receive it deterministic.run_until_parked(); diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 3beff6942a..b1fd05f01e 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -789,7 +789,7 @@ async fn apply_client_operation( if client.fs.metadata(&dot_git_dir).await?.is_none() { client.fs.create_dir(&dot_git_dir).await?; } - client.fs.set_index_for_repo(&dot_git_dir, &contents).await; + client.fs.set_index_for_repo(&dot_git_dir, &contents); } GitOperation::WriteGitBranch { repo_path, @@ -810,7 +810,7 @@ async fn apply_client_operation( if client.fs.metadata(&dot_git_dir).await?.is_none() { client.fs.create_dir(&dot_git_dir).await?; } - client.fs.set_branch_name(&dot_git_dir, new_branch).await; + client.fs.set_branch_name(&dot_git_dir, new_branch); } GitOperation::WriteGitStatuses { repo_path, @@ -840,8 +840,7 @@ async fn apply_client_operation( client .fs - .set_status_for_repo(&dot_git_dir, statuses.as_slice()) - .await; + .set_status_for_repo(&dot_git_dir, statuses.as_slice()); } }, } diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 99562405b5..77878b26a2 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -639,11 +639,11 @@ impl FakeFs { } } - pub async fn set_branch_name(&self, dot_git: &Path, branch: Option>) { + pub fn set_branch_name(&self, dot_git: &Path, branch: Option>) { self.with_git_state(dot_git, |state| state.branch_name = branch.map(Into::into)) } - pub async fn set_index_for_repo(&self, dot_git: &Path, head_state: &[(&Path, String)]) { + pub fn set_index_for_repo(&self, dot_git: &Path, head_state: &[(&Path, String)]) { self.with_git_state(dot_git, |state| { state.index_contents.clear(); state.index_contents.extend( @@ -654,7 +654,7 @@ impl FakeFs { }); } - pub async fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitFileStatus)]) { + pub fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitFileStatus)]) { self.with_git_state(dot_git, |state| { state.worktree_statuses.clear(); state.worktree_statuses.extend( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 9b7efd56d1..1d7a2058f8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -36,7 +36,6 @@ use postage::{ prelude::{Sink as _, Stream as _}, watch, }; -use sha2::digest::typenum::private::IsLessPrivate; use smol::channel::{self, Sender}; use std::{ any::Any, @@ -46,7 +45,7 @@ use std::{ fmt, future::Future, mem, - ops::{Deref, DerefMut}, + ops::{AddAssign, Deref, DerefMut, Sub}, path::{Path, PathBuf}, pin::Pin, sync::{ @@ -1455,7 +1454,7 @@ impl Snapshot { fn delete_entry(&mut self, entry_id: ProjectEntryId) -> Option> { let removed_entry = self.entries_by_id.remove(&entry_id, &())?; self.entries_by_path = { - let mut cursor = self.entries_by_path.cursor(); + let mut cursor = self.entries_by_path.cursor::(); let mut new_entries_by_path = cursor.slice(&TraversalTarget::Path(&removed_entry.path), Bias::Left, &()); while let Some(entry) = cursor.item() { @@ -1671,16 +1670,60 @@ impl Snapshot { }) } - pub fn statuses_for_directories(&self, paths: &[&Path]) -> Vec { - todo!(); - // ["/a/b", "a/b/c", "a/b/d", "j"] + pub fn statuses_for_paths(&self, paths: &[&Path]) -> Vec> { + let mut cursor = self + .entries_by_path + .cursor::<(TraversalProgress, GitStatuses)>(); + let mut paths = paths.iter().peekable(); + let mut path_stack = Vec::<(&Path, usize, GitStatuses)>::new(); + let mut result = Vec::new(); - // Path stack: - // If path has descendents following it, push to stack: ["a/b"] - // Figure out a/b/c - // Figure out a/b/d - // Once no more descendants, pop the stack: - // Figure out a/b + loop { + let next_path = paths.peek(); + let containing_path = path_stack.last(); + + let mut entry_to_finish = None; + let mut path_to_start = None; + match (containing_path, next_path) { + (Some((containing_path, _, _)), Some(next_path)) => { + if next_path.starts_with(containing_path) { + path_to_start = paths.next(); + } else { + entry_to_finish = path_stack.pop(); + } + } + (None, Some(_)) => path_to_start = paths.next(), + (Some(_), None) => entry_to_finish = path_stack.pop(), + (None, None) => break, + } + + if let Some((containing_path, result_ix, prev_statuses)) = entry_to_finish { + cursor.seek_forward( + &TraversalTarget::PathSuccessor(containing_path), + Bias::Left, + &(), + ); + + let statuses = cursor.start().1 - prev_statuses; + + result[result_ix] = if statuses.conflict > 0 { + Some(GitFileStatus::Conflict) + } else if statuses.modified > 0 { + Some(GitFileStatus::Modified) + } else if statuses.added > 0 { + Some(GitFileStatus::Added) + } else { + None + }; + } + if let Some(path_to_start) = path_to_start { + cursor.seek_forward(&TraversalTarget::Path(path_to_start), Bias::Left, &()); + path_stack.push((path_to_start, result.len(), cursor.start().1)); + result.push(None); + } + } + + return result; } pub fn paths(&self) -> impl Iterator> { @@ -1954,8 +1997,6 @@ impl LocalSnapshot { }, ); - self.scan_statuses(repo_lock.deref(), &work_directory, &work_directory.0); - drop(repo_lock); self.git_repositories.insert( @@ -2492,12 +2533,23 @@ impl sum_tree::Item for Entry { visible_file_count = 0; } + let mut statuses = GitStatuses::default(); + match self.git_status { + Some(status) => match status { + GitFileStatus::Added => statuses.added = 1, + GitFileStatus::Modified => statuses.modified = 1, + GitFileStatus::Conflict => statuses.conflict = 1, + }, + None => {} + } + EntrySummary { max_path: self.path.clone(), count: 1, visible_count, file_count, visible_file_count, + statuses, } } } @@ -2517,9 +2569,7 @@ pub struct EntrySummary { visible_count: usize, file_count: usize, visible_file_count: usize, - // git_modified_count: usize, - // git_added_count: usize, - // git_conflict_count: usize, + statuses: GitStatuses, } impl Default for EntrySummary { @@ -2530,6 +2580,7 @@ impl Default for EntrySummary { visible_count: 0, file_count: 0, visible_file_count: 0, + statuses: Default::default(), } } } @@ -2543,6 +2594,7 @@ impl sum_tree::Summary for EntrySummary { self.visible_count += rhs.visible_count; self.file_count += rhs.file_count; self.visible_file_count += rhs.visible_file_count; + self.statuses += rhs.statuses; } } @@ -2761,7 +2813,7 @@ impl BackgroundScanner { if let Some(paths) = paths { for path in paths { - self.reload_repo_for_file_path(&path, &mut *snapshot, self.fs.as_ref()); + self.reload_git_status(&path, &mut *snapshot, self.fs.as_ref()); } } @@ -3131,7 +3183,7 @@ impl BackgroundScanner { Some(()) } - fn reload_repo_for_file_path( + fn reload_git_status( &self, path: &Path, snapshot: &mut LocalSnapshot, @@ -3158,12 +3210,6 @@ impl BackgroundScanner { (*entry_id, repo.repo_ptr.to_owned()) }; - /* - 1. Populate dir, initializes the git repo - 2. Sometimes, we get a file event inside the .git repo, before it's initializaed - In both cases, we should end up with an initialized repo and a full status scan - - */ let work_dir = snapshot .entry_for_id(entry_id) @@ -3575,6 +3621,39 @@ impl<'a> Default for TraversalProgress<'a> { } } +#[derive(Clone, Debug, Default, Copy)] +struct GitStatuses { + added: usize, + modified: usize, + conflict: usize, +} + +impl AddAssign for GitStatuses { + fn add_assign(&mut self, rhs: Self) { + self.added += rhs.added; + self.modified += rhs.modified; + self.conflict += rhs.conflict; + } +} + +impl Sub for GitStatuses { + type Output = GitStatuses; + + fn sub(self, rhs: Self) -> Self::Output { + GitStatuses { + added: self.added - rhs.added, + modified: self.modified - rhs.modified, + conflict: self.conflict - rhs.conflict, + } + } +} + +impl<'a> sum_tree::Dimension<'a, EntrySummary> for GitStatuses { + fn add_summary(&mut self, summary: &'a EntrySummary, _: &()) { + *self += summary.statuses + } +} + pub struct Traversal<'a> { cursor: sum_tree::Cursor<'a, Entry, TraversalProgress<'a>>, include_ignored: bool, @@ -3676,6 +3755,14 @@ impl<'a, 'b> SeekTarget<'a, EntrySummary, TraversalProgress<'a>> for TraversalTa } } +impl<'a, 'b> SeekTarget<'a, EntrySummary, (TraversalProgress<'a>, GitStatuses)> + for TraversalTarget<'b> +{ + fn cmp(&self, cursor_location: &(TraversalProgress<'a>, GitStatuses), _: &()) -> Ordering { + self.cmp(&cursor_location.0, &()) + } +} + struct ChildEntriesIter<'a> { parent_path: &'a Path, traversal: Traversal<'a>, @@ -4962,7 +5049,6 @@ mod tests { }); } - // TODO: Stream statuses UPDATE THIS TO CHECK BUBBLIBG BEHAVIOR #[gpui::test] async fn test_git_status(deterministic: Arc, cx: &mut TestAppContext) { const IGNORE_RULE: &'static str = "**/target"; @@ -5154,6 +5240,84 @@ mod tests { }); } + #[gpui::test] + async fn test_statuses_for_paths(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/root", + json!({ + ".git": {}, + "a": { + "b": { + "c1.txt": "", + "c2.txt": "", + }, + "d": { + "e1.txt": "", + "e2.txt": "", + "e3.txt": "", + } + }, + "f": { + "no-status.txt": "" + }, + "g": { + "h1.txt": "", + "h2.txt": "" + }, + + }), + ) + .await; + + let http_client = FakeHttpClient::with_404_response(); + let client = cx.read(|cx| Client::new(http_client, cx)); + let tree = Worktree::local( + client, + Path::new("/root"), + true, + fs.clone(), + Default::default(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.foreground().run_until_parked(); + + fs.set_status_for_repo( + &Path::new("/root/.git"), + &[ + (Path::new("a/b/c1.txt"), GitFileStatus::Added), + (Path::new("a/d/e2.txt"), GitFileStatus::Modified), + (Path::new("g/h2.txt"), GitFileStatus::Conflict), + ], + ); + + cx.foreground().run_until_parked(); + + let snapshot = tree.read_with(cx, |tree, _| tree.snapshot()); + + assert_eq!( + snapshot.statuses_for_paths(&[ + Path::new(""), + Path::new("a"), + Path::new("a/b"), + Path::new("a/d"), + Path::new("f"), + Path::new("g"), + ]), + &[ + Some(GitFileStatus::Conflict), + Some(GitFileStatus::Modified), + Some(GitFileStatus::Added), + Some(GitFileStatus::Modified), + None, + Some(GitFileStatus::Conflict), + ] + ) + } + #[track_caller] fn git_init(path: &Path) -> git2::Repository { git2::Repository::init(path).expect("Failed to initialize git repository") diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 814b40403f..07925a0486 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -497,11 +497,6 @@ pub fn split_worktree_update( .map(|repo| (repo.work_directory_id, repo)) .collect::>(); - // Maintain a list of inflight repositories - // Every time we send a repository's work directory, stick it in the list of in-flight repositories - // Every go of the loop, drain each in-flight repository's statuses - // Until we have no more data - iter::from_fn(move || { if done_files { return None; From ca077408d7ca04771083ebee18baa15aa8fd9339 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 2 Jun 2023 17:38:39 -0700 Subject: [PATCH 08/29] Fix bug where git statuses would not be initialized on startup move git status queries to be on entry creation co-authored-by: max --- crates/project/src/worktree.rs | 92 ++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 1d7a2058f8..24f28c9dcd 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1822,6 +1822,14 @@ impl LocalSnapshot { self.git_repositories.get(&repo.work_directory.0) } + pub(crate) fn local_repo_for_path( + &self, + path: &Path, + ) -> Option<(RepositoryWorkDirectory, &LocalRepositoryEntry)> { + let (path, repo) = self.repository_and_work_directory_for_path(path)?; + Some((path, self.git_repositories.get(&repo.work_directory_id())?)) + } + pub(crate) fn repo_for_metadata( &self, path: &Path, @@ -1997,6 +2005,8 @@ impl LocalSnapshot { }, ); + self.scan_statuses(repo_lock.deref(), &work_directory); + drop(repo_lock); self.git_repositories.insert( @@ -2016,10 +2026,12 @@ impl LocalSnapshot { &mut self, repo_ptr: &dyn GitRepository, work_directory: &RepositoryWorkDirectory, - path: &Path, ) { let mut edits = vec![]; - for mut entry in self.descendent_entries(false, false, path).cloned() { + for mut entry in self + .descendent_entries(false, false, &work_directory.0) + .cloned() + { let Ok(repo_path) = entry.path.strip_prefix(&work_directory.0) else { continue; }; @@ -2754,6 +2766,7 @@ impl BackgroundScanner { let mut state = self.state.lock(); state.snapshot.completed_scan_id = state.snapshot.scan_id; } + self.send_status_update(false, None); // Process any any FS events that occurred while performing the initial scan. @@ -2813,7 +2826,7 @@ impl BackgroundScanner { if let Some(paths) = paths { for path in paths { - self.reload_git_status(&path, &mut *snapshot, self.fs.as_ref()); + self.reload_git_repo(&path, &mut *snapshot, self.fs.as_ref()); } } @@ -2940,14 +2953,18 @@ impl BackgroundScanner { let mut new_jobs: Vec> = Vec::new(); let mut ignore_stack = job.ignore_stack.clone(); let mut new_ignore = None; - let (root_abs_path, root_char_bag, next_entry_id) = { + let (root_abs_path, root_char_bag, next_entry_id, repository) = { let snapshot = &self.state.lock().snapshot; ( snapshot.abs_path().clone(), snapshot.root_char_bag, self.next_entry_id.clone(), + snapshot + .local_repo_for_path(&job.path) + .map(|(work_dir, repo)| (work_dir, repo.clone())), ) }; + let mut child_paths = self.fs.read_dir(&job.abs_path).await?; while let Some(child_abs_path) = child_paths.next().await { let child_abs_path: Arc = match child_abs_path { @@ -3040,6 +3057,13 @@ impl BackgroundScanner { } } else { child_entry.is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, false); + + if let Some((repo_path, repo)) = &repository { + if let Ok(path) = child_path.strip_prefix(&repo_path.0) { + child_entry.git_status = + repo.repo_ptr.lock().status(&RepoPath(path.into())); + } + } } new_entries.push(child_entry); @@ -3117,6 +3141,7 @@ impl BackgroundScanner { let ignore_stack = state .snapshot .ignore_stack_for_abs_path(&abs_path, metadata.is_dir); + let mut fs_entry = Entry::new( path.clone(), &metadata, @@ -3124,6 +3149,16 @@ impl BackgroundScanner { state.snapshot.root_char_bag, ); fs_entry.is_ignored = ignore_stack.is_all(); + + if !fs_entry.is_dir() { + if let Some((work_dir, repo)) = state.snapshot.local_repo_for_path(&path) { + if let Ok(path) = path.strip_prefix(work_dir.0) { + fs_entry.git_status = + repo.repo_ptr.lock().status(&RepoPath(path.into())) + } + } + } + state.insert_entry(fs_entry, self.fs.as_ref()); if let Some(scan_queue_tx) = &scan_queue_tx { @@ -3183,7 +3218,7 @@ impl BackgroundScanner { Some(()) } - fn reload_git_status( + fn reload_git_repo( &self, path: &Path, snapshot: &mut LocalSnapshot, @@ -3230,32 +3265,7 @@ impl BackgroundScanner { entry.branch = branch.map(Into::into); }); - snapshot.scan_statuses(repo.deref(), &work_dir, &work_dir.0); - } else { - if snapshot - .entry_for_path(&path) - .map(|entry| entry.is_ignored) - .unwrap_or(false) - { - return None; - } - - let repo = snapshot.repository_for_path(&path)?; - let work_directory = &repo.work_directory(snapshot)?; - let work_dir_id = repo.work_directory.clone(); - let (local_repo, git_dir_scan_id) = - snapshot.git_repositories.update(&work_dir_id, |entry| { - (entry.repo_ptr.clone(), entry.git_dir_scan_id) - })?; - - // Short circuit if we've already scanned everything - if git_dir_scan_id == scan_id { - return None; - } - - let repo_ptr = local_repo.lock(); - - snapshot.scan_statuses(repo_ptr.deref(), &work_directory, path); + snapshot.scan_statuses(repo.deref(), &work_dir); } Some(()) @@ -5270,6 +5280,15 @@ mod tests { ) .await; + fs.set_status_for_repo( + &Path::new("/root/.git"), + &[ + (Path::new("a/b/c1.txt"), GitFileStatus::Added), + (Path::new("a/d/e2.txt"), GitFileStatus::Modified), + (Path::new("g/h2.txt"), GitFileStatus::Conflict), + ], + ); + let http_client = FakeHttpClient::with_404_response(); let client = cx.read(|cx| Client::new(http_client, cx)); let tree = Worktree::local( @@ -5285,17 +5304,6 @@ mod tests { cx.foreground().run_until_parked(); - fs.set_status_for_repo( - &Path::new("/root/.git"), - &[ - (Path::new("a/b/c1.txt"), GitFileStatus::Added), - (Path::new("a/d/e2.txt"), GitFileStatus::Modified), - (Path::new("g/h2.txt"), GitFileStatus::Conflict), - ], - ); - - cx.foreground().run_until_parked(); - let snapshot = tree.read_with(cx, |tree, _| tree.snapshot()); assert_eq!( From e56fcd69b5dd9f43f08a9cf69d2a2f139edf07ac Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 11:50:23 -0700 Subject: [PATCH 09/29] Track git status changes with the changed_paths system --- crates/project/src/worktree.rs | 126 +++++++++++++++++++++------------ crates/util/src/util.rs | 2 +- 2 files changed, 83 insertions(+), 45 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 24f28c9dcd..1c1c409dca 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1974,7 +1974,8 @@ impl LocalSnapshot { entry } - fn build_repo(&mut self, parent_path: Arc, fs: &dyn Fs) -> Option<()> { + #[must_use = "Changed paths must be used for diffing later"] + fn build_repo(&mut self, parent_path: Arc, fs: &dyn Fs) -> Option>> { let abs_path = self.abs_path.join(&parent_path); let work_dir: Arc = parent_path.parent().unwrap().into(); @@ -1991,42 +1992,46 @@ impl LocalSnapshot { .entry_for_path(work_dir.clone()) .map(|entry| entry.id)?; - if self.git_repositories.get(&work_dir_id).is_none() { - let repo = fs.open_repo(abs_path.as_path())?; - let work_directory = RepositoryWorkDirectory(work_dir.clone()); - - let repo_lock = repo.lock(); - - self.repository_entries.insert( - work_directory.clone(), - RepositoryEntry { - work_directory: work_dir_id.into(), - branch: repo_lock.branch_name().map(Into::into), - }, - ); - - self.scan_statuses(repo_lock.deref(), &work_directory); - - drop(repo_lock); - - self.git_repositories.insert( - work_dir_id, - LocalRepositoryEntry { - git_dir_scan_id: 0, - repo_ptr: repo, - git_dir_path: parent_path.clone(), - }, - ) + if self.git_repositories.get(&work_dir_id).is_some() { + return None; } - Some(()) + let repo = fs.open_repo(abs_path.as_path())?; + let work_directory = RepositoryWorkDirectory(work_dir.clone()); + + let repo_lock = repo.lock(); + + self.repository_entries.insert( + work_directory.clone(), + RepositoryEntry { + work_directory: work_dir_id.into(), + branch: repo_lock.branch_name().map(Into::into), + }, + ); + + let changed_paths = self.scan_statuses(repo_lock.deref(), &work_directory); + + drop(repo_lock); + + self.git_repositories.insert( + work_dir_id, + LocalRepositoryEntry { + git_dir_scan_id: 0, + repo_ptr: repo, + git_dir_path: parent_path.clone(), + }, + ); + + Some(changed_paths) } + #[must_use = "Changed paths must be used for diffing later"] fn scan_statuses( &mut self, repo_ptr: &dyn GitRepository, work_directory: &RepositoryWorkDirectory, - ) { + ) -> Vec> { + let mut changes = vec![]; let mut edits = vec![]; for mut entry in self .descendent_entries(false, false, &work_directory.0) @@ -2038,10 +2043,12 @@ impl LocalSnapshot { let git_file_status = repo_ptr.status(&RepoPath(repo_path.into())); let status = git_file_status; entry.git_status = status; + changes.push(entry.path.clone()); edits.push(Edit::Insert(entry)); } self.entries_by_path.edit(edits, &()); + changes } fn ancestor_inodes_for_path(&self, path: &Path) -> TreeSet { @@ -2096,13 +2103,14 @@ impl BackgroundScannerState { self.snapshot.insert_entry(entry, fs) } + #[must_use = "Changed paths must be used for diffing later"] fn populate_dir( &mut self, parent_path: Arc, entries: impl IntoIterator, ignore: Option>, fs: &dyn Fs, - ) { + ) -> Option>> { let mut parent_entry = if let Some(parent_entry) = self .snapshot .entries_by_path @@ -2114,7 +2122,7 @@ impl BackgroundScannerState { "populating a directory {:?} that has been removed", parent_path ); - return; + return None; }; match parent_entry.kind { @@ -2122,7 +2130,7 @@ impl BackgroundScannerState { parent_entry.kind = EntryKind::Dir; } EntryKind::Dir => {} - _ => return, + _ => return None, } if let Some(ignore) = ignore { @@ -2152,8 +2160,9 @@ impl BackgroundScannerState { self.snapshot.entries_by_id.edit(entries_by_id_edits, &()); if parent_path.file_name() == Some(&DOT_GIT) { - self.snapshot.build_repo(parent_path, fs); + return self.snapshot.build_repo(parent_path, fs); } + None } fn remove_path(&mut self, path: &Path) { @@ -2822,14 +2831,16 @@ impl BackgroundScanner { self.update_ignore_statuses().await; { - let mut snapshot = &mut self.state.lock().snapshot; + let mut state = self.state.lock(); if let Some(paths) = paths { for path in paths { - self.reload_git_repo(&path, &mut *snapshot, self.fs.as_ref()); + self.reload_git_repo(&path, &mut *state, self.fs.as_ref()); } } + let mut snapshot = &mut state.snapshot; + let mut git_repositories = mem::take(&mut snapshot.git_repositories); git_repositories.retain(|work_directory_id, _| { snapshot @@ -3071,10 +3082,19 @@ impl BackgroundScanner { { let mut state = self.state.lock(); - state.populate_dir(job.path.clone(), new_entries, new_ignore, self.fs.as_ref()); + let changed_paths = + state.populate_dir(job.path.clone(), new_entries, new_ignore, self.fs.as_ref()); if let Err(ix) = state.changed_paths.binary_search(&job.path) { state.changed_paths.insert(ix, job.path.clone()); } + if let Some(changed_paths) = changed_paths { + util::extend_sorted( + &mut state.changed_paths, + changed_paths, + usize::MAX, + Ord::cmp, + ) + } } for new_job in new_jobs { @@ -3221,22 +3241,31 @@ impl BackgroundScanner { fn reload_git_repo( &self, path: &Path, - snapshot: &mut LocalSnapshot, + state: &mut BackgroundScannerState, fs: &dyn Fs, ) -> Option<()> { - let scan_id = snapshot.scan_id; + let scan_id = state.snapshot.scan_id; if path .components() .any(|component| component.as_os_str() == *DOT_GIT) { let (entry_id, repo_ptr) = { - let Some((entry_id, repo)) = snapshot.repo_for_metadata(&path) else { + let Some((entry_id, repo)) = state.snapshot.repo_for_metadata(&path) else { let dot_git_dir = path.ancestors() .skip_while(|ancestor| ancestor.file_name() != Some(&*DOT_GIT)) .next()?; - snapshot.build_repo(dot_git_dir.into(), fs); + let changed_paths = state.snapshot.build_repo(dot_git_dir.into(), fs); + if let Some(changed_paths) = changed_paths { + util::extend_sorted( + &mut state.changed_paths, + changed_paths, + usize::MAX, + Ord::cmp, + ); + } + return None; }; if repo.git_dir_scan_id == scan_id { @@ -3246,7 +3275,8 @@ impl BackgroundScanner { (*entry_id, repo.repo_ptr.to_owned()) }; - let work_dir = snapshot + let work_dir = state + .snapshot .entry_for_id(entry_id) .map(|entry| RepositoryWorkDirectory(entry.path.clone()))?; @@ -3254,18 +3284,26 @@ impl BackgroundScanner { repo.reload_index(); let branch = repo.branch_name(); - snapshot.git_repositories.update(&entry_id, |entry| { + state.snapshot.git_repositories.update(&entry_id, |entry| { entry.git_dir_scan_id = scan_id; }); - snapshot + state + .snapshot .snapshot .repository_entries .update(&work_dir, |entry| { entry.branch = branch.map(Into::into); }); - snapshot.scan_statuses(repo.deref(), &work_dir); + let changed_paths = state.snapshot.scan_statuses(repo.deref(), &work_dir); + + util::extend_sorted( + &mut state.changed_paths, + changed_paths, + usize::MAX, + Ord::cmp, + ) } Some(()) diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 9d787e1389..a182770dce 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -55,7 +55,7 @@ pub fn post_inc + AddAssign + Copy>(value: &mut T) -> T { } /// Extend a sorted vector with a sorted sequence of items, maintaining the vector's sort order and -/// enforcing a maximum length. Sort the items according to the given callback. Before calling this, +/// enforcing a maximum length. This also de-duplicates items. Sort the items according to the given callback. Before calling this, /// both `vec` and `new_items` should already be sorted according to the `cmp` comparator. pub fn extend_sorted(vec: &mut Vec, new_items: I, limit: usize, mut cmp: F) where From 4ac5f7b14ec27218a12bcda2010977011bbde35d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 12:06:23 -0700 Subject: [PATCH 10/29] Add statuses to test integration --- .../20221109000000_test_schema.sql | 18 +----------------- crates/collab/src/db.rs | 8 ++++---- crates/collab/src/db/worktree_entry.rs | 1 + 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 7c6a49f179..c12afafaf3 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -76,6 +76,7 @@ CREATE TABLE "worktree_entries" ( "is_symlink" BOOL NOT NULL, "is_ignored" BOOL NOT NULL, "is_deleted" BOOL NOT NULL, + "git_status" INTEGER, PRIMARY KEY(project_id, worktree_id, id), FOREIGN KEY(project_id, worktree_id) REFERENCES worktrees (project_id, id) ON DELETE CASCADE ); @@ -96,23 +97,6 @@ CREATE TABLE "worktree_repositories" ( CREATE INDEX "index_worktree_repositories_on_project_id" ON "worktree_repositories" ("project_id"); CREATE INDEX "index_worktree_repositories_on_project_id_and_worktree_id" ON "worktree_repositories" ("project_id", "worktree_id"); -CREATE TABLE "worktree_repository_statuses" ( - "project_id" INTEGER NOT NULL, - "worktree_id" INTEGER NOT NULL, - "work_directory_id" INTEGER NOT NULL, - "repo_path" VARCHAR NOT NULL, - "status" INTEGER NOT NULL, - "scan_id" INTEGER NOT NULL, - "is_deleted" BOOL NOT NULL, - PRIMARY KEY(project_id, worktree_id, work_directory_id, repo_path), - FOREIGN KEY(project_id, worktree_id) REFERENCES worktrees (project_id, id) ON DELETE CASCADE, - FOREIGN KEY(project_id, worktree_id, work_directory_id) REFERENCES worktree_entries (project_id, worktree_id, id) ON DELETE CASCADE -); -CREATE INDEX "index_worktree_repository_statuses_on_project_id" ON "worktree_repository_statuses" ("project_id"); -CREATE INDEX "index_worktree_repository_statuses_on_project_id_and_worktree_id" ON "worktree_repository_statuses" ("project_id", "worktree_id"); -CREATE INDEX "index_worktree_repository_statuses_on_project_id_and_worktree_id_and_work_directory_id" ON "worktree_repository_statuses" ("project_id", "worktree_id", "work_directory_id"); - - CREATE TABLE "worktree_diagnostic_summaries" ( "project_id" INTEGER NOT NULL, "worktree_id" INTEGER NOT NULL, diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index e3d553c606..3456bea44e 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1537,8 +1537,7 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, - // TODO stream statuses - git_status: None, + git_status: db_entry.git_status.map(|status| status as i32), }); } } @@ -2329,6 +2328,7 @@ impl Database { mtime_nanos: ActiveValue::set(mtime.nanos as i32), is_symlink: ActiveValue::set(entry.is_symlink), is_ignored: ActiveValue::set(entry.is_ignored), + git_status: ActiveValue::set(entry.git_status.map(|status| status as i64)), is_deleted: ActiveValue::set(false), scan_id: ActiveValue::set(update.scan_id as i64), } @@ -2347,6 +2347,7 @@ impl Database { worktree_entry::Column::MtimeNanos, worktree_entry::Column::IsSymlink, worktree_entry::Column::IsIgnored, + worktree_entry::Column::GitStatus, worktree_entry::Column::ScanId, ]) .to_owned(), @@ -2630,8 +2631,7 @@ impl Database { }), is_symlink: db_entry.is_symlink, is_ignored: db_entry.is_ignored, - // TODO stream statuses - git_status: None, + git_status: db_entry.git_status.map(|status| status as i32), }); } } diff --git a/crates/collab/src/db/worktree_entry.rs b/crates/collab/src/db/worktree_entry.rs index 4eb1648b81..f2df808ee3 100644 --- a/crates/collab/src/db/worktree_entry.rs +++ b/crates/collab/src/db/worktree_entry.rs @@ -15,6 +15,7 @@ pub struct Model { pub inode: i64, pub mtime_seconds: i64, pub mtime_nanos: i32, + pub git_status: Option, pub is_symlink: bool, pub is_ignored: bool, pub is_deleted: bool, From 49c5a3fa86c7a161d0aea63e64197fb811393dda Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 12:17:21 -0700 Subject: [PATCH 11/29] Add postgres migration --- .../migrations/20230605191135_remove_repository_statuses.sql | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 crates/collab/migrations/20230605191135_remove_repository_statuses.sql diff --git a/crates/collab/migrations/20230605191135_remove_repository_statuses.sql b/crates/collab/migrations/20230605191135_remove_repository_statuses.sql new file mode 100644 index 0000000000..02ebc48870 --- /dev/null +++ b/crates/collab/migrations/20230605191135_remove_repository_statuses.sql @@ -0,0 +1,4 @@ +DROP TABLE "worktree_repository_statuses"; + +ALTER TABLE "worktree_entries" +ADD "git_status" INT8; From 9a13a2ba2c0c269eca517685306ffc7bd4472005 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 12:53:04 -0700 Subject: [PATCH 12/29] WIP: Add status bubbling to project panel --- crates/fs/src/repository.rs | 12 ++--- crates/project/src/worktree.rs | 56 +++++++++++++++-------- crates/project_panel/src/project_panel.rs | 12 ++++- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index 70f2916681..efe3a40092 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -25,7 +25,7 @@ pub trait GitRepository: Send { fn statuses(&self) -> Option>; - fn status(&self, path: &RepoPath) -> Option; + fn status(&self, path: &RepoPath) -> Result>; } impl std::fmt::Debug for dyn GitRepository { @@ -92,9 +92,9 @@ impl GitRepository for LibGitRepository { Some(map) } - fn status(&self, path: &RepoPath) -> Option { - let status = self.status_file(path).log_err()?; - read_status(status) + fn status(&self, path: &RepoPath) -> Result> { + let status = self.status_file(path)?; + Ok(read_status(status)) } } @@ -156,9 +156,9 @@ impl GitRepository for FakeGitRepository { Some(map) } - fn status(&self, path: &RepoPath) -> Option { + fn status(&self, path: &RepoPath) -> Result> { let state = self.state.lock(); - state.worktree_statuses.get(path).cloned() + Ok(state.worktree_statuses.get(path).cloned()) } } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 1c1c409dca..d808c41350 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1670,11 +1670,14 @@ impl Snapshot { }) } - pub fn statuses_for_paths(&self, paths: &[&Path]) -> Vec> { + pub fn statuses_for_paths<'a>( + &self, + paths: impl IntoIterator, + ) -> Vec> { let mut cursor = self .entries_by_path .cursor::<(TraversalProgress, GitStatuses)>(); - let mut paths = paths.iter().peekable(); + let mut paths = paths.into_iter().peekable(); let mut path_stack = Vec::<(&Path, usize, GitStatuses)>::new(); let mut result = Vec::new(); @@ -2040,11 +2043,15 @@ impl LocalSnapshot { let Ok(repo_path) = entry.path.strip_prefix(&work_directory.0) else { continue; }; - let git_file_status = repo_ptr.status(&RepoPath(repo_path.into())); - let status = git_file_status; - entry.git_status = status; - changes.push(entry.path.clone()); - edits.push(Edit::Insert(entry)); + let git_file_status = repo_ptr + .status(&RepoPath(repo_path.into())) + .log_err() + .flatten(); + if entry.git_status != git_file_status { + entry.git_status = git_file_status; + changes.push(entry.path.clone()); + edits.push(Edit::Insert(entry)); + } } self.entries_by_path.edit(edits, &()); @@ -3068,11 +3075,16 @@ impl BackgroundScanner { } } else { child_entry.is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, false); - - if let Some((repo_path, repo)) = &repository { - if let Ok(path) = child_path.strip_prefix(&repo_path.0) { - child_entry.git_status = - repo.repo_ptr.lock().status(&RepoPath(path.into())); + if !child_entry.is_ignored { + if let Some((repo_path, repo)) = &repository { + if let Ok(path) = child_path.strip_prefix(&repo_path.0) { + child_entry.git_status = repo + .repo_ptr + .lock() + .status(&RepoPath(path.into())) + .log_err() + .flatten(); + } } } } @@ -3170,11 +3182,19 @@ impl BackgroundScanner { ); fs_entry.is_ignored = ignore_stack.is_all(); - if !fs_entry.is_dir() { - if let Some((work_dir, repo)) = state.snapshot.local_repo_for_path(&path) { - if let Ok(path) = path.strip_prefix(work_dir.0) { - fs_entry.git_status = - repo.repo_ptr.lock().status(&RepoPath(path.into())) + if !fs_entry.is_ignored { + if !fs_entry.is_dir() { + if let Some((work_dir, repo)) = + state.snapshot.local_repo_for_path(&path) + { + if let Ok(path) = path.strip_prefix(work_dir.0) { + fs_entry.git_status = repo + .repo_ptr + .lock() + .status(&RepoPath(path.into())) + .log_err() + .flatten() + } } } } @@ -5345,7 +5365,7 @@ mod tests { let snapshot = tree.read_with(cx, |tree, _| tree.snapshot()); assert_eq!( - snapshot.statuses_for_paths(&[ + snapshot.statuses_for_paths([ Path::new(""), Path::new("a"), Path::new("a/b"), diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index f6f3d67f3a..3d7d32cef4 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1109,8 +1109,16 @@ impl ProjectPanel { .unwrap_or(&[]); let entry_range = range.start.saturating_sub(ix)..end_ix - ix; - for entry in visible_worktree_entries[entry_range].iter() { - let status = git_status_setting.then(|| entry.git_status).flatten(); + let statuses = worktree.read(cx).statuses_for_paths( + visible_worktree_entries[entry_range.clone()] + .iter() + .map(|entry| entry.path.as_ref()), + ); + for (entry, status) in visible_worktree_entries[entry_range] + .iter() + .zip(statuses.into_iter()) + { + let status = git_status_setting.then(|| status).flatten(); let mut details = EntryDetails { filename: entry From c12bdc894a8e475a1aa07779c107f47eb39a3c68 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 15:19:59 -0700 Subject: [PATCH 13/29] Silence not found errors --- crates/fs/src/repository.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/repository.rs b/crates/fs/src/repository.rs index efe3a40092..488262887f 100644 --- a/crates/fs/src/repository.rs +++ b/crates/fs/src/repository.rs @@ -1,5 +1,6 @@ use anyhow::Result; use collections::HashMap; +use git2::ErrorCode; use parking_lot::Mutex; use rpc::proto; use serde_derive::{Deserialize, Serialize}; @@ -93,8 +94,17 @@ impl GitRepository for LibGitRepository { } fn status(&self, path: &RepoPath) -> Result> { - let status = self.status_file(path)?; - Ok(read_status(status)) + let status = self.status_file(path); + match status { + Ok(status) => Ok(read_status(status)), + Err(e) => { + if e.code() == ErrorCode::NotFound { + Ok(None) + } else { + Err(e.into()) + } + } + } } } From a2d58068a7f52f3a81b2e2a4a22bf02a19f084d9 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 5 Jun 2023 17:30:12 -0700 Subject: [PATCH 14/29] Improve test generation and implement status propogation co-authored-by: max --- crates/collab/src/tests/integration_tests.rs | 19 ++- .../src/tests/randomized_integration_tests.rs | 18 ++- crates/fs/src/fs.rs | 40 ++++- crates/project/src/worktree.rs | 138 +++++++++++------- crates/project_panel/src/project_panel.rs | 15 +- 5 files changed, 154 insertions(+), 76 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index f8504c1905..889f5bf77d 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2708,7 +2708,7 @@ async fn test_git_status_sync( const A_TXT: &'static str = "a.txt"; const B_TXT: &'static str = "b.txt"; - client_a.fs.as_fake().set_status_for_repo( + client_a.fs.as_fake().set_status_for_repo_via_git_operation( Path::new("/dir/.git"), &[ (&Path::new(A_TXT), GitFileStatus::Added), @@ -2754,13 +2754,16 @@ async fn test_git_status_sync( assert_status(&Path::new(B_TXT), Some(GitFileStatus::Added), project, cx); }); - client_a.fs.as_fake().set_status_for_repo( - Path::new("/dir/.git"), - &[ - (&Path::new(A_TXT), GitFileStatus::Modified), - (&Path::new(B_TXT), GitFileStatus::Modified), - ], - ); + client_a + .fs + .as_fake() + .set_status_for_repo_via_working_copy_change( + Path::new("/dir/.git"), + &[ + (&Path::new(A_TXT), GitFileStatus::Modified), + (&Path::new(B_TXT), GitFileStatus::Modified), + ], + ); // Wait for buffer_local_a to receive it deterministic.run_until_parked(); diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index b1fd05f01e..7a46d4c3f6 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -815,6 +815,7 @@ async fn apply_client_operation( GitOperation::WriteGitStatuses { repo_path, statuses, + git_operation, } => { if !client.fs.directories().contains(&repo_path) { return Err(TestError::Inapplicable); @@ -838,9 +839,16 @@ async fn apply_client_operation( client.fs.create_dir(&dot_git_dir).await?; } - client - .fs - .set_status_for_repo(&dot_git_dir, statuses.as_slice()); + if git_operation { + client + .fs + .set_status_for_repo_via_git_operation(&dot_git_dir, statuses.as_slice()); + } else { + client.fs.set_status_for_repo_via_working_copy_change( + &dot_git_dir, + statuses.as_slice(), + ); + } } }, } @@ -1229,6 +1237,7 @@ enum GitOperation { WriteGitStatuses { repo_path: PathBuf, statuses: Vec<(PathBuf, GitFileStatus)>, + git_operation: bool, }, } @@ -1854,9 +1863,12 @@ impl TestPlan { }) .collect::>(); + let git_operation = self.rng.gen::(); + GitOperation::WriteGitStatuses { repo_path, statuses, + git_operation, } } _ => unreachable!(), diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 77878b26a2..e8b309242d 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -619,7 +619,7 @@ impl FakeFs { .boxed() } - pub fn with_git_state(&self, dot_git: &Path, f: F) + pub fn with_git_state(&self, dot_git: &Path, emit_git_event: bool, f: F) where F: FnOnce(&mut FakeGitRepositoryState), { @@ -633,18 +633,22 @@ impl FakeFs { f(&mut repo_state); - state.emit_event([dot_git]); + if emit_git_event { + state.emit_event([dot_git]); + } } else { panic!("not a directory"); } } pub fn set_branch_name(&self, dot_git: &Path, branch: Option>) { - self.with_git_state(dot_git, |state| state.branch_name = branch.map(Into::into)) + self.with_git_state(dot_git, true, |state| { + state.branch_name = branch.map(Into::into) + }) } pub fn set_index_for_repo(&self, dot_git: &Path, head_state: &[(&Path, String)]) { - self.with_git_state(dot_git, |state| { + self.with_git_state(dot_git, true, |state| { state.index_contents.clear(); state.index_contents.extend( head_state @@ -654,8 +658,32 @@ impl FakeFs { }); } - pub fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&Path, GitFileStatus)]) { - self.with_git_state(dot_git, |state| { + pub fn set_status_for_repo_via_working_copy_change( + &self, + dot_git: &Path, + statuses: &[(&Path, GitFileStatus)], + ) { + self.with_git_state(dot_git, false, |state| { + state.worktree_statuses.clear(); + state.worktree_statuses.extend( + statuses + .iter() + .map(|(path, content)| ((**path).into(), content.clone())), + ); + }); + self.state.lock().emit_event( + statuses + .iter() + .map(|(path, _)| dot_git.parent().unwrap().join(path)), + ); + } + + pub fn set_status_for_repo_via_git_operation( + &self, + dot_git: &Path, + statuses: &[(&Path, GitFileStatus)], + ) { + self.with_git_state(dot_git, true, |state| { state.worktree_statuses.clear(); state.worktree_statuses.extend( statuses diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index d808c41350..27dc890ec0 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1670,46 +1670,42 @@ impl Snapshot { }) } - pub fn statuses_for_paths<'a>( - &self, - paths: impl IntoIterator, - ) -> Vec> { + /// Update the `git_status` of the given entries such that files' + /// statuses bubble up to their ancestor directories. + pub fn propagate_git_statuses(&self, result: &mut [Entry]) { let mut cursor = self .entries_by_path .cursor::<(TraversalProgress, GitStatuses)>(); - let mut paths = paths.into_iter().peekable(); - let mut path_stack = Vec::<(&Path, usize, GitStatuses)>::new(); - let mut result = Vec::new(); + let mut entry_stack = Vec::<(usize, GitStatuses)>::new(); + let mut result_ix = 0; loop { - let next_path = paths.peek(); - let containing_path = path_stack.last(); + let next_entry = result.get(result_ix); + let containing_entry = entry_stack.last().map(|(ix, _)| &result[*ix]); - let mut entry_to_finish = None; - let mut path_to_start = None; - match (containing_path, next_path) { - (Some((containing_path, _, _)), Some(next_path)) => { - if next_path.starts_with(containing_path) { - path_to_start = paths.next(); + let entry_to_finish = match (containing_entry, next_entry) { + (Some(_), None) => entry_stack.pop(), + (Some(containing_entry), Some(next_path)) => { + if !next_path.path.starts_with(&containing_entry.path) { + entry_stack.pop() } else { - entry_to_finish = path_stack.pop(); + None } } - (None, Some(_)) => path_to_start = paths.next(), - (Some(_), None) => entry_to_finish = path_stack.pop(), + (None, Some(_)) => None, (None, None) => break, - } + }; - if let Some((containing_path, result_ix, prev_statuses)) = entry_to_finish { + if let Some((entry_ix, prev_statuses)) = entry_to_finish { cursor.seek_forward( - &TraversalTarget::PathSuccessor(containing_path), + &TraversalTarget::PathSuccessor(&result[entry_ix].path), Bias::Left, &(), ); let statuses = cursor.start().1 - prev_statuses; - result[result_ix] = if statuses.conflict > 0 { + result[entry_ix].git_status = if statuses.conflict > 0 { Some(GitFileStatus::Conflict) } else if statuses.modified > 0 { Some(GitFileStatus::Modified) @@ -1718,15 +1714,18 @@ impl Snapshot { } else { None }; - } - if let Some(path_to_start) = path_to_start { - cursor.seek_forward(&TraversalTarget::Path(path_to_start), Bias::Left, &()); - path_stack.push((path_to_start, result.len(), cursor.start().1)); - result.push(None); + } else { + if result[result_ix].is_dir() { + cursor.seek_forward( + &TraversalTarget::Path(&result[result_ix].path), + Bias::Left, + &(), + ); + entry_stack.push((result_ix, cursor.start().1)); + } + result_ix += 1; } } - - return result; } pub fn paths(&self) -> impl Iterator> { @@ -5309,7 +5308,7 @@ mod tests { } #[gpui::test] - async fn test_statuses_for_paths(cx: &mut TestAppContext) { + async fn test_propagate_git_statuses(cx: &mut TestAppContext) { let fs = FakeFs::new(cx.background()); fs.insert_tree( "/root", @@ -5338,7 +5337,7 @@ mod tests { ) .await; - fs.set_status_for_repo( + fs.set_status_for_repo_via_git_operation( &Path::new("/root/.git"), &[ (Path::new("a/b/c1.txt"), GitFileStatus::Added), @@ -5361,27 +5360,68 @@ mod tests { .unwrap(); cx.foreground().run_until_parked(); - let snapshot = tree.read_with(cx, |tree, _| tree.snapshot()); - assert_eq!( - snapshot.statuses_for_paths([ - Path::new(""), - Path::new("a"), - Path::new("a/b"), - Path::new("a/d"), - Path::new("f"), - Path::new("g"), - ]), + check_propagated_statuses( + &snapshot, &[ - Some(GitFileStatus::Conflict), - Some(GitFileStatus::Modified), - Some(GitFileStatus::Added), - Some(GitFileStatus::Modified), - None, - Some(GitFileStatus::Conflict), - ] - ) + (Path::new(""), Some(GitFileStatus::Conflict)), + (Path::new("a"), Some(GitFileStatus::Modified)), + (Path::new("a/b"), Some(GitFileStatus::Added)), + (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + (Path::new("a/b/c2.txt"), None), + (Path::new("a/d"), Some(GitFileStatus::Modified)), + (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + (Path::new("f"), None), + (Path::new("f/no-status.txt"), None), + (Path::new("g"), Some(GitFileStatus::Conflict)), + (Path::new("g/h2.txt"), Some(GitFileStatus::Conflict)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new("a/b"), Some(GitFileStatus::Added)), + (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + (Path::new("a/b/c2.txt"), None), + (Path::new("a/d"), Some(GitFileStatus::Modified)), + (Path::new("a/d/e1.txt"), None), + (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + (Path::new("f"), None), + (Path::new("f/no-status.txt"), None), + (Path::new("g"), Some(GitFileStatus::Conflict)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + (Path::new("a/b/c2.txt"), None), + (Path::new("a/d/e1.txt"), None), + (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + (Path::new("f/no-status.txt"), None), + ], + ); + + fn check_propagated_statuses( + snapshot: &Snapshot, + expected_statuses: &[(&Path, Option)], + ) { + let mut entries = expected_statuses + .iter() + .map(|(path, _)| snapshot.entry_for_path(path).unwrap().clone()) + .collect::>(); + snapshot.propagate_git_statuses(&mut entries); + assert_eq!( + entries + .iter() + .map(|e| (e.path.as_ref(), e.git_status)) + .collect::>(), + expected_statuses + ); + } } #[track_caller] diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 3d7d32cef4..9563d54be8 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1012,6 +1012,9 @@ impl ProjectPanel { } entry_iter.advance(); } + + snapshot.propagate_git_statuses(&mut visible_worktree_entries); + visible_worktree_entries.sort_by(|entry_a, entry_b| { let mut components_a = entry_a.path.components().peekable(); let mut components_b = entry_b.path.components().peekable(); @@ -1109,16 +1112,8 @@ impl ProjectPanel { .unwrap_or(&[]); let entry_range = range.start.saturating_sub(ix)..end_ix - ix; - let statuses = worktree.read(cx).statuses_for_paths( - visible_worktree_entries[entry_range.clone()] - .iter() - .map(|entry| entry.path.as_ref()), - ); - for (entry, status) in visible_worktree_entries[entry_range] - .iter() - .zip(statuses.into_iter()) - { - let status = git_status_setting.then(|| status).flatten(); + for entry in visible_worktree_entries[entry_range].iter() { + let status = git_status_setting.then(|| entry.git_status).flatten(); let mut details = EntryDetails { filename: entry From 29de420b59b4734a8c23f313d77d2cab3042093e Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Wed, 7 Jun 2023 12:50:37 -0400 Subject: [PATCH 15/29] Organize misc files into `theme`, `themes` and `styleTrees` --- styles/src/buildThemes.ts | 6 +- styles/src/common.ts | 4 +- styles/src/styleTree/app.ts | 2 +- styles/src/styleTree/assistant.ts | 156 +++++++++--------- styles/src/styleTree/commandPalette.ts | 4 +- styles/src/styleTree/components.ts | 2 +- styles/src/styleTree/contactFinder.ts | 2 +- styles/src/styleTree/contactList.ts | 2 +- styles/src/styleTree/contactNotification.ts | 2 +- styles/src/styleTree/contactsPopover.ts | 2 +- styles/src/styleTree/contextMenu.ts | 2 +- styles/src/styleTree/copilot.ts | 2 +- styles/src/styleTree/editor.ts | 15 +- styles/src/styleTree/feedback.ts | 2 +- styles/src/styleTree/hoverPopover.ts | 2 +- .../src/styleTree/incomingCallNotification.ts | 2 +- styles/src/styleTree/picker.ts | 4 +- styles/src/styleTree/projectDiagnostics.ts | 2 +- styles/src/styleTree/projectPanel.ts | 14 +- .../styleTree/projectSharedNotification.ts | 2 +- styles/src/styleTree/search.ts | 6 +- styles/src/styleTree/sharedScreen.ts | 2 +- .../styleTree/simpleMessageNotification.ts | 2 +- styles/src/styleTree/statusBar.ts | 2 +- styles/src/styleTree/tabBar.ts | 6 +- styles/src/styleTree/terminal.ts | 2 +- styles/src/styleTree/tooltip.ts | 2 +- styles/src/styleTree/updateNotification.ts | 2 +- styles/src/styleTree/welcome.ts | 4 +- styles/src/styleTree/workspace.ts | 8 +- styles/src/{utils => theme}/color.ts | 0 .../{themes/common => theme}/colorScheme.ts | 2 +- styles/src/{themes/common => theme}/index.ts | 2 +- styles/src/{themes/common => theme}/ramps.ts | 2 +- styles/src/{themes/common => theme}/syntax.ts | 2 +- styles/src/{ => theme}/themeConfig.ts | 2 +- styles/src/themes/index.ts | 2 +- 37 files changed, 141 insertions(+), 136 deletions(-) rename styles/src/{utils => theme}/color.ts (100%) rename styles/src/{themes/common => theme}/colorScheme.ts (99%) rename styles/src/{themes/common => theme}/index.ts (69%) rename styles/src/{themes/common => theme}/ramps.ts (98%) rename styles/src/{themes/common => theme}/syntax.ts (99%) rename styles/src/{ => theme}/themeConfig.ts (98%) diff --git a/styles/src/buildThemes.ts b/styles/src/buildThemes.ts index bba300989f..8d807d62f3 100644 --- a/styles/src/buildThemes.ts +++ b/styles/src/buildThemes.ts @@ -2,7 +2,7 @@ import * as fs from "fs" import { tmpdir } from "os" import * as path from "path" import app from "./styleTree/app" -import { ColorScheme, createColorScheme } from "./themes/common/colorScheme" +import { ColorScheme, createColorScheme } from "./theme/colorScheme" import snakeCase from "./utils/snakeCase" import { themes } from "./themes" @@ -35,7 +35,9 @@ function writeThemes(colorSchemes: ColorScheme[], outputDirectory: string) { } } -const colorSchemes: ColorScheme[] = themes.map((theme) => createColorScheme(theme)) +const colorSchemes: ColorScheme[] = themes.map((theme) => + createColorScheme(theme) +) // Write new themes to theme directory writeThemes(colorSchemes, `${assetsDirectory}/themes`) diff --git a/styles/src/common.ts b/styles/src/common.ts index 5b6ddff422..ee47bcc6bd 100644 --- a/styles/src/common.ts +++ b/styles/src/common.ts @@ -1,5 +1,5 @@ import chroma from "chroma-js" -export * from "./themes/common" +export * from "./theme" export { chroma } export const fontFamilies = { @@ -27,7 +27,7 @@ export type FontWeight = | "bold" | "extra_bold" | "black" - + export const fontWeights: { [key: string]: FontWeight } = { thin: "thin", extra_light: "extra_light", diff --git a/styles/src/styleTree/app.ts b/styles/src/styleTree/app.ts index a9700a8d99..7398432ec3 100644 --- a/styles/src/styleTree/app.ts +++ b/styles/src/styleTree/app.ts @@ -18,7 +18,7 @@ import tooltip from "./tooltip" import terminal from "./terminal" import contactList from "./contactList" import incomingCallNotification from "./incomingCallNotification" -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import feedback from "./feedback" import welcome from "./welcome" import copilot from "./copilot" diff --git a/styles/src/styleTree/assistant.ts b/styles/src/styleTree/assistant.ts index 5e33967b50..bbb4aae5e1 100644 --- a/styles/src/styleTree/assistant.ts +++ b/styles/src/styleTree/assistant.ts @@ -1,85 +1,85 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { text, border, background, foreground } from "./components" import editor from "./editor" export default function assistant(colorScheme: ColorScheme) { - const layer = colorScheme.highest; + const layer = colorScheme.highest return { - container: { - background: editor(colorScheme).background, - padding: { left: 12 } - }, - header: { - border: border(layer, "default", { bottom: true, top: true }), - margin: { bottom: 6, top: 6 }, - background: editor(colorScheme).background - }, - userSender: { - ...text(layer, "sans", "default", { size: "sm", weight: "bold" }), - }, - assistantSender: { - ...text(layer, "sans", "accent", { size: "sm", weight: "bold" }), - }, - systemSender: { - ...text(layer, "sans", "variant", { size: "sm", weight: "bold" }), - }, - sentAt: { - margin: { top: 2, left: 8 }, - ...text(layer, "sans", "default", { size: "2xs" }), - }, - modelInfoContainer: { - margin: { right: 16, top: 4 }, - }, - model: { - background: background(layer, "on"), - border: border(layer, "on", { overlay: true }), - padding: 4, - cornerRadius: 4, - ...text(layer, "sans", "default", { size: "xs" }), - hover: { - background: background(layer, "on", "hovered"), - } - }, - remainingTokens: { - background: background(layer, "on"), - border: border(layer, "on", { overlay: true }), - padding: 4, - margin: { left: 4 }, - cornerRadius: 4, - ...text(layer, "sans", "positive", { size: "xs" }), - }, - noRemainingTokens: { - background: background(layer, "on"), - border: border(layer, "on", { overlay: true }), - padding: 4, - margin: { left: 4 }, - cornerRadius: 4, - ...text(layer, "sans", "negative", { size: "xs" }), - }, - errorIcon: { - margin: { left: 8 }, - color: foreground(layer, "negative"), - width: 12, - }, - apiKeyEditor: { - background: background(layer, "on"), - cornerRadius: 6, - text: text(layer, "mono", "on"), - placeholderText: text(layer, "mono", "on", "disabled", { - size: "xs", - }), - selection: colorScheme.players[0], - border: border(layer, "on"), - padding: { - bottom: 4, - left: 8, - right: 8, - top: 4, - }, - }, - apiKeyPrompt: { - padding: 10, - ...text(layer, "sans", "default", { size: "xs" }), - } + container: { + background: editor(colorScheme).background, + padding: { left: 12 }, + }, + header: { + border: border(layer, "default", { bottom: true, top: true }), + margin: { bottom: 6, top: 6 }, + background: editor(colorScheme).background, + }, + userSender: { + ...text(layer, "sans", "default", { size: "sm", weight: "bold" }), + }, + assistantSender: { + ...text(layer, "sans", "accent", { size: "sm", weight: "bold" }), + }, + systemSender: { + ...text(layer, "sans", "variant", { size: "sm", weight: "bold" }), + }, + sentAt: { + margin: { top: 2, left: 8 }, + ...text(layer, "sans", "default", { size: "2xs" }), + }, + modelInfoContainer: { + margin: { right: 16, top: 4 }, + }, + model: { + background: background(layer, "on"), + border: border(layer, "on", { overlay: true }), + padding: 4, + cornerRadius: 4, + ...text(layer, "sans", "default", { size: "xs" }), + hover: { + background: background(layer, "on", "hovered"), + }, + }, + remainingTokens: { + background: background(layer, "on"), + border: border(layer, "on", { overlay: true }), + padding: 4, + margin: { left: 4 }, + cornerRadius: 4, + ...text(layer, "sans", "positive", { size: "xs" }), + }, + noRemainingTokens: { + background: background(layer, "on"), + border: border(layer, "on", { overlay: true }), + padding: 4, + margin: { left: 4 }, + cornerRadius: 4, + ...text(layer, "sans", "negative", { size: "xs" }), + }, + errorIcon: { + margin: { left: 8 }, + color: foreground(layer, "negative"), + width: 12, + }, + apiKeyEditor: { + background: background(layer, "on"), + cornerRadius: 6, + text: text(layer, "mono", "on"), + placeholderText: text(layer, "mono", "on", "disabled", { + size: "xs", + }), + selection: colorScheme.players[0], + border: border(layer, "on"), + padding: { + bottom: 4, + left: 8, + right: 8, + top: 4, + }, + }, + apiKeyPrompt: { + padding: 10, + ...text(layer, "sans", "default", { size: "xs" }), + }, } } diff --git a/styles/src/styleTree/commandPalette.ts b/styles/src/styleTree/commandPalette.ts index 038a009007..c49e1f194c 100644 --- a/styles/src/styleTree/commandPalette.ts +++ b/styles/src/styleTree/commandPalette.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { text, background } from "./components" export default function commandPalette(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/components.ts b/styles/src/styleTree/components.ts index efd4a95672..a575dad527 100644 --- a/styles/src/styleTree/components.ts +++ b/styles/src/styleTree/components.ts @@ -1,5 +1,5 @@ import { fontFamilies, fontSizes, FontWeight } from "../common" -import { Layer, Styles, StyleSets, Style } from "../themes/common/colorScheme" +import { Layer, Styles, StyleSets, Style } from "../theme/colorScheme" function isStyleSet(key: any): key is StyleSets { return [ diff --git a/styles/src/styleTree/contactFinder.ts b/styles/src/styleTree/contactFinder.ts index 7b76cde444..e45647c3d6 100644 --- a/styles/src/styleTree/contactFinder.ts +++ b/styles/src/styleTree/contactFinder.ts @@ -1,5 +1,5 @@ import picker from "./picker" -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, foreground, text } from "./components" export default function contactFinder(colorScheme: ColorScheme): any { diff --git a/styles/src/styleTree/contactList.ts b/styles/src/styleTree/contactList.ts index 8ff5d903d8..a597e44d9f 100644 --- a/styles/src/styleTree/contactList.ts +++ b/styles/src/styleTree/contactList.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, borderColor, foreground, text } from "./components" export default function contactsPanel(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/contactNotification.ts b/styles/src/styleTree/contactNotification.ts index 186f7e8685..85a0b9d0de 100644 --- a/styles/src/styleTree/contactNotification.ts +++ b/styles/src/styleTree/contactNotification.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, foreground, text } from "./components" const avatarSize = 12 diff --git a/styles/src/styleTree/contactsPopover.ts b/styles/src/styleTree/contactsPopover.ts index 5e865aff17..5946bfb82c 100644 --- a/styles/src/styleTree/contactsPopover.ts +++ b/styles/src/styleTree/contactsPopover.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, text } from "./components" export default function contactsPopover(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/contextMenu.ts b/styles/src/styleTree/contextMenu.ts index b4a21deba4..f14cd90219 100644 --- a/styles/src/styleTree/contextMenu.ts +++ b/styles/src/styleTree/contextMenu.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, borderColor, text } from "./components" export default function contextMenu(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/copilot.ts b/styles/src/styleTree/copilot.ts index 9fa86cd741..8614cb6976 100644 --- a/styles/src/styleTree/copilot.ts +++ b/styles/src/styleTree/copilot.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, foreground, svg, text } from "./components" export default function copilot(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/editor.ts b/styles/src/styleTree/editor.ts index 66074aa684..55f3da6e90 100644 --- a/styles/src/styleTree/editor.ts +++ b/styles/src/styleTree/editor.ts @@ -1,9 +1,9 @@ -import { withOpacity } from "../utils/color" -import { ColorScheme, Layer, StyleSets } from "../themes/common/colorScheme" +import { withOpacity } from "../theme/color" +import { ColorScheme, Layer, StyleSets } from "../theme/colorScheme" import { background, border, borderColor, foreground, text } from "./components" import hoverPopover from "./hoverPopover" -import { buildSyntax } from "../themes/common/syntax" +import { buildSyntax } from "../theme/syntax" export default function editor(colorScheme: ColorScheme) { const { isLight } = colorScheme @@ -186,7 +186,10 @@ export default function editor(colorScheme: ColorScheme) { }, }, source: { - text: text(colorScheme.middle, "sans", { size: "sm", weight: "bold", }), + text: text(colorScheme.middle, "sans", { + size: "sm", + weight: "bold", + }), }, message: { highlightText: text(colorScheme.middle, "sans", { @@ -250,7 +253,7 @@ export default function editor(colorScheme: ColorScheme) { right: true, left: true, bottom: false, - } + }, }, git: { deleted: isLight @@ -262,7 +265,7 @@ export default function editor(colorScheme: ColorScheme) { inserted: isLight ? withOpacity(colorScheme.ramps.green(0.5).hex(), 0.8) : withOpacity(colorScheme.ramps.green(0.4).hex(), 0.8), - } + }, }, compositionMark: { underline: { diff --git a/styles/src/styleTree/feedback.ts b/styles/src/styleTree/feedback.ts index ba688d1e2e..5eef4b4279 100644 --- a/styles/src/styleTree/feedback.ts +++ b/styles/src/styleTree/feedback.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, text } from "./components" export default function feedback(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/hoverPopover.ts b/styles/src/styleTree/hoverPopover.ts index b01402e069..f8988f1f3a 100644 --- a/styles/src/styleTree/hoverPopover.ts +++ b/styles/src/styleTree/hoverPopover.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, foreground, text } from "./components" export default function HoverPopover(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/incomingCallNotification.ts b/styles/src/styleTree/incomingCallNotification.ts index 8a57554fce..c42558059c 100644 --- a/styles/src/styleTree/incomingCallNotification.ts +++ b/styles/src/styleTree/incomingCallNotification.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, text } from "./components" export default function incomingCallNotification( diff --git a/styles/src/styleTree/picker.ts b/styles/src/styleTree/picker.ts index 66a0d2c126..d84bd6fc7a 100644 --- a/styles/src/styleTree/picker.ts +++ b/styles/src/styleTree/picker.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { background, border, text } from "./components" export default function picker(colorScheme: ColorScheme): any { diff --git a/styles/src/styleTree/projectDiagnostics.ts b/styles/src/styleTree/projectDiagnostics.ts index 127cbdb353..cf0f07dd8c 100644 --- a/styles/src/styleTree/projectDiagnostics.ts +++ b/styles/src/styleTree/projectDiagnostics.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, text } from "./components" export default function projectDiagnostics(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/projectPanel.ts b/styles/src/styleTree/projectPanel.ts index d2cc129452..a86ae010b6 100644 --- a/styles/src/styleTree/projectPanel.ts +++ b/styles/src/styleTree/projectPanel.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { background, border, foreground, text } from "./components" export default function projectPanel(colorScheme: ColorScheme) { @@ -24,8 +24,8 @@ export default function projectPanel(colorScheme: ColorScheme) { : colorScheme.ramps.green(0.5).hex(), conflict: isLight ? colorScheme.ramps.red(0.6).hex() - : colorScheme.ramps.red(0.5).hex() - } + : colorScheme.ramps.red(0.5).hex(), + }, } let entry = { @@ -44,7 +44,7 @@ export default function projectPanel(colorScheme: ColorScheme) { background: background(layer, "active"), text: text(layer, "mono", "active", { size: "sm" }), }, - status + status, } return { @@ -79,7 +79,7 @@ export default function projectPanel(colorScheme: ColorScheme) { text: text(layer, "mono", "on", { size: "sm" }), background: withOpacity(background(layer, "on"), 0.9), border: border(layer), - status + status, }, ignoredEntry: { ...entry, @@ -88,7 +88,7 @@ export default function projectPanel(colorScheme: ColorScheme) { active: { ...entry.active, iconColor: foreground(layer, "variant"), - } + }, }, cutEntry: { ...entry, diff --git a/styles/src/styleTree/projectSharedNotification.ts b/styles/src/styleTree/projectSharedNotification.ts index af731f8850..d05eb1b0c5 100644 --- a/styles/src/styleTree/projectSharedNotification.ts +++ b/styles/src/styleTree/projectSharedNotification.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, text } from "./components" export default function projectSharedNotification( diff --git a/styles/src/styleTree/search.ts b/styles/src/styleTree/search.ts index 2094a2e369..d69c4bb2d9 100644 --- a/styles/src/styleTree/search.ts +++ b/styles/src/styleTree/search.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { background, border, foreground, text } from "./components" export default function search(colorScheme: ColorScheme) { @@ -30,7 +30,7 @@ export default function search(colorScheme: ColorScheme) { ...editor, minWidth: 100, maxWidth: 250, - }; + } return { // TODO: Add an activeMatchBackground on the rust side to differentiate between active and inactive diff --git a/styles/src/styleTree/sharedScreen.ts b/styles/src/styleTree/sharedScreen.ts index cd0d44236e..2563e718ff 100644 --- a/styles/src/styleTree/sharedScreen.ts +++ b/styles/src/styleTree/sharedScreen.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background } from "./components" export default function sharedScreen(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/simpleMessageNotification.ts b/styles/src/styleTree/simpleMessageNotification.ts index 2e057ed783..8d88f05c53 100644 --- a/styles/src/styleTree/simpleMessageNotification.ts +++ b/styles/src/styleTree/simpleMessageNotification.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, foreground, text } from "./components" const headerPadding = 8 diff --git a/styles/src/styleTree/statusBar.ts b/styles/src/styleTree/statusBar.ts index eca537c150..a8d926f40e 100644 --- a/styles/src/styleTree/statusBar.ts +++ b/styles/src/styleTree/statusBar.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, foreground, text } from "./components" export default function statusBar(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/tabBar.ts b/styles/src/styleTree/tabBar.ts index 39a1ef0407..c5b397b34a 100644 --- a/styles/src/styleTree/tabBar.ts +++ b/styles/src/styleTree/tabBar.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { text, border, background, foreground } from "./components" export default function tabBar(colorScheme: ColorScheme) { @@ -96,7 +96,7 @@ export default function tabBar(colorScheme: ColorScheme) { }, active: { color: foreground(layer, "accent"), - } + }, }, paneButtonContainer: { background: tab.background, diff --git a/styles/src/styleTree/terminal.ts b/styles/src/styleTree/terminal.ts index 4158ace300..8a7eb7a549 100644 --- a/styles/src/styleTree/terminal.ts +++ b/styles/src/styleTree/terminal.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" export default function terminal(colorScheme: ColorScheme) { /** diff --git a/styles/src/styleTree/tooltip.ts b/styles/src/styleTree/tooltip.ts index bb66cd413c..1666ce5658 100644 --- a/styles/src/styleTree/tooltip.ts +++ b/styles/src/styleTree/tooltip.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { background, border, text } from "./components" export default function tooltip(colorScheme: ColorScheme) { diff --git a/styles/src/styleTree/updateNotification.ts b/styles/src/styleTree/updateNotification.ts index 44472a7662..281012e62f 100644 --- a/styles/src/styleTree/updateNotification.ts +++ b/styles/src/styleTree/updateNotification.ts @@ -1,4 +1,4 @@ -import { ColorScheme } from "../themes/common/colorScheme" +import { ColorScheme } from "../theme/colorScheme" import { foreground, text } from "./components" const headerPadding = 8 diff --git a/styles/src/styleTree/welcome.ts b/styles/src/styleTree/welcome.ts index 23e29c4a40..10e6e02b95 100644 --- a/styles/src/styleTree/welcome.ts +++ b/styles/src/styleTree/welcome.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { border, background, diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index ec992c9c0b..ae8de178f8 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -1,5 +1,5 @@ -import { ColorScheme } from "../themes/common/colorScheme" -import { withOpacity } from "../utils/color" +import { ColorScheme } from "../theme/colorScheme" +import { withOpacity } from "../theme/color" import { background, border, @@ -123,7 +123,7 @@ export default function workspace(colorScheme: ColorScheme) { cursor: "Arrow", background: isLight ? withOpacity(background(colorScheme.lowest), 0.8) - : withOpacity(background(colorScheme.highest), 0.6) + : withOpacity(background(colorScheme.highest), 0.6), }, zoomedPaneForeground: { margin: 16, @@ -143,7 +143,7 @@ export default function workspace(colorScheme: ColorScheme) { }, right: { border: border(layer, { left: true }), - } + }, }, paneDivider: { color: borderColor(layer), diff --git a/styles/src/utils/color.ts b/styles/src/theme/color.ts similarity index 100% rename from styles/src/utils/color.ts rename to styles/src/theme/color.ts diff --git a/styles/src/themes/common/colorScheme.ts b/styles/src/theme/colorScheme.ts similarity index 99% rename from styles/src/themes/common/colorScheme.ts rename to styles/src/theme/colorScheme.ts index d4ef2ae013..9a81073086 100644 --- a/styles/src/themes/common/colorScheme.ts +++ b/styles/src/theme/colorScheme.ts @@ -5,7 +5,7 @@ import { ThemeConfig, ThemeAppearance, ThemeConfigInputColors, -} from "../../themeConfig" +} from "./themeConfig" import { getRamps } from "./ramps" export interface ColorScheme { diff --git a/styles/src/themes/common/index.ts b/styles/src/theme/index.ts similarity index 69% rename from styles/src/themes/common/index.ts rename to styles/src/theme/index.ts index cd7cf9b0f4..2bf625521c 100644 --- a/styles/src/themes/common/index.ts +++ b/styles/src/theme/index.ts @@ -1,4 +1,4 @@ export * from "./colorScheme" export * from "./ramps" export * from "./syntax" -export * from "../../themeConfig" +export * from "./themeConfig" diff --git a/styles/src/themes/common/ramps.ts b/styles/src/theme/ramps.ts similarity index 98% rename from styles/src/themes/common/ramps.ts rename to styles/src/theme/ramps.ts index ef93ee163f..f8c44ba3f9 100644 --- a/styles/src/themes/common/ramps.ts +++ b/styles/src/theme/ramps.ts @@ -3,7 +3,7 @@ import { RampSet } from "./colorScheme" import { ThemeConfigInputColors, ThemeConfigInputColorsKeys, -} from "../../themeConfig" +} from "./themeConfig" export function colorRamp(color: Color): Scale { let endColor = color.desaturate(1).brighten(5) diff --git a/styles/src/themes/common/syntax.ts b/styles/src/theme/syntax.ts similarity index 99% rename from styles/src/themes/common/syntax.ts rename to styles/src/theme/syntax.ts index 258b845142..380fd31786 100644 --- a/styles/src/themes/common/syntax.ts +++ b/styles/src/theme/syntax.ts @@ -1,5 +1,5 @@ import deepmerge from "deepmerge" -import { FontWeight, fontWeights } from "../../common" +import { FontWeight, fontWeights } from "../common" import { ColorScheme } from "./colorScheme" import chroma from "chroma-js" diff --git a/styles/src/themeConfig.ts b/styles/src/theme/themeConfig.ts similarity index 98% rename from styles/src/themeConfig.ts rename to styles/src/theme/themeConfig.ts index 5c62b94c52..0342d0226d 100644 --- a/styles/src/themeConfig.ts +++ b/styles/src/theme/themeConfig.ts @@ -1,5 +1,5 @@ import { Scale, Color } from "chroma-js" -import { Syntax } from "./themes/common/syntax" +import { Syntax } from "./syntax" interface ThemeMeta { /** The name of the theme */ diff --git a/styles/src/themes/index.ts b/styles/src/themes/index.ts index 52ce1b9327..75853bc042 100644 --- a/styles/src/themes/index.ts +++ b/styles/src/themes/index.ts @@ -1,4 +1,4 @@ -import { ThemeConfig } from "./common" +import { ThemeConfig } from "../theme" import { darkDefault as gruvboxDark } from "./gruvbox/gruvbox-dark" import { darkHard as gruvboxDarkHard } from "./gruvbox/gruvbox-dark-hard" import { darkSoft as gruvboxDarkSoft } from "./gruvbox/gruvbox-dark-soft" From 34e134fafba92652f6f699389ea5b55d440d9ba3 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 14:10:17 -0700 Subject: [PATCH 16/29] Fix several randomized test failures with the new git status implementation --- .../src/tests/randomized_integration_tests.rs | 30 +++++++++++++------ crates/fs/src/fs.rs | 18 ++++++++--- crates/project/src/worktree.rs | 16 +++++++--- crates/sum_tree/src/sum_tree.rs | 5 ++++ 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 7a46d4c3f6..54f8515d6e 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -422,7 +422,7 @@ async fn apply_client_operation( ); ensure_project_shared(&project, client, cx).await; - if !client.fs.paths().contains(&new_root_path) { + if !client.fs.paths(false).contains(&new_root_path) { client.fs.create_dir(&new_root_path).await.unwrap(); } project @@ -743,7 +743,7 @@ async fn apply_client_operation( } => { if !client .fs - .directories() + .directories(false) .contains(&path.parent().unwrap().to_owned()) { return Err(TestError::Inapplicable); @@ -770,10 +770,16 @@ async fn apply_client_operation( repo_path, contents, } => { - if !client.fs.directories().contains(&repo_path) { + if !client.fs.directories(false).contains(&repo_path) { return Err(TestError::Inapplicable); } + for (path, _) in contents.iter() { + if !client.fs.files().contains(&repo_path.join(path)) { + return Err(TestError::Inapplicable); + } + } + log::info!( "{}: writing git index for repo {:?}: {:?}", client.username, @@ -795,7 +801,7 @@ async fn apply_client_operation( repo_path, new_branch, } => { - if !client.fs.directories().contains(&repo_path) { + if !client.fs.directories(false).contains(&repo_path) { return Err(TestError::Inapplicable); } @@ -817,9 +823,14 @@ async fn apply_client_operation( statuses, git_operation, } => { - if !client.fs.directories().contains(&repo_path) { + if !client.fs.directories(false).contains(&repo_path) { return Err(TestError::Inapplicable); } + for (path, _) in statuses.iter() { + if !client.fs.files().contains(&repo_path.join(path)) { + return Err(TestError::Inapplicable); + } + } log::info!( "{}: writing git statuses for repo {:?}: {:?}", @@ -920,9 +931,10 @@ fn check_consistency_between_clients(clients: &[(Rc, TestAppContext) assert_eq!( guest_snapshot.entries(false).collect::>(), host_snapshot.entries(false).collect::>(), - "{} has different snapshot than the host for worktree {:?} and project {:?}", + "{} has different snapshot than the host for worktree {:?} ({:?}) and project {:?}", client.username, host_snapshot.abs_path(), + id, guest_project.remote_id(), ); assert_eq!(guest_snapshot.repositories().collect::>(), host_snapshot.repositories().collect::>(), @@ -1583,7 +1595,7 @@ impl TestPlan { .choose(&mut self.rng) .cloned() else { continue }; let project_root_name = root_name_for_project(&project, cx); - let mut paths = client.fs.paths(); + let mut paths = client.fs.paths(false); paths.remove(0); let new_root_path = if paths.is_empty() || self.rng.gen() { Path::new("/").join(&self.next_root_dir_name(user_id)) @@ -1763,7 +1775,7 @@ impl TestPlan { let is_dir = self.rng.gen::(); let content; let mut path; - let dir_paths = client.fs.directories(); + let dir_paths = client.fs.directories(false); if is_dir { content = String::new(); @@ -1817,7 +1829,7 @@ impl TestPlan { let repo_path = client .fs - .directories() + .directories(false) .choose(&mut self.rng) .unwrap() .clone(); diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index e8b309242d..aff7439f0a 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -12,6 +12,7 @@ use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; use std::cmp; +use std::ffi::OsStr; use std::io::Write; use std::sync::Arc; use std::{ @@ -501,6 +502,11 @@ impl FakeFsState { } } +#[cfg(any(test, feature = "test-support"))] +lazy_static! { + pub static ref FS_DOT_GIT: &'static OsStr = OsStr::new(".git"); +} + #[cfg(any(test, feature = "test-support"))] impl FakeFs { pub fn new(executor: Arc) -> Arc { @@ -693,7 +699,7 @@ impl FakeFs { }); } - pub fn paths(&self) -> Vec { + pub fn paths(&self, include_dot_git: bool) -> Vec { let mut result = Vec::new(); let mut queue = collections::VecDeque::new(); queue.push_back((PathBuf::from("/"), self.state.lock().root.clone())); @@ -703,12 +709,14 @@ impl FakeFs { queue.push_back((path.join(name), entry.clone())); } } - result.push(path); + if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) { + result.push(path); + } } result } - pub fn directories(&self) -> Vec { + pub fn directories(&self, include_dot_git: bool) -> Vec { let mut result = Vec::new(); let mut queue = collections::VecDeque::new(); queue.push_back((PathBuf::from("/"), self.state.lock().root.clone())); @@ -717,7 +725,9 @@ impl FakeFs { for (name, entry) in entries { queue.push_back((path.join(name), entry.clone())); } - result.push(path); + if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) { + result.push(path); + } } } result diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 27dc890ec0..abaddd450e 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -2541,7 +2541,7 @@ impl Entry { } pub fn git_status(&self) -> Option { - self.git_status /*.status() */ + self.git_status } } @@ -3149,12 +3149,14 @@ impl BackgroundScanner { // refreshed. Do this before adding any new entries, so that renames can be // detected regardless of the order of the paths. let mut event_paths = Vec::>::with_capacity(abs_paths.len()); + let mut event_metadata = Vec::<_>::with_capacity(abs_paths.len()); for (abs_path, metadata) in abs_paths.iter().zip(metadata.iter()) { if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) { if matches!(metadata, Ok(None)) || doing_recursive_update { state.remove_path(path); } event_paths.push(path.into()); + event_metadata.push(metadata); } else { log::error!( "unexpected event {:?} for root path {:?}", @@ -3164,7 +3166,7 @@ impl BackgroundScanner { } } - for (path, metadata) in event_paths.iter().cloned().zip(metadata.into_iter()) { + for (path, metadata) in event_paths.iter().cloned().zip(event_metadata.into_iter()) { let abs_path: Arc = root_abs_path.join(&path).into(); match metadata { @@ -3487,7 +3489,6 @@ impl BackgroundScanner { if new_paths.item().map_or(false, |e| e.path < path.0) { new_paths.seek_forward(&path, Bias::Left, &()); } - loop { match (old_paths.item(), new_paths.item()) { (Some(old_entry), Some(new_entry)) => { @@ -3506,6 +3507,13 @@ impl BackgroundScanner { } Ordering::Equal => { if self.phase == EventsReceivedDuringInitialScan { + if old_entry.id != new_entry.id { + changes.push(( + old_entry.path.clone(), + old_entry.id, + Removed, + )); + } // If the worktree was not fully initialized when this event was generated, // we can't know whether this entry was added during the scan or whether // it was merely updated. @@ -4685,7 +4693,7 @@ mod tests { log::info!("mutating fs"); let mut files = Vec::new(); let mut dirs = Vec::new(); - for path in fs.as_fake().paths() { + for path in fs.as_fake().paths(false) { if path.starts_with(root_path) { if fs.is_file(&path).await { files.push(path); diff --git a/crates/sum_tree/src/sum_tree.rs b/crates/sum_tree/src/sum_tree.rs index 36f0f926cd..6c6150aa3a 100644 --- a/crates/sum_tree/src/sum_tree.rs +++ b/crates/sum_tree/src/sum_tree.rs @@ -480,6 +480,11 @@ impl SumTree { } => child_trees.last().unwrap().rightmost_leaf(), } } + + #[cfg(debug_assertions)] + pub fn _debug_entries(&self) -> Vec<&T> { + self.iter().collect::>() + } } impl PartialEq for SumTree { From 0ac7a3bc21ea8a27db3eab10830c1f44ea9e37f6 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 14:13:57 -0700 Subject: [PATCH 17/29] fmt --- crates/fs/src/fs.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 60e0a14461..2e0a03dbeb 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -709,7 +709,11 @@ impl FakeFs { queue.push_back((path.join(name), entry.clone())); } } - if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) { + if include_dot_git + || !path + .components() + .any(|component| component.as_os_str() == *FS_DOT_GIT) + { result.push(path); } } @@ -725,7 +729,11 @@ impl FakeFs { for (name, entry) in entries { queue.push_back((path.join(name), entry.clone())); } - if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) { + if include_dot_git + || !path + .components() + .any(|component| component.as_os_str() == *FS_DOT_GIT) + { result.push(path); } } From 03a96d2793b9b187d4bc636440597132d7ae643d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 14:15:20 -0700 Subject: [PATCH 18/29] Feature gate import --- crates/fs/src/fs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 2e0a03dbeb..16936fc6d4 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -12,7 +12,6 @@ use rope::Rope; use smol::io::{AsyncReadExt, AsyncWriteExt}; use std::borrow::Cow; use std::cmp; -use std::ffi::OsStr; use std::io::Write; use std::sync::Arc; use std::{ @@ -31,6 +30,8 @@ use collections::{btree_map, BTreeMap}; use repository::{FakeGitRepositoryState, GitFileStatus}; #[cfg(any(test, feature = "test-support"))] use std::sync::Weak; +#[cfg(any(test, feature = "test-support"))] +use std::ffi::OsStr; lazy_static! { static ref LINE_SEPARATORS_REGEX: Regex = Regex::new("\r\n|\r|\u{2028}|\u{2029}").unwrap(); From cd63ec2c7f3100edec902bc56408784a314179d2 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 14:20:01 -0700 Subject: [PATCH 19/29] fmt --- crates/fs/src/fs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 16936fc6d4..fee7765d49 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -29,9 +29,9 @@ use collections::{btree_map, BTreeMap}; #[cfg(any(test, feature = "test-support"))] use repository::{FakeGitRepositoryState, GitFileStatus}; #[cfg(any(test, feature = "test-support"))] -use std::sync::Weak; -#[cfg(any(test, feature = "test-support"))] use std::ffi::OsStr; +#[cfg(any(test, feature = "test-support"))] +use std::sync::Weak; lazy_static! { static ref LINE_SEPARATORS_REGEX: Regex = Regex::new("\r\n|\r|\u{2028}|\u{2029}").unwrap(); From 2c5e83bf724c2f2f589d6793793dcf56eaff873c Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:10:23 -0700 Subject: [PATCH 20/29] Fixed a bug where buffer saved clocks would desynchronize in rare execution paths co-authored-by: Max --- crates/clock/src/clock.rs | 1 + .../src/tests/randomized_integration_tests.rs | 11 +++++----- crates/project/src/project.rs | 21 +++++++++---------- crates/project/src/worktree.rs | 8 +++---- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/crates/clock/src/clock.rs b/crates/clock/src/clock.rs index 89f8fda3ee..bc936fcb99 100644 --- a/crates/clock/src/clock.rs +++ b/crates/clock/src/clock.rs @@ -66,6 +66,7 @@ impl<'a> AddAssign<&'a Local> for Local { } } +/// A vector clock #[derive(Clone, Default, Hash, Eq, PartialEq)] pub struct Global(SmallVec<[u32; 8]>); diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 54f8515d6e..a95938f6b8 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -628,12 +628,13 @@ async fn apply_client_operation( ensure_project_shared(&project, client, cx).await; let requested_version = buffer.read_with(cx, |buffer, _| buffer.version()); - let save = project.update(cx, |project, cx| project.save_buffer(buffer, cx)); - let save = cx.background().spawn(async move { - let (saved_version, _, _) = save - .await + let save = project.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)); + let save = cx.spawn(|cx| async move { + save.await .map_err(|err| anyhow!("save request failed: {:?}", err))?; - assert!(saved_version.observed_all(&requested_version)); + assert!(buffer + .read_with(&cx, |buffer, _| { buffer.saved_version().to_owned() }) + .observed_all(&requested_version)); anyhow::Ok(()) }); if detach { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 6c3f90bedd..3406a3248c 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -37,8 +37,8 @@ use language::{ range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Diff, Event as BufferEvent, File as _, Language, LanguageRegistry, LanguageServerName, LocalFile, OffsetRangeExt, Operation, Patch, - PendingLanguageServer, PointUtf16, RopeFingerprint, TextBufferSnapshot, ToOffset, ToPointUtf16, - Transaction, Unclipped, + PendingLanguageServer, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, + Unclipped, }; use log::error; use lsp::{ @@ -69,7 +69,7 @@ use std::{ atomic::{AtomicUsize, Ordering::SeqCst}, Arc, }, - time::{Duration, Instant, SystemTime}, + time::{Duration, Instant}, }; use terminals::Terminals; use util::{ @@ -1617,7 +1617,7 @@ impl Project { &self, buffer: ModelHandle, cx: &mut ModelContext, - ) -> Task> { + ) -> Task> { let Some(file) = File::from_dyn(buffer.read(cx).file()) else { return Task::ready(Err(anyhow!("buffer doesn't have a file"))); }; @@ -5985,16 +5985,15 @@ impl Project { .await?; let buffer_id = buffer.read_with(&cx, |buffer, _| buffer.remote_id()); - let (saved_version, fingerprint, mtime) = this - .update(&mut cx, |this, cx| this.save_buffer(buffer, cx)) + this.update(&mut cx, |this, cx| this.save_buffer(buffer.clone(), cx)) .await?; - Ok(proto::BufferSaved { + Ok(buffer.read_with(&cx, |buffer, _| proto::BufferSaved { project_id, buffer_id, - version: serialize_version(&saved_version), - mtime: Some(mtime.into()), - fingerprint: language::proto::serialize_fingerprint(fingerprint), - }) + version: serialize_version(buffer.saved_version()), + mtime: Some(buffer.saved_mtime().into()), + fingerprint: language::proto::serialize_fingerprint(buffer.saved_version_fingerprint()), + })) } async fn handle_reload_buffers( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 0fa22c7e66..eb5ad83db8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -923,7 +923,7 @@ impl LocalWorktree { path: Arc, has_changed_file: bool, cx: &mut ModelContext, - ) -> Task> { + ) -> Task> { let handle = cx.handle(); let buffer = buffer_handle.read(cx); @@ -979,7 +979,7 @@ impl LocalWorktree { buffer.did_save(version.clone(), fingerprint, entry.mtime, cx); }); - Ok((version, fingerprint, entry.mtime)) + Ok(()) }) } @@ -1290,7 +1290,7 @@ impl RemoteWorktree { &self, buffer_handle: ModelHandle, cx: &mut ModelContext, - ) -> Task> { + ) -> Task> { let buffer = buffer_handle.read(cx); let buffer_id = buffer.remote_id(); let version = buffer.version(); @@ -1315,7 +1315,7 @@ impl RemoteWorktree { buffer.did_save(version.clone(), fingerprint, mtime, cx); }); - Ok((version, fingerprint, mtime)) + Ok(()) }) } From 572d40381a348aaa7e1fd25600c4ba4c79dbb16c Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:39:10 -0700 Subject: [PATCH 21/29] Add track caller --- crates/project/src/worktree.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index eb5ad83db8..d50dc24e67 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5433,6 +5433,7 @@ mod tests { ], ); + #[track_caller] fn check_propagated_statuses( snapshot: &Snapshot, expected_statuses: &[(&Path, Option)], From 5f143f689fb3a9b54d5cba980a7f8fdc3a3863e5 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:44:13 -0700 Subject: [PATCH 22/29] Attempting to debug on ci... --- crates/project/src/worktree.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index d50dc24e67..f9ed2552d4 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5407,6 +5407,8 @@ mod tests { ], ); + panic!(); + check_propagated_statuses( &snapshot, &[ @@ -5443,11 +5445,12 @@ mod tests { .map(|(path, _)| snapshot.entry_for_path(path).unwrap().clone()) .collect::>(); snapshot.propagate_git_statuses(&mut entries); + dbg!(&entries); assert_eq!( - entries + dbg!(entries .iter() .map(|e| (e.path.as_ref(), e.git_status)) - .collect::>(), + .collect::>()), expected_statuses ); } From 9d58c4526d23f6182a68633a1db8a8bf16d522ae Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:45:36 -0700 Subject: [PATCH 23/29] Fix warning --- crates/project/src/worktree.rs | 48 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index f9ed2552d4..a43564d065 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5409,31 +5409,31 @@ mod tests { panic!(); - check_propagated_statuses( - &snapshot, - &[ - (Path::new("a/b"), Some(GitFileStatus::Added)), - (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), - (Path::new("a/b/c2.txt"), None), - (Path::new("a/d"), Some(GitFileStatus::Modified)), - (Path::new("a/d/e1.txt"), None), - (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), - (Path::new("f"), None), - (Path::new("f/no-status.txt"), None), - (Path::new("g"), Some(GitFileStatus::Conflict)), - ], - ); + // check_propagated_statuses( + // &snapshot, + // &[ + // (Path::new("a/b"), Some(GitFileStatus::Added)), + // (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + // (Path::new("a/b/c2.txt"), None), + // (Path::new("a/d"), Some(GitFileStatus::Modified)), + // (Path::new("a/d/e1.txt"), None), + // (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + // (Path::new("f"), None), + // (Path::new("f/no-status.txt"), None), + // (Path::new("g"), Some(GitFileStatus::Conflict)), + // ], + // ); - check_propagated_statuses( - &snapshot, - &[ - (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), - (Path::new("a/b/c2.txt"), None), - (Path::new("a/d/e1.txt"), None), - (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), - (Path::new("f/no-status.txt"), None), - ], - ); + // check_propagated_statuses( + // &snapshot, + // &[ + // (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + // (Path::new("a/b/c2.txt"), None), + // (Path::new("a/d/e1.txt"), None), + // (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + // (Path::new("f/no-status.txt"), None), + // ], + // ); #[track_caller] fn check_propagated_statuses( From 9e9d8e3a7be137bb076bee05c11cc925c7c61b4f Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:50:15 -0700 Subject: [PATCH 24/29] add mroe dbg --- crates/project/src/worktree.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index a43564d065..efb898bd6c 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5444,6 +5444,7 @@ mod tests { .iter() .map(|(path, _)| snapshot.entry_for_path(path).unwrap().clone()) .collect::>(); + dbg!(&entries); snapshot.propagate_git_statuses(&mut entries); dbg!(&entries); assert_eq!( From 8ca1a7d43d441fa4c4f7ddcdbe75d7bf87c3cbbf Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:51:54 -0700 Subject: [PATCH 25/29] add scan_complete await --- crates/project/src/worktree.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index efb898bd6c..8972a55604 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5387,6 +5387,9 @@ mod tests { .await .unwrap(); + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + cx.foreground().run_until_parked(); let snapshot = tree.read_with(cx, |tree, _| tree.snapshot()); From fc1f8c5657684c58bf01bb28a80d77463115238a Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 7 Jun 2023 16:58:55 -0700 Subject: [PATCH 26/29] Fixed ci --- crates/project/src/worktree.rs | 56 ++++++++++++++++------------------ 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 8972a55604..ee190e1a31 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5410,33 +5410,31 @@ mod tests { ], ); - panic!(); + check_propagated_statuses( + &snapshot, + &[ + (Path::new("a/b"), Some(GitFileStatus::Added)), + (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + (Path::new("a/b/c2.txt"), None), + (Path::new("a/d"), Some(GitFileStatus::Modified)), + (Path::new("a/d/e1.txt"), None), + (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + (Path::new("f"), None), + (Path::new("f/no-status.txt"), None), + (Path::new("g"), Some(GitFileStatus::Conflict)), + ], + ); - // check_propagated_statuses( - // &snapshot, - // &[ - // (Path::new("a/b"), Some(GitFileStatus::Added)), - // (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), - // (Path::new("a/b/c2.txt"), None), - // (Path::new("a/d"), Some(GitFileStatus::Modified)), - // (Path::new("a/d/e1.txt"), None), - // (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), - // (Path::new("f"), None), - // (Path::new("f/no-status.txt"), None), - // (Path::new("g"), Some(GitFileStatus::Conflict)), - // ], - // ); - - // check_propagated_statuses( - // &snapshot, - // &[ - // (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), - // (Path::new("a/b/c2.txt"), None), - // (Path::new("a/d/e1.txt"), None), - // (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), - // (Path::new("f/no-status.txt"), None), - // ], - // ); + check_propagated_statuses( + &snapshot, + &[ + (Path::new("a/b/c1.txt"), Some(GitFileStatus::Added)), + (Path::new("a/b/c2.txt"), None), + (Path::new("a/d/e1.txt"), None), + (Path::new("a/d/e2.txt"), Some(GitFileStatus::Modified)), + (Path::new("f/no-status.txt"), None), + ], + ); #[track_caller] fn check_propagated_statuses( @@ -5447,14 +5445,12 @@ mod tests { .iter() .map(|(path, _)| snapshot.entry_for_path(path).unwrap().clone()) .collect::>(); - dbg!(&entries); snapshot.propagate_git_statuses(&mut entries); - dbg!(&entries); assert_eq!( - dbg!(entries + entries .iter() .map(|e| (e.path.as_ref(), e.git_status)) - .collect::>()), + .collect::>(), expected_statuses ); } From c8a9d73ea685d172a49322c5922512a49c4dcdec Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 8 Jun 2023 00:37:00 -0400 Subject: [PATCH 27/29] Add foundation for exporting tokens --- styles/package-lock.json | 11 ++++++++ styles/package.json | 4 ++- styles/src/buildTokens.ts | 39 ++++++++++++++++++++++++++ styles/src/theme/tokens/colorScheme.ts | 12 ++++++++ styles/src/theme/tokens/players.ts | 28 ++++++++++++++++++ styles/src/theme/tokens/token.ts | 14 +++++++++ styles/src/utils/slugify.ts | 1 + 7 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 styles/src/buildTokens.ts create mode 100644 styles/src/theme/tokens/colorScheme.ts create mode 100644 styles/src/theme/tokens/players.ts create mode 100644 styles/src/theme/tokens/token.ts create mode 100644 styles/src/utils/slugify.ts diff --git a/styles/package-lock.json b/styles/package-lock.json index c03c27c1e8..d1d0ed0eb8 100644 --- a/styles/package-lock.json +++ b/styles/package-lock.json @@ -9,6 +9,7 @@ "version": "1.0.0", "license": "ISC", "dependencies": { + "@tokens-studio/types": "^0.2.3", "@types/chroma-js": "^2.4.0", "@types/node": "^18.14.1", "ayu": "^8.0.1", @@ -53,6 +54,11 @@ "@jridgewell/sourcemap-codec": "^1.4.10" } }, + "node_modules/@tokens-studio/types": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/@tokens-studio/types/-/types-0.2.3.tgz", + "integrity": "sha512-2KN3V0JPf+Zh8aoVMwykJq29Lsi7vYgKGYBQ/zQ+FbDEmrH6T/Vwn8kG7cvbTmW1JAAvgxVxMIivgC9PmFelNA==" + }, "node_modules/@tsconfig/node10": { "version": "1.0.9", "resolved": "https://registry.npmjs.org/@tsconfig/node10/-/node10-1.0.9.tgz", @@ -271,6 +277,11 @@ "@jridgewell/sourcemap-codec": "^1.4.10" } }, + "@tokens-studio/types": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/@tokens-studio/types/-/types-0.2.3.tgz", + "integrity": "sha512-2KN3V0JPf+Zh8aoVMwykJq29Lsi7vYgKGYBQ/zQ+FbDEmrH6T/Vwn8kG7cvbTmW1JAAvgxVxMIivgC9PmFelNA==" + }, "@tsconfig/node10": { "version": "1.0.9", "resolved": "https://registry.npmjs.org/@tsconfig/node10/-/node10-1.0.9.tgz", diff --git a/styles/package.json b/styles/package.json index 5f103c5d6c..2a0881863b 100644 --- a/styles/package.json +++ b/styles/package.json @@ -5,11 +5,13 @@ "main": "index.js", "scripts": { "build": "ts-node ./src/buildThemes.ts", - "build-licenses": "ts-node ./src/buildLicenses.ts" + "build-licenses": "ts-node ./src/buildLicenses.ts", + "build-tokens": "ts-node ./src/buildTokens.ts" }, "author": "", "license": "ISC", "dependencies": { + "@tokens-studio/types": "^0.2.3", "@types/chroma-js": "^2.4.0", "@types/node": "^18.14.1", "ayu": "^8.0.1", diff --git a/styles/src/buildTokens.ts b/styles/src/buildTokens.ts new file mode 100644 index 0000000000..81ffd3f3cb --- /dev/null +++ b/styles/src/buildTokens.ts @@ -0,0 +1,39 @@ +import * as fs from "fs" +import * as path from "path" +import { ColorScheme, createColorScheme } from "./common" +import { themes } from "./themes" +import { slugify } from "./utils/slugify" +import { colorSchemeTokens } from "./theme/tokens/colorScheme" + +const TOKENS_DIRECTORY = path.join(__dirname, "..", "target", "tokens") + +function clearTokens(tokensDirectory: string) { + if (!fs.existsSync(tokensDirectory)) { + fs.mkdirSync(tokensDirectory, { recursive: true }) + } else { + for (const file of fs.readdirSync(tokensDirectory)) { + if (file.endsWith(".json")) { + fs.unlinkSync(path.join(tokensDirectory, file)) + } + } + } +} + +function writeTokens(colorSchemes: ColorScheme[], tokensDirectory: string) { + clearTokens(tokensDirectory) + + for (const colorScheme of colorSchemes) { + const fileName = slugify(colorScheme.name) + const tokens = colorSchemeTokens(colorScheme) + const tokensJSON = JSON.stringify(tokens, null, 2) + const outPath = path.join(tokensDirectory, `${fileName}.json`) + fs.writeFileSync(outPath, tokensJSON) + console.log(`- ${outPath} created`) + } +} + +const colorSchemes: ColorScheme[] = themes.map((theme) => + createColorScheme(theme) +) + +writeTokens(colorSchemes, TOKENS_DIRECTORY) diff --git a/styles/src/theme/tokens/colorScheme.ts b/styles/src/theme/tokens/colorScheme.ts new file mode 100644 index 0000000000..eaf0705e14 --- /dev/null +++ b/styles/src/theme/tokens/colorScheme.ts @@ -0,0 +1,12 @@ +import { ColorScheme } from "../colorScheme" +import { PlayerTokens, players } from "./players" + +interface ColorSchemeTokens { + players: PlayerTokens +} + +export function colorSchemeTokens(colorScheme: ColorScheme): ColorSchemeTokens { + return { + players: players(colorScheme), + } +} diff --git a/styles/src/theme/tokens/players.ts b/styles/src/theme/tokens/players.ts new file mode 100644 index 0000000000..c65fb6885e --- /dev/null +++ b/styles/src/theme/tokens/players.ts @@ -0,0 +1,28 @@ +import { SingleColorToken } from "@tokens-studio/types" +import { ColorScheme, Players } from "../../common" +import { colorToken } from "./token" + +export type PlayerToken = Record<"selection" | "cursor", SingleColorToken> + +export type PlayerTokens = Record + +function buildPlayerToken(colorScheme: ColorScheme, index: number): PlayerToken { + + const playerNumber = index.toString() as keyof Players + + return { + selection: colorToken(`player${index}Selection`, colorScheme.players[playerNumber].selection), + cursor: colorToken(`player${index}Cursor`, colorScheme.players[playerNumber].cursor), + } +} + +export const players = (colorScheme: ColorScheme): PlayerTokens => ({ + "0": buildPlayerToken(colorScheme, 0), + "1": buildPlayerToken(colorScheme, 1), + "2": buildPlayerToken(colorScheme, 2), + "3": buildPlayerToken(colorScheme, 3), + "4": buildPlayerToken(colorScheme, 4), + "5": buildPlayerToken(colorScheme, 5), + "6": buildPlayerToken(colorScheme, 6), + "7": buildPlayerToken(colorScheme, 7) +}) diff --git a/styles/src/theme/tokens/token.ts b/styles/src/theme/tokens/token.ts new file mode 100644 index 0000000000..3e5187dd64 --- /dev/null +++ b/styles/src/theme/tokens/token.ts @@ -0,0 +1,14 @@ +import { SingleColorToken, TokenTypes } from "@tokens-studio/types" + +export function colorToken(name: string, value: string, description?: string): SingleColorToken { + const token: SingleColorToken = { + name, + type: TokenTypes.COLOR, + value, + description, + } + + if (!token.value || token.value === '') throw new Error("Color token must have a value") + + return token +} diff --git a/styles/src/utils/slugify.ts b/styles/src/utils/slugify.ts new file mode 100644 index 0000000000..62b226cd10 --- /dev/null +++ b/styles/src/utils/slugify.ts @@ -0,0 +1 @@ +export function slugify(t: string): string { return t.toString().toLowerCase().replace(/\s+/g, '-').replace(/[^\w\-]+/g, '').replace(/\-\-+/g, '-').replace(/^-+/, '').replace(/-+$/, '') } From e996a66596d8f011c11e630f951c81b00aee87bc Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 8 Jun 2023 01:15:57 -0400 Subject: [PATCH 28/29] Update TSCondif Based on #2558. Also fixes errors resulting from the stricter options. --- styles/src/theme/themeConfig.ts | 5 +++++ styles/src/themes/atelier/common.ts | 4 ++-- styles/src/themes/ayu/common.ts | 4 ++-- styles/src/themes/gruvbox/gruvbox-common.ts | 3 ++- styles/tsconfig.json | 16 +++++++++++++++- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/styles/src/theme/themeConfig.ts b/styles/src/theme/themeConfig.ts index 0342d0226d..176ae83bb7 100644 --- a/styles/src/theme/themeConfig.ts +++ b/styles/src/theme/themeConfig.ts @@ -23,6 +23,11 @@ interface ThemeMeta { themeUrl?: string } +export type ThemeFamilyMeta = Pick< + ThemeMeta, + "name" | "author" | "licenseType" | "licenseUrl" +> + export interface ThemeConfigInputColors { neutral: Scale red: Scale diff --git a/styles/src/themes/atelier/common.ts b/styles/src/themes/atelier/common.ts index 961ca64270..2e5be61f52 100644 --- a/styles/src/themes/atelier/common.ts +++ b/styles/src/themes/atelier/common.ts @@ -1,4 +1,4 @@ -import { ThemeLicenseType, ThemeConfig, ThemeSyntax } from "../../common" +import { ThemeLicenseType, ThemeSyntax, ThemeFamilyMeta } from "../../common" export interface Variant { colors: { @@ -21,7 +21,7 @@ export interface Variant { } } -export const meta: Partial = { +export const meta: ThemeFamilyMeta = { name: "Atelier", author: "Bram de Haan (http://atelierbramdehaan.nl)", licenseType: ThemeLicenseType.MIT, diff --git a/styles/src/themes/ayu/common.ts b/styles/src/themes/ayu/common.ts index 26e21c1910..1eb2c91916 100644 --- a/styles/src/themes/ayu/common.ts +++ b/styles/src/themes/ayu/common.ts @@ -3,8 +3,8 @@ import { chroma, colorRamp, ThemeLicenseType, - ThemeConfig, ThemeSyntax, + ThemeFamilyMeta, } from "../../common" export const ayu = { @@ -77,7 +77,7 @@ export const buildSyntax = (t: typeof dark): ThemeSyntax => { } } -export const meta: Partial = { +export const meta: ThemeFamilyMeta = { name: "Ayu", author: "dempfi", licenseType: ThemeLicenseType.MIT, diff --git a/styles/src/themes/gruvbox/gruvbox-common.ts b/styles/src/themes/gruvbox/gruvbox-common.ts index f1c04a069c..18e8c5b97e 100644 --- a/styles/src/themes/gruvbox/gruvbox-common.ts +++ b/styles/src/themes/gruvbox/gruvbox-common.ts @@ -5,9 +5,10 @@ import { ThemeLicenseType, ThemeConfig, ThemeSyntax, + ThemeFamilyMeta, } from "../../common" -const meta: Partial = { +const meta: ThemeFamilyMeta = { name: "Gruvbox", author: "morhetz ", licenseType: ThemeLicenseType.MIT, diff --git a/styles/tsconfig.json b/styles/tsconfig.json index 13a5faf32b..6d24728a0a 100644 --- a/styles/tsconfig.json +++ b/styles/tsconfig.json @@ -6,7 +6,21 @@ "noImplicitAny": true, "removeComments": true, "preserveConstEnums": true, - "sourceMap": true + "sourceMap": true, + "noEmit": true, + "forceConsistentCasingInFileNames": true, + "declaration": true, + "strict": true, + "strictNullChecks": true, + "noImplicitThis": true, + "alwaysStrict": true, + "noUnusedLocals": false, + "noUnusedParameters": false, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": false, + "experimentalDecorators": true, + "strictPropertyInitialization": false, + "skipLibCheck": true }, "exclude": ["node_modules"] } From 1e43fec1c5bede80da249b4f2a0f5837da1dd42f Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 8 Jun 2023 01:23:19 -0400 Subject: [PATCH 29/29] Update buildLicenses to only include the theme url if there is one --- styles/src/buildLicenses.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/styles/src/buildLicenses.ts b/styles/src/buildLicenses.ts index 2e1325044d..13a6951a82 100644 --- a/styles/src/buildLicenses.ts +++ b/styles/src/buildLicenses.ts @@ -30,17 +30,19 @@ function generateLicenseFile(themes: ThemeConfig[]) { checkLicenses(themes) for (const theme of themes) { const licenseText = fs.readFileSync(theme.licenseFile).toString() - writeLicense(theme.name, theme.licenseUrl, licenseText) + writeLicense(theme.name, licenseText, theme.licenseUrl) } } function writeLicense( themeName: string, - licenseUrl: string, - licenseText: String + licenseText: string, + licenseUrl?: string ) { process.stdout.write( - `## [${themeName}](${licenseUrl})\n\n${licenseText}\n********************************************************************************\n\n` + licenseUrl + ? `## [${themeName}](${licenseUrl})\n\n${licenseText}\n********************************************************************************\n\n` + : `## ${themeName}\n\n${licenseText}\n********************************************************************************\n\n` ) }