diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 1894dc5b1..a546ce5e1 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -72,8 +72,12 @@ impl Client { doc_path: Option<&std::path::PathBuf>, may_support_workspace: bool, ) -> bool { - let (workspace, workspace_is_cwd) = find_workspace(); - let workspace = path::normalize(workspace); + let (workspace_path, workspace_is_cwd) = find_workspace(); + // Attempt to canonicalize the workspace path. Fallback to original if error. + let workspace = match helix_stdx::path::canonicalize(&workspace_path) { + Ok(p) => p, + Err(_) => workspace_path, // Or handle error more explicitly, e.g. return false + }; let root = find_lsp_workspace( doc_path .and_then(|x| x.parent().and_then(|x| x.to_str())) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 567e8a702..bcf7c30f3 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -872,7 +872,7 @@ fn start_client( enable_snippets: bool, ) -> Result { let (workspace, workspace_is_cwd) = helix_loader::find_workspace(); - let workspace = path::normalize(workspace); + let workspace = helix_stdx::path::canonicalize(&workspace).map_err(|err| StartupError::Error(anyhow::anyhow!("failed to canonicalize workspace {:?}: {}", workspace, err)))?; let root = find_lsp_workspace( doc_path .and_then(|x| x.parent().and_then(|x| x.to_str())) @@ -885,8 +885,8 @@ fn start_client( // `root_uri` and `workspace_folder` can be empty in case there is no workspace // `root_url` can not, use `workspace` as a fallback - let root_path = root.clone().unwrap_or_else(|| workspace.clone()); - let root_uri = root.and_then(|root| lsp::Url::from_file_path(root).ok()); + let root_path = root.clone().unwrap_or_else(|| workspace.clone()); // workspace is now canonical + let root_uri = root.as_ref().and_then(|r| lsp::Url::from_file_path(r).ok()); // root is now canonical if let Some(globset) = &ls_config.required_root_patterns { if !root_path @@ -956,46 +956,62 @@ pub fn find_lsp_workspace( workspace: &Path, workspace_is_cwd: bool, ) -> Option { - let file = std::path::Path::new(file); - let mut file = if file.is_absolute() { - file.to_path_buf() + let file_path = std::path::Path::new(file); + let initial_file_path = if file_path.is_absolute() { + file_path.to_path_buf() } else { let current_dir = helix_stdx::env::current_working_dir(); - current_dir.join(file) + current_dir.join(file_path) }; - file = path::normalize(&file); + // Attempt to canonicalize the initial file path. If it fails, we might not be able to proceed. + // For now, let's return None if canonicalization of the base file path fails. + let file = match helix_stdx::path::canonicalize(&initial_file_path) { + Ok(p) => p, + Err(_) => return None, // Or handle error more explicitly + }; + + // Workspace itself should be canonical. The caller (start_client) ensures this. if !file.starts_with(workspace) { return None; } - let mut top_marker = None; + let mut top_marker_path = None; for ancestor in file.ancestors() { + // Canonicalize ancestor before joining and checking for markers/root_dirs + // However, ancestor itself is already part of a canonical path `file`. + // The paths joined (marker, root_dir) should be relative to a canonical ancestor. + // And `workspace.join(root_dir)` should be canonicalized for comparison. + if root_markers .iter() .any(|marker| ancestor.join(marker).exists()) { - top_marker = Some(ancestor); + // Store the canonical ancestor if a marker is found + top_marker_path = Some(ancestor.to_path_buf()); } - if root_dirs - .iter() - .any(|root_dir| path::normalize(workspace.join(root_dir)) == ancestor) - { - // if the worskapce is the cwd do not search any higher for workspaces - // but specify - return Some(top_marker.unwrap_or(workspace).to_owned()); + if root_dirs.iter().any(|root_dir| { + match helix_stdx::path::canonicalize(&workspace.join(root_dir)) { + Ok(canonical_root_dir) => canonical_root_dir == ancestor, + Err(_) => false, + } + }) { + // If a root_dir matches the current ancestor, decide what to return. + // The path returned must be canonical. + return top_marker_path.or_else(|| Some(ancestor.to_path_buf())); } + if ancestor == workspace { - // if the workspace is the CWD, let the LSP decide what the workspace - // is - return top_marker - .or_else(|| (!workspace_is_cwd).then_some(workspace)) - .map(Path::to_owned); + // If we've reached the workspace root. + // `top_marker_path` would be a canonical path if set. + // `workspace` is already canonical. + return top_marker_path + .or_else(|| (!workspace_is_cwd).then_some(workspace.to_path_buf())); } } - debug_assert!(false, "workspace must be an ancestor of "); + debug_assert!(false, "workspace must be an ancestor of or logic error in loop"); None } diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index 469242e40..083e9c3bc 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -22,4 +22,5 @@ mod test { mod languages; mod movement; mod splits; + mod lsp; } diff --git a/helix-term/tests/lsp.rs b/helix-term/tests/lsp.rs new file mode 100644 index 000000000..251b9b2cf --- /dev/null +++ b/helix-term/tests/lsp.rs @@ -0,0 +1,84 @@ +#[cfg(all(feature = "integration", unix))] // Keep cfg for consistency, though it won't be an integration test +mod test { + use std::{fs, path::PathBuf, os::unix::fs::symlink}; + use tempfile::tempdir; + use indoc::indoc; + use helix_lsp::lsp::Url; // Only Url needed for this simplified test + + // Note: This is no longer an async tokio test. + // Note: This does not use AppBuilder or Application. + #[test] + fn verify_symlink_canonicalization_for_uri() -> anyhow::Result<()> { + println!("--- Test verify_symlink_canonicalization_for_uri started ---"); + + // 1. Create a temporary directory for test files. + let temp_dir = tempdir()?; + let dir_path = temp_dir.path(); + println!("Temporary directory created: {:?}", dir_path); + + // 2. Inside this directory: + // a. Create original_file.py + let original_file_path = dir_path.join("original_file.py"); + let python_content = indoc! {r#" + def my_function(): + pass + + my_function() + "#}; + fs::write(&original_file_path, python_content)?; + println!("original_file.py created at: {:?}", original_file_path); + + // b. Create a symlink linked_file.py pointing to original_file.py + let linked_file_path = dir_path.join("linked_file.py"); + symlink(&original_file_path, &linked_file_path)?; + println!("linked_file.py created at: {:?}, pointing to {:?}", linked_file_path, original_file_path); + + // Core Logic Verification: + // Get the canonical path for both the original and the symlinked file. + let canonical_original_path = original_file_path.canonicalize()?; + let canonical_linked_path = linked_file_path.canonicalize()?; + println!("Canonical original path: {:?}", canonical_original_path); + println!("Canonical linked path: {:?}", canonical_linked_path); + + // Assert that the canonical paths are the same. + assert_eq!(canonical_original_path, canonical_linked_path, "Canonical paths of original and symlink should be identical."); + println!("Assertion 1 passed: Canonical paths are identical."); + + // Convert these canonical paths to file URIs. + let uri_from_original = Url::from_file_path(canonical_original_path).map_err(|_| anyhow::anyhow!("Failed to create URI from original path"))?; + let uri_from_linked = Url::from_file_path(canonical_linked_path).map_err(|_| anyhow::anyhow!("Failed to create URI from linked path"))?; + println!("URI from original's canonical path: {:?}", uri_from_original); + println!("URI from linked file's canonical path: {:?}", uri_from_linked); + + // Assert that the URIs are the same. + assert_eq!(uri_from_original, uri_from_linked, "URIs from canonical paths should be identical."); + println!("Assertion 2 passed: URIs from canonical paths are identical."); + + // Also, check if creating a URI from the non-canonical symlink path, + // and then canonicalizing the path from *that* URI (if possible, though Url doesn't directly do that), + // would match. The key is that `helix_stdx::path::canonicalize` should be used *before* Uri creation + // as per the original subtask that modified `convert_url_to_uri`. + + // The original change was: Uri::File(helix_stdx::path::canonicalize(path).into()) + // So, if we simulate this: + // 1. Path comes from url.to_file_path() - this would be /path/to/linked_file.py + // 2. Then helix_stdx::path::canonicalize is applied to it. + + let path_from_symlink_url = linked_file_path; // Simulating url.to_file_path() for the symlink + let canonicalized_path_for_uri_construction = helix_stdx::path::canonicalize(path_from_symlink_url).map_err(|e| anyhow::anyhow!("helix_stdx::path::canonicalize failed: {}",e))?; + println!("Path from symlink after helix_stdx::path::canonicalize: {:?}", canonicalized_path_for_uri_construction); + + assert_eq!(canonicalized_path_for_uri_construction, canonical_original_path, "helix_stdx::path::canonicalize(symlink_path) should yield original's canonical path."); + println!("Assertion 3 passed: helix_stdx::path::canonicalize(symlink_path) is correct."); + + let constructed_uri = Url::from_file_path(canonicalized_path_for_uri_construction).map_err(|_| anyhow::anyhow!("Failed to create URI from stdx canonicalized path"))?; + assert_eq!(constructed_uri, uri_from_original, "URI constructed using helix_stdx::path::canonicalize should match original's canonical URI."); + println!("Assertion 4 passed: Final URI construction matches."); + + + // Clean up the temporary directory + temp_dir.close()?; + println!("--- Test verify_symlink_canonicalization_for_uri finished ---"); + Ok(()) + } +}