Skip to content

Commit

Permalink
Remove dependencies from the Worktree crate and make it more focused (z…
Browse files Browse the repository at this point in the history
…ed-industries#12747)

The `worktree` crate mainly provides an in-memory model of a directory
and its git repositories. But because it was originally extracted from
the Project crate, it also contained lingering bits of code that were
outside of that area:
* it had a little bit of logic related to buffers (though most buffer
management lives in `project`)
* it had a *little* bit of logic for storing diagnostics (though the
vast majority of LSP and diagnostic logic lives in `project`)
* it had a little bit of logic for sending RPC message (though the
*receiving* logic for those RPC messages lived in `project`)

In this PR, I've moved those concerns entirely to the project crate
(where they were already dealt with for the most part), so that the
worktree crate can be more focused on its main job, and have fewer
dependencies.

Worktree no longer depends on `client` or `lsp`. It still depends on
`language`, but only because of `impl language::File for
worktree::File`.

Release Notes:

- N/A
  • Loading branch information
maxbrunsfeld authored Jun 6, 2024
1 parent 00dfd21 commit 4858116
Show file tree
Hide file tree
Showing 10 changed files with 554 additions and 689 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions crates/collab/src/tests/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ async fn test_host_disconnect(
let project_b = client_b.build_dev_server_project(project_id, cx_b).await;
cx_a.background_executor.run_until_parked();

assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared()));
assert!(worktree_a.read_with(cx_a, |tree, _| tree
.as_local()
.unwrap()
.has_update_observer()));

let workspace_b = cx_b
.add_window(|cx| Workspace::new(None, project_b.clone(), client_b.app_state.clone(), cx));
Expand Down Expand Up @@ -120,7 +123,10 @@ async fn test_host_disconnect(

project_b.read_with(cx_b, |project, _| project.is_read_only());

assert!(worktree_a.read_with(cx_a, |tree, _| !tree.as_local().unwrap().is_shared()));
assert!(worktree_a.read_with(cx_a, |tree, _| !tree
.as_local()
.unwrap()
.has_update_observer()));

// Ensure client B's edited state is reset and that the whole window is blurred.

Expand Down
37 changes: 27 additions & 10 deletions crates/collab/src/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1378,7 +1378,10 @@ async fn test_unshare_project(
let project_b = client_b.build_dev_server_project(project_id, cx_b).await;
executor.run_until_parked();

assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared()));
assert!(worktree_a.read_with(cx_a, |tree, _| tree
.as_local()
.unwrap()
.has_update_observer()));

project_b
.update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
Expand All @@ -1403,7 +1406,10 @@ async fn test_unshare_project(
.unwrap();
executor.run_until_parked();

assert!(worktree_a.read_with(cx_a, |tree, _| !tree.as_local().unwrap().is_shared()));
assert!(worktree_a.read_with(cx_a, |tree, _| !tree
.as_local()
.unwrap()
.has_update_observer()));

assert!(project_c.read_with(cx_c, |project, _| project.is_disconnected()));

Expand All @@ -1415,7 +1421,10 @@ async fn test_unshare_project(
let project_c2 = client_c.build_dev_server_project(project_id, cx_c).await;
executor.run_until_parked();

assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared()));
assert!(worktree_a.read_with(cx_a, |tree, _| tree
.as_local()
.unwrap()
.has_update_observer()));
project_c2
.update(cx_c, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
.await
Expand Down Expand Up @@ -1522,7 +1531,7 @@ async fn test_project_reconnect(
executor.run_until_parked();

let worktree1_id = worktree_a1.read_with(cx_a, |worktree, _| {
assert!(worktree.as_local().unwrap().is_shared());
assert!(worktree.as_local().unwrap().has_update_observer());
worktree.id()
});
let (worktree_a2, _) = project_a1
Expand All @@ -1534,7 +1543,7 @@ async fn test_project_reconnect(
executor.run_until_parked();

let worktree2_id = worktree_a2.read_with(cx_a, |tree, _| {
assert!(tree.as_local().unwrap().is_shared());
assert!(tree.as_local().unwrap().has_update_observer());
tree.id()
});
executor.run_until_parked();
Expand Down Expand Up @@ -1568,7 +1577,7 @@ async fn test_project_reconnect(
});

worktree_a1.read_with(cx_a, |tree, _| {
assert!(tree.as_local().unwrap().is_shared())
assert!(tree.as_local().unwrap().has_update_observer())
});

// While client A is disconnected, add and remove files from client A's project.
Expand Down Expand Up @@ -1611,7 +1620,7 @@ async fn test_project_reconnect(
.await;

let worktree3_id = worktree_a3.read_with(cx_a, |tree, _| {
assert!(!tree.as_local().unwrap().is_shared());
assert!(!tree.as_local().unwrap().has_update_observer());
tree.id()
});
executor.run_until_parked();
Expand All @@ -1634,7 +1643,11 @@ async fn test_project_reconnect(

project_a1.read_with(cx_a, |project, cx| {
assert!(project.is_shared());
assert!(worktree_a1.read(cx).as_local().unwrap().is_shared());
assert!(worktree_a1
.read(cx)
.as_local()
.unwrap()
.has_update_observer());
assert_eq!(
worktree_a1
.read(cx)
Expand All @@ -1652,7 +1665,11 @@ async fn test_project_reconnect(
"subdir2/i.txt"
]
);
assert!(worktree_a3.read(cx).as_local().unwrap().is_shared());
assert!(worktree_a3
.read(cx)
.as_local()
.unwrap()
.has_update_observer());
assert_eq!(
worktree_a3
.read(cx)
Expand Down Expand Up @@ -1733,7 +1750,7 @@ async fn test_project_reconnect(
executor.run_until_parked();

let worktree4_id = worktree_a4.read_with(cx_a, |tree, _| {
assert!(tree.as_local().unwrap().is_shared());
assert!(tree.as_local().unwrap().has_update_observer());
tree.id()
});
project_a1.update(cx_a, |project, cx| {
Expand Down
12 changes: 0 additions & 12 deletions crates/copilot/src/copilot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,6 @@ async fn get_copilot_lsp(http: Arc<dyn HttpClient>) -> anyhow::Result<PathBuf> {
mod tests {
use super::*;
use gpui::TestAppContext;
use language::BufferId;

#[gpui::test(iterations = 10)]
async fn test_buffer_management(cx: &mut TestAppContext) {
Expand Down Expand Up @@ -1258,16 +1257,5 @@ mod tests {
fn load(&self, _: &AppContext) -> Task<Result<String>> {
unimplemented!()
}

fn buffer_reloaded(
&self,
_: BufferId,
_: &clock::Global,
_: language::LineEnding,
_: Option<std::time::SystemTime>,
_: &mut AppContext,
) {
unimplemented!()
}
}
}
19 changes: 0 additions & 19 deletions crates/language/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,16 +382,6 @@ pub trait LocalFile: File {
/// Loads the file's contents from disk.
fn load(&self, cx: &AppContext) -> Task<Result<String>>;

/// Called when the buffer is reloaded from disk.
fn buffer_reloaded(
&self,
buffer_id: BufferId,
version: &clock::Global,
line_ending: LineEnding,
mtime: Option<SystemTime>,
cx: &mut AppContext,
);

/// Returns true if the file should not be shared with collaborators.
fn is_private(&self, _: &AppContext) -> bool {
false
Expand Down Expand Up @@ -884,15 +874,6 @@ impl Buffer {
self.saved_version = version;
self.text.set_line_ending(line_ending);
self.saved_mtime = mtime;
if let Some(file) = self.file.as_ref().and_then(|f| f.as_local()) {
file.buffer_reloaded(
self.remote_id(),
&self.saved_version,
self.line_ending(),
self.saved_mtime,
cx,
);
}
cx.emit(Event::Reloaded);
cx.notify();
}
Expand Down
Loading

0 comments on commit 4858116

Please sign in to comment.