Handle conversion to/from new LSP URL type

Cargo.lock

@@ -1218,6 +1218,7 @@ dependencies = [
  "nucleo",
  "once_cell",
  "parking_lot",
+ "percent-encoding",
  "quickcheck",
  "regex",
  "ropey",
@@ -1232,7 +1233,6 @@ dependencies = [
  "unicode-general-category",
  "unicode-segmentation",
  "unicode-width",
- "url",
 ]
 
 [[package]]
@@ -1312,10 +1312,10 @@ name = "helix-lsp-types"
 version = "0.95.1"
 dependencies = [
  "bitflags",
+ "percent-encoding",
  "serde",
  "serde_json",
  "serde_repr",
- "url",
 ]
 
 [[package]]
@@ -1446,7 +1446,6 @@ dependencies = [
  "tokio",
  "tokio-stream",
  "toml",
- "url",
 ]
 
 [[package]]
@@ -2420,7 +2419,6 @@ dependencies = [
  "form_urlencoded",
  "idna",
  "percent-encoding",
- "serde",
 ]
 
 [[package]]

Cargo.toml

@@ -42,6 +42,7 @@ tree-sitter = { version = "0.22" }
 nucleo = "0.5.0"
 slotmap = "1.0.7"
 thiserror = "1.0"
+percent-encoding = "2.3"
 
 [workspace.package]
 version = "24.7.0"

helix-core/Cargo.toml

@@ -39,7 +39,7 @@ bitflags = "2.6"
 ahash = "0.8.11"
 hashbrown = { version = "0.14.5", features = ["raw"] }
 dunce = "1.0"
-url = "2.5.0"
+percent-encoding.workspace = true
 
 log = "0.4"
 serde = { version = "1.0", features = ["derive"] }

helix-core/src/uri.rs

@@ -1,6 +1,7 @@
 use std::{
     fmt,
     path::{Path, PathBuf},
+    str::FromStr,
     sync::Arc,
 };
 
@@ -16,14 +17,6 @@ pub enum Uri {
 }
 
 impl Uri {
-    // This clippy allow mirrors url::Url::from_file_path
-    #[allow(clippy::result_unit_err)]
-    pub fn to_url(&self) -> Result<url::Url, ()> {
-        match self {
-            Uri::File(path) => url::Url::from_file_path(path),
-        }
-    }
-
     pub fn as_path(&self) -> Option<&Path> {
         match self {
             Self::File(path) => Some(path),
@@ -45,81 +38,92 @@ impl fmt::Display for Uri {
     }
 }
 
-#[derive(Debug)]
-pub struct UrlConversionError {
-    source: url::Url,
-    kind: UrlConversionErrorKind,
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct UriParseError {
+    source: String,
+    kind: UriParseErrorKind,
 }
 
-#[derive(Debug)]
-pub enum UrlConversionErrorKind {
-    UnsupportedScheme,
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum UriParseErrorKind {
+    UnsupportedScheme(String),
     UnableToConvert,
 }
 
-impl fmt::Display for UrlConversionError {
+impl fmt::Display for UriParseError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self.kind {
-            UrlConversionErrorKind::UnsupportedScheme => {
-                write!(
-                    f,
-                    "unsupported scheme '{}' in URL {}",
-                    self.source.scheme(),
-                    self.source
-                )
+        match &self.kind {
+            UriParseErrorKind::UnsupportedScheme(scheme) => {
+                write!(f, "unsupported scheme '{scheme}' in URL {}", self.source)
             }
-            UrlConversionErrorKind::UnableToConvert => {
+            UriParseErrorKind::UnableToConvert => {
                 write!(f, "unable to convert URL to file path: {}", self.source)
             }
         }
     }
 }
 
-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).into()))
-            .map_err(|_| UrlConversionErrorKind::UnableToConvert)
-    } else {
-        Err(UrlConversionErrorKind::UnsupportedScheme)
-    }
-}
+impl std::error::Error for UriParseError {}
+
+impl FromStr for Uri {
+    type Err = UriParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        use std::ffi::OsStr;
+        #[cfg(any(unix, target_os = "redox"))]
+        use std::os::unix::prelude::OsStrExt;
+        #[cfg(target_os = "wasi")]
+        use std::os::wasi::prelude::OsStrExt;
+
+        let Some((scheme, rest)) = s.split_once("://") else {
+            return Err(Self::Err {
+                source: s.to_string(),
+                kind: UriParseErrorKind::UnableToConvert,
+            });
+        };
+
+        if scheme != "file" {
+            return Err(Self::Err {
+                source: s.to_string(),
+                kind: UriParseErrorKind::UnsupportedScheme(scheme.to_string()),
+            });
+        }
 
-impl TryFrom<url::Url> for Uri {
-    type Error = UrlConversionError;
+        // Assert there is no query or fragment in the URI.
+        if s.find(['?', '#']).is_some() {
+            return Err(Self::Err {
+                source: s.to_string(),
+                kind: UriParseErrorKind::UnableToConvert,
+            });
+        }
 
-    fn try_from(url: url::Url) -> Result<Self, Self::Error> {
-        convert_url_to_uri(&url).map_err(|kind| Self::Error { source: url, kind })
+        let mut bytes = Vec::new();
+        bytes.extend(percent_encoding::percent_decode(rest.as_bytes()));
+        Ok(PathBuf::from(OsStr::from_bytes(&bytes)).into())
     }
 }
 
-impl TryFrom<&url::Url> for Uri {
-    type Error = UrlConversionError;
+impl TryFrom<&str> for Uri {
+    type Error = UriParseError;
 
-    fn try_from(url: &url::Url) -> Result<Self, Self::Error> {
-        convert_url_to_uri(url).map_err(|kind| Self::Error {
-            source: url.clone(),
-            kind,
-        })
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
+        s.parse()
     }
 }
 
 #[cfg(test)]
 mod test {
     use super::*;
-    use url::Url;
 
     #[test]
     fn unknown_scheme() {
-        let url = Url::parse("csharp:/metadata/foo/bar/Baz.cs").unwrap();
-        assert!(matches!(
-            Uri::try_from(url),
-            Err(UrlConversionError {
-                kind: UrlConversionErrorKind::UnsupportedScheme,
-                ..
+        let uri = "csharp://metadata/foo/barBaz.cs";
+        assert_eq!(
+            uri.parse::<Uri>(),
+            Err(UriParseError {
+                source: uri.to_string(),
+                kind: UriParseErrorKind::UnsupportedScheme("csharp".to_string()),
             })
-        ));
+        );
     }
 }

helix-lsp/src/client.rs

@@ -42,7 +42,8 @@ fn workspace_for_path(path: &Path) -> WorkspaceFolder {
 
     lsp::WorkspaceFolder {
         name,
-        uri: lsp::Url::from_file_path(path).expect("absolute paths can be converted to `Url`s"),
+        uri: lsp::Url::from_directory_path(path)
+            .expect("absolute paths can be converted to `Url`s"),
     }
 }
 
@@ -742,7 +743,7 @@ impl Client {
             } else {
                 Url::from_file_path(path)
             };
-            Some(url.ok()?.to_string())
+            Some(url.ok()?.into_string())
         };
         let files = vec![lsp::FileRename {
             old_uri: url_from_path(old_path)?,
@@ -776,7 +777,7 @@ impl Client {
             } else {
                 Url::from_file_path(path)
             };
-            Some(url.ok()?.to_string())
+            Some(url.ok()?.into_string())
         };
 
         let files = vec![lsp::FileRename {

helix-term/src/application.rs

@@ -738,7 +738,7 @@ impl Application {
                         }
                     }
                     Notification::PublishDiagnostics(mut params) => {
-                        let uri = match helix_core::Uri::try_from(params.uri) {
+                        let uri = match helix_core::Uri::try_from(params.uri.as_str()) {
                             Ok(uri) => uri,
                             Err(err) => {
                                 log::error!("{err}");
@@ -1137,7 +1137,8 @@ impl Application {
             ..
         } = params
         {
-            self.jobs.callback(crate::open_external_url_callback(uri));
+            self.jobs
+                .callback(crate::open_external_url_callback(uri.as_str()));
             return lsp::ShowDocumentResult { success: true };
         };
 
@@ -1148,7 +1149,7 @@ impl Application {
             ..
         } = params;
 
-        let uri = match helix_core::Uri::try_from(uri) {
+        let uri = match helix_core::Uri::try_from(uri.as_str()) {
             Ok(uri) => uri,
             Err(err) => {
                 log::error!("{err}");

helix-term/src/commands.rs

@@ -1350,7 +1350,9 @@ fn open_url(cx: &mut Context, url: Url, action: Action) {
         .unwrap_or_default();
 
     if url.scheme() != "file" {
-        return cx.jobs.callback(crate::open_external_url_callback(url));
+        return cx
+            .jobs
+            .callback(crate::open_external_url_callback(url.as_str()));
     }
 
     let content_type = std::fs::File::open(url.path()).and_then(|file| {
@@ -1363,9 +1365,9 @@ fn open_url(cx: &mut Context, url: Url, action: Action) {
     // we attempt to open binary files - files that can't be open in helix - using external
     // program as well, e.g. pdf files or images
     match content_type {
-        Ok(content_inspector::ContentType::BINARY) => {
-            cx.jobs.callback(crate::open_external_url_callback(url))
-        }
+        Ok(content_inspector::ContentType::BINARY) => cx
+            .jobs
+            .callback(crate::open_external_url_callback(url.as_str())),
         Ok(_) | Err(_) => {
             let path = &rel_path.join(url.path());
             if path.is_dir() {

helix-term/src/commands/lsp.rs

@@ -69,7 +69,7 @@ struct Location {
 }
 
 fn lsp_location_to_location(location: lsp::Location) -> Option<Location> {
-    let uri = match location.uri.try_into() {
+    let uri = match location.uri.as_str().try_into() {
         Ok(uri) => uri,
         Err(err) => {
             log::warn!("discarding invalid or unsupported URI: {err}");
@@ -456,7 +456,7 @@ pub fn workspace_symbol_picker(cx: &mut Context) {
                             .unwrap_or_default()
                             .into_iter()
                             .filter_map(|symbol| {
-                                let uri = match Uri::try_from(&symbol.location.uri) {
+                                let uri = match Uri::try_from(symbol.location.uri.as_str()) {
                                     Ok(uri) => uri,
                                     Err(err) => {
                                         log::warn!("discarding symbol with invalid URI: {err}");
@@ -510,7 +510,7 @@ pub fn workspace_symbol_picker(cx: &mut Context) {
                     .to_string()
                     .into()
             } else {
-                item.symbol.location.uri.to_string().into()
+                item.symbol.location.uri.as_str().into()
             }
         }),
     ];

helix-term/src/lib.rs

@@ -18,7 +18,6 @@ use futures_util::Future;
 mod handlers;
 
 use ignore::DirEntry;
-use url::Url;
 
 #[cfg(windows)]
 fn true_color() -> bool {
@@ -70,10 +69,10 @@ fn filter_picker_entry(entry: &DirEntry, root: &Path, dedup_symlinks: bool) -> b
 }
 
 /// Opens URL in external program.
-fn open_external_url_callback(
-    url: Url,
+fn open_external_url_callback<U: AsRef<std::ffi::OsStr>>(
+    url: U,
 ) -> impl Future<Output = Result<job::Callback, anyhow::Error>> + Send + 'static {
-    let commands = open::commands(url.as_str());
+    let commands = open::commands(url);
     async {
         for cmd in commands {
             let mut command = tokio::process::Command::new(cmd.get_program());

helix-view/Cargo.toml

@@ -30,9 +30,7 @@ crossterm = { version = "0.28", optional = true }
 
 tempfile = "3.13"
 
-# Conversion traits
 once_cell = "1.20"
-url = "2.5.2"
 
 arc-swap = { version = "1.7.1" }
 

helix-view/src/document.rs

@@ -640,7 +640,6 @@ where
 }
 
 use helix_lsp::{lsp, Client, LanguageServerId, LanguageServerName};
-use url::Url;
 
 impl Document {
     pub fn from(
@@ -1811,8 +1810,8 @@ impl Document {
     }
 
     /// File path as a URL.
-    pub fn url(&self) -> Option<Url> {
-        Url::from_file_path(self.path()?).ok()
+    pub fn url(&self) -> Option<lsp::Url> {
+        lsp::Url::from_file_path(self.path()?).ok()
     }
 
     pub fn uri(&self) -> Option<helix_core::Uri> {
@@ -1898,7 +1897,7 @@ impl Document {
     pub fn lsp_diagnostic_to_diagnostic(
         text: &Rope,
         language_config: Option<&LanguageConfiguration>,
-        diagnostic: &helix_lsp::lsp::Diagnostic,
+        diagnostic: &lsp::Diagnostic,
         language_server_id: LanguageServerId,
         offset_encoding: helix_lsp::OffsetEncoding,
     ) -> Option<Diagnostic> {

helix-view/src/handlers/lsp.rs

@@ -57,7 +57,7 @@ pub struct ApplyEditError {
 pub enum ApplyEditErrorKind {
     DocumentChanged,
     FileNotFound,
-    InvalidUrl(helix_core::uri::UrlConversionError),
+    InvalidUrl(helix_core::uri::UriParseError),
     IoError(std::io::Error),
     // TODO: check edits before applying and propagate failure
     // InvalidEdit,
@@ -69,8 +69,8 @@ impl From<std::io::Error> for ApplyEditErrorKind {
     }
 }
 
-impl From<helix_core::uri::UrlConversionError> for ApplyEditErrorKind {
-    fn from(err: helix_core::uri::UrlConversionError) -> Self {
+impl From<helix_core::uri::UriParseError> for ApplyEditErrorKind {
+    fn from(err: helix_core::uri::UriParseError) -> Self {
         ApplyEditErrorKind::InvalidUrl(err)
     }
 }
@@ -94,7 +94,7 @@ impl Editor {
         text_edits: Vec<lsp::TextEdit>,
         offset_encoding: OffsetEncoding,
     ) -> Result<(), ApplyEditErrorKind> {
-        let uri = match Uri::try_from(url) {
+        let uri = match Uri::try_from(url.as_str()) {
             Ok(uri) => uri,
             Err(err) => {
                 log::error!("{err}");
@@ -242,7 +242,7 @@ impl Editor {
         // may no longer be valid.
         match op {
             ResourceOp::Create(op) => {
-                let uri = Uri::try_from(&op.uri)?;
+                let uri = Uri::try_from(op.uri.as_str())?;
                 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)
@@ -262,7 +262,7 @@ impl Editor {
                 }
             }
             ResourceOp::Delete(op) => {
-                let uri = Uri::try_from(&op.uri)?;
+                let uri = Uri::try_from(op.uri.as_str())?;
                 let path = uri.as_path().expect("URIs are valid paths");
                 if path.is_dir() {
                     let recursive = op
@@ -284,9 +284,9 @@ impl Editor {
                 }
             }
             ResourceOp::Rename(op) => {
-                let from_uri = Uri::try_from(&op.old_uri)?;
+                let from_uri = Uri::try_from(op.old_uri.as_str())?;
                 let from = from_uri.as_path().expect("URIs are valid paths");
-                let to_uri = Uri::try_from(&op.new_uri)?;
+                let to_uri = Uri::try_from(op.new_uri.as_str())?;
                 let to = to_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)