From 636902ea3ea0f2fc5b771a761ed77de30df66091 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Thu, 15 Aug 2024 17:49:00 -0400 Subject: [PATCH] Handle conversion to/from new LSP URL type --- Cargo.lock | 6 +- Cargo.toml | 1 + helix-core/Cargo.toml | 2 +- helix-core/src/uri.rs | 114 +++++++++++++++++---------------- helix-lsp/src/client.rs | 7 +- helix-term/src/application.rs | 7 +- helix-term/src/commands.rs | 10 +-- helix-term/src/commands/lsp.rs | 6 +- helix-term/src/lib.rs | 7 +- helix-view/Cargo.toml | 2 - helix-view/src/document.rs | 7 +- helix-view/src/handlers/lsp.rs | 16 ++--- 12 files changed, 94 insertions(+), 91 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e622fbe59..db27baa67 100644 --- a/Cargo.lock +++ b/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]] diff --git a/Cargo.toml b/Cargo.toml index 763992480..6f686984e 100644 --- a/Cargo.toml +++ b/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" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 4cd516268..1c0fe3368 100644 --- a/helix-core/Cargo.toml +++ b/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"] } diff --git a/helix-core/src/uri.rs b/helix-core/src/uri.rs index cbe0fadda..cb852e808 100644 --- a/helix-core/src/uri.rs +++ b/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 { - 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 { - 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 { + 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 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 { - 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 { - convert_url_to_uri(url).map_err(|kind| Self::Error { - source: url.clone(), - kind, - }) + fn try_from(s: &str) -> Result { + 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::(), + Err(UriParseError { + source: uri.to_string(), + kind: UriParseErrorKind::UnsupportedScheme("csharp".to_string()), }) - )); + ); } } diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 5df547100..e20723bdf 100644 --- a/helix-lsp/src/client.rs +++ b/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 { diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a567815fc..471da2af8 100644 --- a/helix-term/src/application.rs +++ b/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}"); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b1c29378d..4a0c89028 100644 --- a/helix-term/src/commands.rs +++ b/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() { diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index fcc0333e8..4fd957c87 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -69,7 +69,7 @@ struct Location { } fn lsp_location_to_location(location: lsp::Location) -> Option { - 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() } }), ]; diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index cf4fbd9fa..71fb3b4f5 100644 --- a/helix-term/src/lib.rs +++ b/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>( + url: U, ) -> impl Future> + 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()); diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index 725a77547..f1fe5e378 100644 --- a/helix-view/Cargo.toml +++ b/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" } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 91ec27874..9ef2850f4 100644 --- a/helix-view/src/document.rs +++ b/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::from_file_path(self.path()?).ok() + pub fn url(&self) -> Option { + lsp::Url::from_file_path(self.path()?).ok() } pub fn uri(&self) -> Option { @@ -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 { diff --git a/helix-view/src/handlers/lsp.rs b/helix-view/src/handlers/lsp.rs index 1fd2289db..77f876965 100644 --- a/helix-view/src/handlers/lsp.rs +++ b/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 for ApplyEditErrorKind { } } -impl From for ApplyEditErrorKind { - fn from(err: helix_core::uri::UrlConversionError) -> Self { +impl From for ApplyEditErrorKind { + fn from(err: helix_core::uri::UriParseError) -> Self { ApplyEditErrorKind::InvalidUrl(err) } } @@ -94,7 +94,7 @@ impl Editor { text_edits: Vec, 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)