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 #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

Author: rbino

helix-core/src/lib.rs

@@ -47,9 +47,41 @@ pub fn find_first_non_whitespace_char(line: RopeSlice) -> Option<usize> {
 /// * 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<std::path::PathBuf> {
-    helix_loader::find_root_impl(root, root_markers)
-        .first()
-        .cloned()
+    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(|a| a.to_path_buf()).or(Some(current_dir))
 }
 
 pub use ropey::{str_utils, Rope, RopeBuilder, RopeSlice};

helix-loader/src/lib.rs

@@ -59,7 +59,7 @@ pub fn config_dir() -> PathBuf {
 }
 
 pub fn local_config_dirs() -> Vec<PathBuf> {
-    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<PathBuf> {
+pub fn find_local_config_dirs() -> Vec<PathBuf> {
     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());
         }
     }