From da2b0a74844f77ba233638c2b5a0ee2367a66871 Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Thu, 8 Aug 2024 11:17:20 -0400
Subject: [PATCH] Make helix_core::Uri cheap to clone

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.
---
 helix-core/src/uri.rs          | 25 ++++++-------------------
 helix-view/src/handlers/lsp.rs | 18 +++++++++++-------
 2 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/helix-core/src/uri.rs b/helix-core/src/uri.rs
index ddb9fb7a8..cbe0fadda 100644
--- a/helix-core/src/uri.rs
+++ b/helix-core/src/uri.rs
@@ -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 {
@@ -26,27 +29,11 @@ pub fn as_path(&self) -> Option<&Path> {
             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())
     }
 }
 
@@ -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)
diff --git a/helix-view/src/handlers/lsp.rs b/helix-view/src/handlers/lsp.rs
index 6aff2e50c..1fd2289db 100644
--- a/helix-view/src/handlers/lsp.rs
+++ b/helix-view/src/handlers/lsp.rs
@@ -243,7 +243,7 @@ fn apply_document_resource_op(
         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)
                 });
@@ -255,13 +255,15 @@ fn apply_document_resource_op(
                         }
                     }
 
-                    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
@@ -270,11 +272,13 @@ fn apply_document_resource_op(
                         .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)?;
                 }