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 authored and rmburg committed Jan 20, 2025
1 parent b6ccc6c commit f5fcc57
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 26 deletions.
25 changes: 6 additions & 19 deletions helix-core/src/uri.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
use std::{
fmt,
path::{Path, PathBuf},
sync::Arc,
};

/// A generic pointer to a file location.
///
/// Currently this type only supports paths to local files.
///
/// Cloning this type is cheap: the internal representation uses an Arc.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[non_exhaustive]
pub enum Uri {
File(PathBuf),
File(Arc<Path>),
}

impl Uri {
Expand All @@ -26,27 +29,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 @@ -93,7 +80,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 f5fcc57

Please sign in to comment.