From 9fc753804eac21dbb4882788c96a914260ac782f Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Fri, 30 Aug 2024 02:47:49 -0700 Subject: [PATCH 1/9] refactor run --- src/search.rs | 18 ++++++- src/subcommand.rs | 124 ++++++++++++++++------------------------------ 2 files changed, 60 insertions(+), 82 deletions(-) diff --git a/src/search.rs b/src/search.rs index 01dcdacff9..233e638bd4 100644 --- a/src/search.rs +++ b/src/search.rs @@ -34,12 +34,13 @@ impl Search { paths } + /// Attempt to find a justfile given the search configuration and invocation directory pub(crate) fn find( search_config: &SearchConfig, invocation_directory: &Path, ) -> SearchResult { match search_config { - SearchConfig::FromInvocationDirectory => Self::find_next(invocation_directory), + SearchConfig::FromInvocationDirectory => Self::find_in_directory(invocation_directory), SearchConfig::FromSearchDirectory { search_directory } => { let search_directory = Self::clean(invocation_directory, search_directory); let justfile = Self::justfile(&search_directory)?; @@ -75,7 +76,20 @@ impl Search { } } - pub(crate) fn find_next(starting_dir: &Path) -> SearchResult { + /// Look for a justfile starting from the parent directory to the current justfile + pub(crate) fn search_parent_directory(&self) -> SearchResult { + let parent_dir = self + .justfile + .parent() + .and_then(|p| p.parent()) + .ok_or_else(|| SearchError::JustfileHadNoParent { + path: self.justfile.clone(), + })?; + Self::find_in_directory(parent_dir) + } + + /// Find a justfile starting in the given directory and searching upwards in the directory tree + fn find_in_directory(starting_dir: &Path) -> SearchResult { let justfile = Self::justfile(starting_dir)?; let working_directory = Self::working_directory_from_justfile(&justfile)?; Ok(Self { diff --git a/src/subcommand.rs b/src/subcommand.rs index 449d4a0920..36a9a9c0fd 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -53,10 +53,6 @@ impl Subcommand { Completions { shell } => return Self::completions(*shell), Init => return Self::init(config), Man => return Self::man(), - Run { - arguments, - overrides, - } => return Self::run(config, loader, arguments, overrides), _ => {} } @@ -70,6 +66,10 @@ impl Subcommand { let justfile = &compilation.justfile; match self { + Run { + arguments, + overrides, + } => Self::run(config, loader, search, arguments, overrides)?, Choose { overrides, chooser } => { Self::choose(config, justfile, &search, overrides, chooser.as_deref())?; } @@ -83,7 +83,7 @@ impl Subcommand { Show { path } => Self::show(config, justfile, path)?, Summary => Self::summary(config, justfile), Variables => Self::variables(justfile), - Changelog | Completions { .. } | Edit | Init | Man | Run { .. } => unreachable!(), + Changelog | Completions { .. } | Edit | Init | Man => unreachable!(), } Ok(()) @@ -99,90 +99,54 @@ impl Subcommand { fn run<'src>( config: &Config, loader: &'src Loader, + initial_search: Search, arguments: &[String], overrides: &BTreeMap, ) -> RunResult<'src> { - if matches!( - config.search_config, - SearchConfig::FromInvocationDirectory | SearchConfig::FromSearchDirectory { .. } - ) { - let starting_path = match &config.search_config { - SearchConfig::FromInvocationDirectory => config.invocation_directory.clone(), - SearchConfig::FromSearchDirectory { search_directory } => config - .invocation_directory - .join(search_directory) - .lexiclean(), - _ => unreachable!(), - }; - - let mut path = starting_path.clone(); - - let mut unknown_recipes_errors = None; - - loop { - let search = match Search::find_next(&path) { - Err(SearchError::NotFound) => match unknown_recipes_errors { - Some(err) => return Err(err), - None => return Err(SearchError::NotFound.into()), - }, - Err(err) => return Err(err.into()), - Ok(search) => { - if config.verbosity.loquacious() && path != starting_path { - eprintln!( - "Trying {}", - starting_path - .strip_prefix(path) - .unwrap() - .components() - .map(|_| path::Component::ParentDir) - .collect::() - .join(search.justfile.file_name().unwrap()) - .display() - ); - } - search - } - }; - - match Self::run_inner(config, loader, arguments, overrides, &search) { - Err((err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. }), true)) => { - match search.justfile.parent().unwrap().parent() { - Some(parent) => { - unknown_recipes_errors.get_or_insert(err); - path = parent.into(); - } - None => return Err(err), - } + let starting_path = initial_search + .justfile + .parent() + .as_ref() + .unwrap() + .lexiclean(); + + let mut search = initial_search; + + loop { + let compilation = Self::compile(config, loader, &search)?; + let justfile = &compilation.justfile; + let fallback = justfile.settings.fallback + && matches!( + config.search_config, + SearchConfig::FromInvocationDirectory | SearchConfig::FromSearchDirectory { .. } + ); + let run_result = justfile.run(config, &search, overrides, arguments); + + match run_result { + Err(err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. })) if fallback => { + let new_search = search + .search_parent_directory() + .map_err(|_search_err| err)?; + let p = new_search.justfile.parent().unwrap(); + let new_path = starting_path + .strip_prefix(p) + .unwrap() + .components() + .map(|_| path::Component::ParentDir) + .collect::() + .join(new_search.justfile.file_name().unwrap()); + + search = new_search; + + if config.verbosity.loquacious() { + eprintln!("Trying {}", new_path.display()); } - result => return result.map_err(|(err, _fallback)| err), } + result => return result, } - } else { - Self::run_inner( - config, - loader, - arguments, - overrides, - &Search::find(&config.search_config, &config.invocation_directory)?, - ) - .map_err(|(err, _fallback)| err) } } - fn run_inner<'src>( - config: &Config, - loader: &'src Loader, - arguments: &[String], - overrides: &BTreeMap, - search: &Search, - ) -> Result<(), (Error<'src>, bool)> { - let compilation = Self::compile(config, loader, search).map_err(|err| (err, false))?; - let justfile = &compilation.justfile; - justfile - .run(config, search, overrides, arguments) - .map_err(|err| (err, justfile.settings.fallback)) - } - fn compile<'src>( config: &Config, loader: &'src Loader, From f20298967e6d1dfced9ea7585cde63b68352978f Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Mon, 2 Sep 2024 15:27:36 -0700 Subject: [PATCH 2/9] Avoid double-compile --- src/subcommand.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index 36a9a9c0fd..a0f4621dec 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -69,7 +69,7 @@ impl Subcommand { Run { arguments, overrides, - } => Self::run(config, loader, search, arguments, overrides)?, + } => Self::run(config, loader, search, compilation, arguments, overrides)?, Choose { overrides, chooser } => { Self::choose(config, justfile, &search, overrides, chooser.as_deref())?; } @@ -100,6 +100,7 @@ impl Subcommand { config: &Config, loader: &'src Loader, initial_search: Search, + initial_compilation: Compilation<'src>, arguments: &[String], overrides: &BTreeMap, ) -> RunResult<'src> { @@ -111,9 +112,9 @@ impl Subcommand { .lexiclean(); let mut search = initial_search; + let mut compilation = initial_compilation; loop { - let compilation = Self::compile(config, loader, &search)?; let justfile = &compilation.justfile; let fallback = justfile.settings.fallback && matches!( @@ -141,6 +142,7 @@ impl Subcommand { if config.verbosity.loquacious() { eprintln!("Trying {}", new_path.display()); } + compilation = Self::compile(config, loader, &search)?; } result => return result, } From 3aab5fc0f3b9ad4aff601e79be8d3698e17c25da Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:22:04 -0700 Subject: [PATCH 3/9] Reuse search and completion variables --- src/subcommand.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index 9a8979fafc..fd9245c1ed 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -99,20 +99,12 @@ impl Subcommand { fn run<'src>( config: &Config, loader: &'src Loader, - initial_search: Search, - initial_compilation: Compilation<'src>, + mut search: Search, + mut compilation: Compilation<'src>, arguments: &[String], overrides: &BTreeMap, ) -> RunResult<'src> { - let starting_path = initial_search - .justfile - .parent() - .as_ref() - .unwrap() - .lexiclean(); - - let mut search = initial_search; - let mut compilation = initial_compilation; + let starting_path = search.justfile.parent().as_ref().unwrap().lexiclean(); loop { let justfile = &compilation.justfile; From 99fa4cb86a5c45864e93b5964d1b28cb8fe2eae6 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:23:39 -0700 Subject: [PATCH 4/9] Reuse search variable --- src/subcommand.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index fd9245c1ed..5fe1d49489 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -117,19 +117,17 @@ impl Subcommand { match run_result { Err(err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. })) if fallback => { - let new_search = search + search = search .search_parent_directory() .map_err(|_search_err| err)?; - let p = new_search.justfile.parent().unwrap(); + let p = search.justfile.parent().unwrap(); let new_path = starting_path .strip_prefix(p) .unwrap() .components() .map(|_| path::Component::ParentDir) .collect::() - .join(new_search.justfile.file_name().unwrap()); - - search = new_search; + .join(search.justfile.file_name().unwrap()); if config.verbosity.loquacious() { eprintln!("Trying {}", new_path.display()); From 80d5803d18517f04114210a50b37330e66ea830b Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:31:30 -0700 Subject: [PATCH 5/9] Refactor --- src/subcommand.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index 5fe1d49489..7e9e45ee3a 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -104,7 +104,7 @@ impl Subcommand { arguments: &[String], overrides: &BTreeMap, ) -> RunResult<'src> { - let starting_path = search.justfile.parent().as_ref().unwrap().lexiclean(); + let starting_parent = search.justfile.parent().as_ref().unwrap().lexiclean(); loop { let justfile = &compilation.justfile; @@ -113,16 +113,17 @@ impl Subcommand { config.search_config, SearchConfig::FromInvocationDirectory | SearchConfig::FromSearchDirectory { .. } ); - let run_result = justfile.run(config, &search, overrides, arguments); - match run_result { - Err(err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. })) if fallback => { + let result = justfile.run(config, &search, overrides, arguments); + + if fallback { + if let Err(err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. })) = result { search = search .search_parent_directory() .map_err(|_search_err| err)?; - let p = search.justfile.parent().unwrap(); - let new_path = starting_path - .strip_prefix(p) + + let new_parent = starting_parent + .strip_prefix(search.justfile.parent().unwrap()) .unwrap() .components() .map(|_| path::Component::ParentDir) @@ -130,12 +131,16 @@ impl Subcommand { .join(search.justfile.file_name().unwrap()); if config.verbosity.loquacious() { - eprintln!("Trying {}", new_path.display()); + eprintln!("Trying {}", new_parent.display()); } + compilation = Self::compile(config, loader, &search)?; + + continue; } - result => return result, } + + return result; } } From 89a5f0f44085185e6f6f60013d7cddb38f126dd5 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:34:36 -0700 Subject: [PATCH 6/9] Enhance --- src/subcommand.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/subcommand.rs b/src/subcommand.rs index 7e9e45ee3a..eb8f3c6cec 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -118,9 +118,7 @@ impl Subcommand { if fallback { if let Err(err @ (Error::UnknownRecipe { .. } | Error::UnknownSubmodule { .. })) = result { - search = search - .search_parent_directory() - .map_err(|_search_err| err)?; + search = search.search_parent_directory().map_err(|_| err)?; let new_parent = starting_parent .strip_prefix(search.justfile.parent().unwrap()) From 10efe358feef316da63fe10d5cf9c48a93e6d6fb Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:35:09 -0700 Subject: [PATCH 7/9] Enhance --- src/search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search.rs b/src/search.rs index 233e638bd4..b2c714ace6 100644 --- a/src/search.rs +++ b/src/search.rs @@ -34,7 +34,7 @@ impl Search { paths } - /// Attempt to find a justfile given the search configuration and invocation directory + /// find justfile given search configuration and invocation directory pub(crate) fn find( search_config: &SearchConfig, invocation_directory: &Path, From 5b1ba6d451f0516775b2c2d59ed4c2ae46d7c752 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:36:06 -0700 Subject: [PATCH 8/9] Revise --- src/search.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/search.rs b/src/search.rs index b2c714ace6..f6b9d97b92 100644 --- a/src/search.rs +++ b/src/search.rs @@ -76,16 +76,16 @@ impl Search { } } - /// Look for a justfile starting from the parent directory to the current justfile + /// find justfile starting from parent directory of current justfile pub(crate) fn search_parent_directory(&self) -> SearchResult { - let parent_dir = self + let parent = self .justfile .parent() - .and_then(|p| p.parent()) + .and_then(|path| path.parent()) .ok_or_else(|| SearchError::JustfileHadNoParent { path: self.justfile.clone(), })?; - Self::find_in_directory(parent_dir) + Self::find_in_directory(parent) } /// Find a justfile starting in the given directory and searching upwards in the directory tree From 8d131eefc16512de64547c8743e096171a81c56f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 6 Sep 2024 14:36:41 -0700 Subject: [PATCH 9/9] Adapt --- src/search.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/search.rs b/src/search.rs index f6b9d97b92..09d8f1a5ee 100644 --- a/src/search.rs +++ b/src/search.rs @@ -34,7 +34,7 @@ impl Search { paths } - /// find justfile given search configuration and invocation directory + /// Find justfile given search configuration and invocation directory pub(crate) fn find( search_config: &SearchConfig, invocation_directory: &Path, @@ -76,7 +76,7 @@ impl Search { } } - /// find justfile starting from parent directory of current justfile + /// Find justfile starting from parent directory of current justfile pub(crate) fn search_parent_directory(&self) -> SearchResult { let parent = self .justfile @@ -88,7 +88,7 @@ impl Search { Self::find_in_directory(parent) } - /// Find a justfile starting in the given directory and searching upwards in the directory tree + /// Find justfile starting in given directory searching upwards in directory tree fn find_in_directory(starting_dir: &Path) -> SearchResult { let justfile = Self::justfile(starting_dir)?; let working_directory = Self::working_directory_from_justfile(&justfile)?;