From 888f4fef6f975412c8215c4b76871ffba6e1b41d Mon Sep 17 00:00:00 2001 From: Riccardo Binetti Date: Fri, 23 Sep 2022 10:04:07 +0200 Subject: [PATCH] Split helix_core::find_root and helix_loader::find_local_config_dirs (#3929) * Split helix_core::find_root and helix_loader::find_local_config_dirs The documentation of find_root described the following priority for detecting a project root: - Top-most folder containing a root marker in current git repository - Git repository root if no marker detected - Top-most folder containing a root marker if not git repository detected - Current working directory as fallback The commit contained in https://github.com/helix-editor/helix/pull/1249 extracted and changed the implementation of find_root in find_root_impl, actually reversing its result order (since that is the order that made sense for the local configuration merge, from innermost to outermost ancestors). Since the two uses of find_root_impl have different requirements (and it's not a matter of reversing the order of results since, e.g., the top repository dir should be used by find_root only if there's not marker in other dirs), this PR splits the two implementations in two different specialized functions. In doing so, find_root_impl is removed and the implementation is moved back in find_root, moving it closer to the documented behaviour thus making it easier to verify it's actually correct * helix-core: remove Option from find_root return type It always returns some result, so Option is not needed --- helix-core/src/lib.rs | 40 ++++++++++++++++++++++++++++++++++---- helix-loader/src/lib.rs | 26 +++++-------------------- helix-lsp/src/client.rs | 11 +++-------- helix-term/src/commands.rs | 5 +++-- 4 files changed, 47 insertions(+), 35 deletions(-) diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 735a62c1b..8f869e352 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -46,10 +46,42 @@ pub fn find_first_non_whitespace_char(line: RopeSlice) -> Option { /// * Git repository root if no marker detected /// * Top-most folder containing a root marker if not git repository detected /// * Current working directory as fallback -pub fn find_root(root: Option<&str>, root_markers: &[String]) -> Option { - helix_loader::find_root_impl(root, root_markers) - .first() - .cloned() +pub fn find_root(root: Option<&str>, root_markers: &[String]) -> std::path::PathBuf { + let current_dir = std::env::current_dir().expect("unable to determine current directory"); + + let root = match root { + Some(root) => { + let root = std::path::Path::new(root); + if root.is_absolute() { + root.to_path_buf() + } else { + current_dir.join(root) + } + } + None => current_dir.clone(), + }; + + let mut top_marker = None; + for ancestor in root.ancestors() { + if root_markers + .iter() + .any(|marker| ancestor.join(marker).exists()) + { + top_marker = Some(ancestor); + } + + if ancestor.join(".git").is_dir() { + // Top marker is repo root if not root marker was detected yet + if top_marker.is_none() { + top_marker = Some(ancestor); + } + // Don't go higher than repo if we're in one + break; + } + } + + // Return the found top marker or the current_dir as fallback + top_marker.map_or(current_dir, |a| a.to_path_buf()) } pub use ropey::{str_utils, Rope, RopeBuilder, RopeSlice}; diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 3c9905f5a..a02a59afc 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -59,7 +59,7 @@ pub fn config_dir() -> PathBuf { } pub fn local_config_dirs() -> Vec { - let directories = find_root_impl(None, &[".helix".to_string()]) + let directories = find_local_config_dirs() .into_iter() .map(|path| path.join(".helix")) .collect(); @@ -90,32 +90,16 @@ pub fn log_file() -> PathBuf { cache_dir().join("helix.log") } -pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec { +pub fn find_local_config_dirs() -> Vec { let current_dir = std::env::current_dir().expect("unable to determine current directory"); let mut directories = Vec::new(); - let root = match root { - Some(root) => { - let root = std::path::Path::new(root); - if root.is_absolute() { - root.to_path_buf() - } else { - current_dir.join(root) - } - } - None => current_dir, - }; - - for ancestor in root.ancestors() { - // don't go higher than repo + for ancestor in current_dir.ancestors() { if ancestor.join(".git").is_dir() { - // Use workspace if detected from marker directories.push(ancestor.to_path_buf()); + // Don't go higher than repo if we're in one break; - } else if root_markers - .iter() - .any(|marker| ancestor.join(marker).exists()) - { + } else if ancestor.join(".helix").is_dir() { directories.push(ancestor.to_path_buf()); } } diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 9ae8f20e5..497ce80cf 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -34,7 +34,7 @@ pub struct Client { pub(crate) capabilities: OnceCell, offset_encoding: OffsetEncoding, config: Option, - root_path: Option, + root_path: std::path::PathBuf, root_uri: Option, workspace_folders: Vec, req_timeout: u64, @@ -74,9 +74,7 @@ impl Client { let root_path = find_root(None, root_markers); - let root_uri = root_path - .clone() - .and_then(|root| lsp::Url::from_file_path(root).ok()); + let root_uri = lsp::Url::from_file_path(root_path.clone()).ok(); // TODO: support multiple workspace folders let workspace_folders = root_uri @@ -281,10 +279,7 @@ impl Client { workspace_folders: Some(self.workspace_folders.clone()), // root_path is obsolete, but some clients like pyright still use it so we specify both. // clients will prefer _uri if possible - root_path: self - .root_path - .clone() - .and_then(|path| path.to_str().map(|path| path.to_owned())), + root_path: self.root_path.to_str().map(|path| path.to_owned()), root_uri: self.root_uri.clone(), initialization_options: self.config.clone(), capabilities: lsp::ClientCapabilities { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a5203352e..c87ad0ca2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2225,8 +2225,9 @@ fn append_mode(cx: &mut Context) { } fn file_picker(cx: &mut Context) { - // We don't specify language markers, root will be the root of the current git repo - let root = find_root(None, &[]).unwrap_or_else(|| PathBuf::from("./")); + // We don't specify language markers, root will be the root of the current + // git repo or the current dir if we're not in a repo + let root = find_root(None, &[]); let picker = ui::file_picker(root, &cx.editor.config()); cx.push_layer(Box::new(overlayed(picker))); }