Avoid using worktree handle in File's path methods

This avoids a circular model update that was happening
when trying to retrieve the absolute path from a buffer's
file while applying remote operations.
This commit is contained in:
Max Brunsfeld 2021-11-02 14:33:55 -07:00
parent 1995bd89a6
commit 89392cd23d
3 changed files with 68 additions and 54 deletions

View file

@ -127,15 +127,15 @@ pub trait File {
fn path(&self) -> &Arc<Path>; fn path(&self) -> &Arc<Path>;
/// Returns the absolute path of this file. /// Returns the absolute path of this file.
fn abs_path(&self, cx: &AppContext) -> Option<PathBuf>; fn abs_path(&self) -> Option<PathBuf>;
/// Returns the path of this file relative to the worktree's parent directory (this means it /// Returns the path of this file relative to the worktree's parent directory (this means it
/// includes the name of the worktree's root folder). /// includes the name of the worktree's root folder).
fn full_path(&self, cx: &AppContext) -> PathBuf; fn full_path(&self) -> PathBuf;
/// Returns the last component of this handle's absolute path. If this handle refers to the root /// Returns the last component of this handle's absolute path. If this handle refers to the root
/// of its worktree, then this method will return the name of the worktree itself. /// of its worktree, then this method will return the name of the worktree itself.
fn file_name<'a>(&'a self, cx: &'a AppContext) -> Option<OsString>; fn file_name(&self) -> Option<OsString>;
fn is_deleted(&self) -> bool; fn is_deleted(&self) -> bool;
@ -455,7 +455,7 @@ impl Buffer {
}; };
self.reparse(cx); self.reparse(cx);
self.update_language_server(cx); self.update_language_server();
} }
pub fn did_save( pub fn did_save(
@ -479,7 +479,7 @@ impl Buffer {
lsp::DidSaveTextDocumentParams { lsp::DidSaveTextDocumentParams {
text_document: lsp::TextDocumentIdentifier { text_document: lsp::TextDocumentIdentifier {
uri: lsp::Url::from_file_path( uri: lsp::Url::from_file_path(
self.file.as_ref().unwrap().abs_path(cx).unwrap(), self.file.as_ref().unwrap().abs_path().unwrap(),
) )
.unwrap(), .unwrap(),
}, },
@ -1121,7 +1121,7 @@ impl Buffer {
Ok(()) Ok(())
} }
fn update_language_server(&mut self, cx: &AppContext) { fn update_language_server(&mut self) {
let language_server = if let Some(language_server) = self.language_server.as_mut() { let language_server = if let Some(language_server) = self.language_server.as_mut() {
language_server language_server
} else { } else {
@ -1131,7 +1131,7 @@ impl Buffer {
.file .file
.as_ref() .as_ref()
.map_or(Path::new("/").to_path_buf(), |file| { .map_or(Path::new("/").to_path_buf(), |file| {
file.abs_path(cx).unwrap() file.abs_path().unwrap()
}); });
let version = post_inc(&mut language_server.next_version); let version = post_inc(&mut language_server.next_version);
@ -1266,7 +1266,7 @@ impl Buffer {
} }
self.reparse(cx); self.reparse(cx);
self.update_language_server(cx); self.update_language_server();
cx.emit(Event::Edited); cx.emit(Event::Edited);
if !was_dirty { if !was_dirty {

View file

@ -616,6 +616,8 @@ impl Worktree {
} }
}; };
let local = self.as_local().is_some();
let worktree_path = self.abs_path.clone();
let worktree_handle = cx.handle(); let worktree_handle = cx.handle();
let mut buffers_to_delete = Vec::new(); let mut buffers_to_delete = Vec::new();
for (buffer_id, buffer) in open_buffers { for (buffer_id, buffer) in open_buffers {
@ -627,6 +629,8 @@ impl Worktree {
.and_then(|entry_id| self.entry_for_id(entry_id)) .and_then(|entry_id| self.entry_for_id(entry_id))
{ {
File { File {
is_local: local,
worktree_path: worktree_path.clone(),
entry_id: Some(entry.id), entry_id: Some(entry.id),
mtime: entry.mtime, mtime: entry.mtime,
path: entry.path.clone(), path: entry.path.clone(),
@ -634,6 +638,8 @@ impl Worktree {
} }
} else if let Some(entry) = self.entry_for_path(old_file.path().as_ref()) { } else if let Some(entry) = self.entry_for_path(old_file.path().as_ref()) {
File { File {
is_local: local,
worktree_path: worktree_path.clone(),
entry_id: Some(entry.id), entry_id: Some(entry.id),
mtime: entry.mtime, mtime: entry.mtime,
path: entry.path.clone(), path: entry.path.clone(),
@ -641,6 +647,8 @@ impl Worktree {
} }
} else { } else {
File { File {
is_local: local,
worktree_path: worktree_path.clone(),
entry_id: None, entry_id: None,
path: old_file.path().clone(), path: old_file.path().clone(),
mtime: old_file.mtime(), mtime: old_file.mtime(),
@ -976,12 +984,9 @@ impl LocalWorktree {
let (file, contents) = this let (file, contents) = this
.update(&mut cx, |this, cx| this.as_local().unwrap().load(&path, cx)) .update(&mut cx, |this, cx| this.as_local().unwrap().load(&path, cx))
.await?; .await?;
let language = this.read_with(&cx, |this, cx| { let language = this.read_with(&cx, |this, _| {
use language::File; use language::File;
this.languages().select_language(file.full_path()).cloned()
this.languages()
.select_language(file.full_path(cx))
.cloned()
}); });
let diagnostics = this.update(&mut cx, |this, _| { let diagnostics = this.update(&mut cx, |this, _| {
this.as_local_mut() this.as_local_mut()
@ -1144,6 +1149,7 @@ impl LocalWorktree {
fn load(&self, path: &Path, cx: &mut ModelContext<Worktree>) -> Task<Result<(File, String)>> { fn load(&self, path: &Path, cx: &mut ModelContext<Worktree>) -> Task<Result<(File, String)>> {
let handle = cx.handle(); let handle = cx.handle();
let path = Arc::from(path); let path = Arc::from(path);
let worktree_path = self.abs_path.clone();
let abs_path = self.absolutize(&path); let abs_path = self.absolutize(&path);
let background_snapshot = self.background_snapshot.clone(); let background_snapshot = self.background_snapshot.clone();
let fs = self.fs.clone(); let fs = self.fs.clone();
@ -1152,7 +1158,17 @@ impl LocalWorktree {
// Eagerly populate the snapshot with an updated entry for the loaded file // Eagerly populate the snapshot with an updated entry for the loaded file
let entry = refresh_entry(fs.as_ref(), &background_snapshot, path, &abs_path).await?; let entry = refresh_entry(fs.as_ref(), &background_snapshot, path, &abs_path).await?;
this.update(&mut cx, |this, cx| this.poll_snapshot(cx)); this.update(&mut cx, |this, cx| this.poll_snapshot(cx));
Ok((File::new(entry.id, handle, entry.path, entry.mtime), text)) Ok((
File {
entry_id: Some(entry.id),
worktree: handle,
worktree_path,
path: entry.path,
mtime: entry.mtime,
is_local: true,
},
text,
))
}) })
} }
@ -1167,11 +1183,16 @@ impl LocalWorktree {
cx.spawn(|this, mut cx| async move { cx.spawn(|this, mut cx| async move {
let entry = save.await?; let entry = save.await?;
this.update(&mut cx, |this, cx| { this.update(&mut cx, |this, cx| {
this.as_local_mut() let this = this.as_local_mut().unwrap();
.unwrap() this.open_buffers.insert(buffer.id(), buffer.downgrade());
.open_buffers Ok(File {
.insert(buffer.id(), buffer.downgrade()); entry_id: Some(entry.id),
Ok(File::new(entry.id, cx.handle(), entry.path, entry.mtime)) worktree: cx.handle(),
worktree_path: this.abs_path.clone(),
path: entry.path,
mtime: entry.mtime,
is_local: true,
})
}) })
}) })
} }
@ -1360,6 +1381,7 @@ impl RemoteWorktree {
let rpc = self.client.clone(); let rpc = self.client.clone();
let replica_id = self.replica_id; let replica_id = self.replica_id;
let remote_worktree_id = self.remote_id; let remote_worktree_id = self.remote_id;
let root_path = self.snapshot.abs_path.clone();
let path = path.to_string_lossy().to_string(); let path = path.to_string_lossy().to_string();
cx.spawn_weak(|this, mut cx| async move { cx.spawn_weak(|this, mut cx| async move {
if let Some(existing_buffer) = existing_buffer { if let Some(existing_buffer) = existing_buffer {
@ -1380,13 +1402,17 @@ impl RemoteWorktree {
let this = this let this = this
.upgrade(&cx) .upgrade(&cx)
.ok_or_else(|| anyhow!("worktree was closed"))?; .ok_or_else(|| anyhow!("worktree was closed"))?;
let file = File::new(entry.id, this.clone(), entry.path, entry.mtime); let file = File {
let language = this.read_with(&cx, |this, cx| { entry_id: Some(entry.id),
worktree: this.clone(),
worktree_path: root_path,
path: entry.path,
mtime: entry.mtime,
is_local: false,
};
let language = this.read_with(&cx, |this, _| {
use language::File; use language::File;
this.languages().select_language(file.full_path()).cloned()
this.languages()
.select_language(file.full_path(cx))
.cloned()
}); });
let remote_buffer = response.buffer.ok_or_else(|| anyhow!("empty buffer"))?; let remote_buffer = response.buffer.ok_or_else(|| anyhow!("empty buffer"))?;
let buffer_id = remote_buffer.id as usize; let buffer_id = remote_buffer.id as usize;
@ -1868,24 +1894,10 @@ impl fmt::Debug for Snapshot {
pub struct File { pub struct File {
entry_id: Option<usize>, entry_id: Option<usize>,
worktree: ModelHandle<Worktree>, worktree: ModelHandle<Worktree>,
worktree_path: Arc<Path>,
pub path: Arc<Path>, pub path: Arc<Path>,
pub mtime: SystemTime, pub mtime: SystemTime,
} is_local: bool,
impl File {
pub fn new(
entry_id: usize,
worktree: ModelHandle<Worktree>,
path: Arc<Path>,
mtime: SystemTime,
) -> Self {
Self {
entry_id: Some(entry_id),
worktree,
path,
mtime,
}
}
} }
impl language::File for File { impl language::File for File {
@ -1905,27 +1917,29 @@ impl language::File for File {
&self.path &self.path
} }
fn abs_path(&self, cx: &AppContext) -> Option<PathBuf> { fn abs_path(&self) -> Option<PathBuf> {
let worktree = self.worktree.read(cx); if self.is_local {
worktree Some(self.worktree_path.join(&self.path))
.as_local() } else {
.map(|worktree| worktree.absolutize(&self.path)) None
}
} }
fn full_path(&self, cx: &AppContext) -> PathBuf { fn full_path(&self) -> PathBuf {
let worktree = self.worktree.read(cx);
let mut full_path = PathBuf::new(); let mut full_path = PathBuf::new();
full_path.push(worktree.root_name()); if let Some(worktree_name) = self.worktree_path.file_name() {
full_path.push(worktree_name);
}
full_path.push(&self.path); full_path.push(&self.path);
full_path full_path
} }
/// Returns the last component of this handle's absolute path. If this handle refers to the root /// Returns the last component of this handle's absolute path. If this handle refers to the root
/// of its worktree, then this method will return the name of the worktree itself. /// of its worktree, then this method will return the name of the worktree itself.
fn file_name<'a>(&'a self, cx: &'a AppContext) -> Option<OsString> { fn file_name<'a>(&'a self) -> Option<OsString> {
self.path self.path
.file_name() .file_name()
.or_else(|| Some(OsStr::new(self.worktree.read(cx).root_name()))) .or_else(|| self.worktree_path.file_name())
.map(Into::into) .map(Into::into)
} }

View file

@ -77,7 +77,7 @@ impl ItemView for Editor {
.buffer() .buffer()
.read(cx) .read(cx)
.file() .file()
.and_then(|file| file.file_name(cx)); .and_then(|file| file.file_name());
if let Some(name) = filename { if let Some(name) = filename {
name.to_string_lossy().into() name.to_string_lossy().into()
} else { } else {
@ -127,8 +127,8 @@ impl ItemView for Editor {
cx.spawn(|buffer, mut cx| async move { cx.spawn(|buffer, mut cx| async move {
save_as.await.map(|new_file| { save_as.await.map(|new_file| {
let (language, language_server) = worktree.read_with(&cx, |worktree, cx| { let (language, language_server) = worktree.read_with(&cx, |worktree, _| {
let language = worktree.languages().select_language(new_file.full_path(cx)); let language = worktree.languages().select_language(new_file.full_path());
let language_server = worktree.language_server(); let language_server = worktree.language_server();
(language.cloned(), language_server.cloned()) (language.cloned(), language_server.cloned())
}); });