on master help sign in
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

commit888f4fef6f975412c8215c4b76871ffba6e1b41d authorRiccardo Binetti <rbino@gmx.com> dateFri 23 Sep 2022, 10:04 (4 years ago) parent4133f1f
4 files changed+47−35
helix-core/src/lib.rs+36−4 helix-loader/src/lib.rs+5−21 helix-lsp/src/client.rs+3−8 helix-term/src/commands.rs+3−2
helix-core/src/lib.rs+36−4
@@ -46,10 +46,42 @@ pub fn find_first_non_whitespace_char(line: RopeSlice) -> Option<usize> {
4646/// * Git repository root if no marker detected
4747/// * Top-most folder containing a root marker if not git repository detected
4848/// * Current working directory as fallback
49−pub fn find_root(root: Option<&str>, root_markers: &[String]) -> Option<std::path::PathBuf> {
50− helix_loader::find_root_impl(root, root_markers)
51− .first()
52− .cloned()
49+pub fn find_root(root: Option<&str>, root_markers: &[String]) -> std::path::PathBuf {
50+ let current_dir = std::env::current_dir().expect("unable to determine current directory");
51+
52+ let root = match root {
53+ Some(root) => {
54+ let root = std::path::Path::new(root);
55+ if root.is_absolute() {
56+ root.to_path_buf()
57+ } else {
58+ current_dir.join(root)
59+ }
60+ }
61+ None => current_dir.clone(),
62+ };
63+
64+ let mut top_marker = None;
65+ for ancestor in root.ancestors() {
66+ if root_markers
67+ .iter()
68+ .any(|marker| ancestor.join(marker).exists())
69+ {
70+ top_marker = Some(ancestor);
71+ }
72+
73+ if ancestor.join(".git").is_dir() {
74+ // Top marker is repo root if not root marker was detected yet
75+ if top_marker.is_none() {
76+ top_marker = Some(ancestor);
77+ }
78+ // Don't go higher than repo if we're in one
79+ break;
80+ }
81+ }
82+
83+ // Return the found top marker or the current_dir as fallback
84+ top_marker.map_or(current_dir, |a| a.to_path_buf())
5385}
5486
5587pub use ropey::{str_utils, Rope, RopeBuilder, RopeSlice};
helix-loader/src/lib.rs+5−21
@@ -59,7 +59,7 @@ pub fn config_dir() -> PathBuf {
5959}
6060
6161pub fn local_config_dirs() -> Vec<PathBuf> {
62− let directories = find_root_impl(None, &[".helix".to_string()])
62+ let directories = find_local_config_dirs()
6363 .into_iter()
6464 .map(|path| path.join(".helix"))
6565 .collect();
@@ -90,32 +90,16 @@ pub fn log_file() -> PathBuf {
9090 cache_dir().join("helix.log")
9191}
9292
93−pub fn find_root_impl(root: Option<&str>, root_markers: &[String]) -> Vec<PathBuf> {
93+pub fn find_local_config_dirs() -> Vec<PathBuf> {
9494 let current_dir = std::env::current_dir().expect("unable to determine current directory");
9595 let mut directories = Vec::new();
9696
97− let root = match root {
98− Some(root) => {
99− let root = std::path::Path::new(root);
100− if root.is_absolute() {
101− root.to_path_buf()
102− } else {
103− current_dir.join(root)
104− }
105− }
106− None => current_dir,
107− };
108−
109− for ancestor in root.ancestors() {
110− // don't go higher than repo
97+ for ancestor in current_dir.ancestors() {
11198 if ancestor.join(".git").is_dir() {
112− // Use workspace if detected from marker
11399 directories.push(ancestor.to_path_buf());
100+ // Don't go higher than repo if we're in one
114101 break;
115− } else if root_markers
116− .iter()
117− .any(|marker| ancestor.join(marker).exists())
118− {
102+ } else if ancestor.join(".helix").is_dir() {
119103 directories.push(ancestor.to_path_buf());
120104 }
121105 }
helix-lsp/src/client.rs+3−8
@@ -34,7 +34,7 @@ pub struct Client {
3434 pub(crate) capabilities: OnceCell<lsp::ServerCapabilities>,
3535 offset_encoding: OffsetEncoding,
3636 config: Option<Value>,
37− root_path: Option<std::path::PathBuf>,
37+ root_path: std::path::PathBuf,
3838 root_uri: Option<lsp::Url>,
3939 workspace_folders: Vec<lsp::WorkspaceFolder>,
4040 req_timeout: u64,
@@ -74,9 +74,7 @@ impl Client {
7474
7575 let root_path = find_root(None, root_markers);
7676
77− let root_uri = root_path
78− .clone()
79− .and_then(|root| lsp::Url::from_file_path(root).ok());
77+ let root_uri = lsp::Url::from_file_path(root_path.clone()).ok();
8078
8179 // TODO: support multiple workspace folders
8280 let workspace_folders = root_uri
@@ -281,10 +279,7 @@ impl Client {
281279 workspace_folders: Some(self.workspace_folders.clone()),
282280 // root_path is obsolete, but some clients like pyright still use it so we specify both.
283281 // clients will prefer _uri if possible
284− root_path: self
285− .root_path
286− .clone()
287− .and_then(|path| path.to_str().map(|path| path.to_owned())),
282+ root_path: self.root_path.to_str().map(|path| path.to_owned()),
288283 root_uri: self.root_uri.clone(),
289284 initialization_options: self.config.clone(),
290285 capabilities: lsp::ClientCapabilities {
helix-term/src/commands.rs+3−2
@@ -2225,8 +2225,9 @@ fn append_mode(cx: &mut Context) {
22252225}
22262226
22272227fn file_picker(cx: &mut Context) {
2228− // We don't specify language markers, root will be the root of the current git repo
2229− let root = find_root(None, &[]).unwrap_or_else(|| PathBuf::from("./"));
2228+ // We don't specify language markers, root will be the root of the current
2229+ // git repo or the current dir if we're not in a repo
2230+ let root = find_root(None, &[]);
22302231 let picker = ui::file_picker(root, &cx.editor.config());
22312232 cx.push_layer(Box::new(overlayed(picker)));
22322233}