Skip to content

Commit

Permalink
Make helix_core::Uri cheap to clone
Browse files Browse the repository at this point in the history
We clone this type very often in LSP pickers, for example diagnostics
and symbols. We can use a single Arc in many cases to avoid the
unnecessary clones.
  • Loading branch information
the-mikedavis committed Aug 14, 2024
1 parent be1cb8d commit 50bd90d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 26 deletions.
23 changes: 4 additions & 19 deletions helix-core/src/uri.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
fmt,
path::{Path, PathBuf},
sync::Arc,
};

/// A generic pointer to a file location.
Expand All @@ -9,7 +10,7 @@ use std::{
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[non_exhaustive]
pub enum Uri {
File(PathBuf),
File(Arc<Path>),
}

impl Uri {
Expand All @@ -26,27 +27,11 @@ impl Uri {
Self::File(path) => Some(path),
}
}

pub fn as_path_buf(self) -> Option<PathBuf> {
match self {
Self::File(path) => Some(path),
}
}
}

impl From<PathBuf> for Uri {
fn from(path: PathBuf) -> Self {
Self::File(path)
}
}

impl TryFrom<Uri> for PathBuf {
type Error = ();

fn try_from(uri: Uri) -> Result<Self, Self::Error> {
match uri {
Uri::File(path) => Ok(path),
}
Self::File(path.into())
}
}

Expand Down Expand Up @@ -88,7 +73,7 @@ impl std::error::Error for UrlConversionError {}
fn convert_url_to_uri(url: &url::Url) -> Result<Uri, UrlConversionErrorKind> {
if url.scheme() == "file" {
url.to_file_path()
.map(|path| Uri::File(helix_stdx::path::normalize(path)))
.map(|path| Uri::File(helix_stdx::path::normalize(path).into()))
.map_err(|_| UrlConversionErrorKind::UnableToConvert)
} else {
Err(UrlConversionErrorKind::UnsupportedScheme)
Expand Down
18 changes: 11 additions & 7 deletions helix-view/src/handlers/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl Editor {
match op {
ResourceOp::Create(op) => {
let uri = Uri::try_from(&op.uri)?;
let path = uri.as_path_buf().expect("URIs are valid paths");
let path = uri.as_path().expect("URIs are valid paths");
let ignore_if_exists = op.options.as_ref().map_or(false, |options| {
!options.overwrite.unwrap_or(false) && options.ignore_if_exists.unwrap_or(false)
});
Expand All @@ -255,13 +255,15 @@ impl Editor {
}
}

fs::write(&path, [])?;
self.language_servers.file_event_handler.file_changed(path);
fs::write(path, [])?;
self.language_servers
.file_event_handler
.file_changed(path.to_path_buf());
}
}
ResourceOp::Delete(op) => {
let uri = Uri::try_from(&op.uri)?;
let path = uri.as_path_buf().expect("URIs are valid paths");
let path = uri.as_path().expect("URIs are valid paths");
if path.is_dir() {
let recursive = op
.options
Expand All @@ -270,11 +272,13 @@ impl Editor {
.unwrap_or(false);

if recursive {
fs::remove_dir_all(&path)?
fs::remove_dir_all(path)?
} else {
fs::remove_dir(&path)?
fs::remove_dir(path)?
}
self.language_servers.file_event_handler.file_changed(path);
self.language_servers
.file_event_handler
.file_changed(path.to_path_buf());
} else if path.is_file() {
fs::remove_file(path)?;
}
Expand Down

0 comments on commit 50bd90d

Please sign in to comment.