diff --git a/scope/src/shared/models/internal/command.rs b/scope/src/shared/models/internal/command.rs index 238acfc..1992e1c 100644 --- a/scope/src/shared/models/internal/command.rs +++ b/scope/src/shared/models/internal/command.rs @@ -1,8 +1,8 @@ -use std::path::Path; - +use anyhow::Result; use derive_builder::Builder; +use std::path::Path; -use super::extract_command_path; +use super::{extract_command_path, substitute_templates}; #[derive(Debug, PartialEq, Clone, Builder)] #[builder(setter(into))] @@ -10,6 +10,20 @@ pub struct DoctorCommand { pub commands: Vec, } +impl DoctorCommand { + pub fn from_commands( + containing_dir: &Path, + working_dir: &str, + commands: &Vec, + ) -> Result { + let mut templated_commands = Vec::new(); + for command in commands { + templated_commands.push(substitute_templates(working_dir, command)?); + } + Ok(DoctorCommand::from((containing_dir, templated_commands))) + } +} + impl From<(&Path, Vec)> for DoctorCommand where String: for<'a> From<&'a T>, @@ -74,4 +88,26 @@ mod tests { actual ) } + + #[test] + fn from_commands_succeeds() { + let containing_dir = Path::new("/foo/bar"); + let working_dir = "/some/working_dir"; + let commands = vec!["{{ working_dir }}/foo.sh", "./bar.sh"] + .iter() + .map(|cmd| cmd.to_string()) + .collect::>(); + + let actual = DoctorCommand::from_commands(containing_dir, working_dir, &commands) + .expect("Expected Ok"); + + let templated_commands = commands + .iter() + .map(|cmd| substitute_templates(&working_dir, &cmd).unwrap()) + .collect::>(); + + let expected = DoctorCommand::from((containing_dir, templated_commands)); + + assert_eq!(expected, actual) + } } diff --git a/scope/src/shared/models/internal/doctor_group.rs b/scope/src/shared/models/internal/doctor_group.rs index 828a4ff..53e5df7 100644 --- a/scope/src/shared/models/internal/doctor_group.rs +++ b/scope/src/shared/models/internal/doctor_group.rs @@ -3,12 +3,13 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use derive_builder::Builder; -use minijinja::{context, Environment}; use crate::models::prelude::{ModelMetadata, V1AlphaDoctorGroup}; use crate::models::HelpMetadata; use crate::prelude::{DoctorGroupActionSpec, DoctorInclude}; -use crate::shared::models::internal::{DoctorCommand, DoctorFix, DoctorFixPrompt}; +use crate::shared::models::internal::{DoctorCommand, DoctorFix}; + +use super::substitute_templates; #[derive(Debug, PartialEq, Clone, Builder)] #[builder(setter(into))] @@ -67,15 +68,6 @@ impl HelpMetadata for DoctorGroup { } } -fn substitute_templates(work_dir: &str, input_str: &str) -> Result { - let mut env = Environment::new(); - env.add_template("input_str", input_str)?; - let template = env.get_template("input_str")?; - let result = template.render(context! { working_dir => work_dir })?; - - Ok(result) -} - impl TryFrom for DoctorGroup { type Error = anyhow::Error; @@ -112,29 +104,19 @@ fn parse_action( .clone(); let spec_action = action.clone(); - let help_text = spec_action - .fix - .as_ref() - .and_then(|x| x.help_text.as_ref().map(|st| st.trim().to_string()).clone()); - let help_url = spec_action.fix.as_ref().and_then(|x| x.help_url.clone()); - let fix_command = if let Some(fix) = &spec_action.fix { - let mut templated_commands = Vec::new(); - for command in &fix.commands { - templated_commands.push(substitute_templates(&working_dir, command)?); - } - Some(DoctorCommand::from((containing_dir, templated_commands))) - } else { - None + + let fix = match &spec_action.fix { + Some(fix_spec) => DoctorFix::from_spec(containing_dir, &working_dir, fix_spec.clone())?, + None => DoctorFix::empty(), }; - let check_command = if let Some(ref check) = spec_action.check.commands { - let mut templated_commands = Vec::new(); - for command in check { - templated_commands.push(substitute_templates(&working_dir, command)?); - } - Some(DoctorCommand::from((containing_dir, templated_commands))) - } else { - None + let check_command = match spec_action.check.commands { + Some(ref check) => Some(DoctorCommand::from_commands( + containing_dir, + &working_dir, + check, + )?), + None => None, }; Ok(DoctorGroupAction { @@ -143,15 +125,7 @@ fn parse_action( description: spec_action .description .unwrap_or_else(|| "default".to_string()), - fix: DoctorFix { - command: fix_command, - prompt: spec_action - .fix - .and_then(|fix| fix.prompt) - .map(DoctorFixPrompt::from), - help_text, - help_url, - }, + fix, check: DoctorGroupActionCheck { command: check_command, files: spec_action.check.paths.map(|paths| DoctorGroupCachePath { diff --git a/scope/src/shared/models/internal/fix.rs b/scope/src/shared/models/internal/fix.rs index fe25781..8da52e9 100644 --- a/scope/src/shared/models/internal/fix.rs +++ b/scope/src/shared/models/internal/fix.rs @@ -1,4 +1,10 @@ -use crate::{prelude::DoctorFixPromptSpec, shared::prelude::*}; +use anyhow::Result; +use std::path::Path; + +use crate::{ + prelude::{DoctorFixPromptSpec, DoctorFixSpec}, + shared::prelude::*, +}; use derive_builder::Builder; #[derive(Debug, PartialEq, Clone, Builder)] @@ -14,6 +20,35 @@ pub struct DoctorFix { pub prompt: Option, } +impl DoctorFix { + pub fn empty() -> Self { + DoctorFix { + command: None, + help_text: None, + help_url: None, + prompt: None, + } + } + + pub fn from_spec(containing_dir: &Path, working_dir: &str, fix: DoctorFixSpec) -> Result { + let commands = DoctorCommand::from_commands(containing_dir, working_dir, &fix.commands)?; + let help_text = fix + .help_text + .as_ref() + .map(|st| st.trim().to_string()) + .clone(); + let help_url = fix.help_url.clone(); + let prompt = fix.prompt.map(DoctorFixPrompt::from); + + Ok(DoctorFix { + command: Some(commands), + help_text, + help_url, + prompt, + }) + } +} + #[derive(Debug, PartialEq, Clone, Builder)] #[builder(setter(into))] pub struct DoctorFixPrompt { @@ -53,4 +88,57 @@ mod tests { actual ) } + + #[test] + fn empty_returns_a_fix_full_of_none() { + // I can argue that we should use Option instead, + // but for now, this is where we're at. + assert_eq!( + DoctorFix { + command: None, + help_text: None, + help_url: None, + prompt: None, + }, + DoctorFix::empty() + ) + } + + #[test] + fn from_spec_translates_to_fix() { + let spec = DoctorFixSpec { + commands: vec![ + "some/command", + "./other_command", + "{{ working_dir }}/.foo.sh", + ] + .iter() + .map(|cmd| cmd.to_string()) + .collect(), + help_text: Some("text".to_string()), + help_url: Some("https.example.com".to_string()), + prompt: Some(DoctorFixPromptSpec { + text: "do you want to do the thing?".to_string(), + extra_context: Some("additional context".to_string()), + }), + }; + + let expected = DoctorFix { + command: Some( + DoctorCommand::from_commands( + Path::new("/some/dir"), + "/some/work/dir", + &spec.commands, + ) + .unwrap(), + ), + help_text: spec.help_text.clone(), + help_url: spec.help_url.clone(), + prompt: Some(DoctorFixPrompt::from(spec.prompt.clone().unwrap())), + }; + + let actual = DoctorFix::from_spec(Path::new("/some/dir"), "/some/work/dir", spec).unwrap(); + + assert_eq!(expected, actual) + } } diff --git a/scope/src/shared/models/internal/mod.rs b/scope/src/shared/models/internal/mod.rs index 9308402..39ed582 100644 --- a/scope/src/shared/models/internal/mod.rs +++ b/scope/src/shared/models/internal/mod.rs @@ -4,6 +4,8 @@ use crate::models::prelude::{ use crate::models::InternalScopeModel; use crate::shared::prelude::*; use anyhow::anyhow; +use anyhow::Result; +use minijinja::{context, Environment}; use path_clean::PathClean; use serde_yaml::Value; use std::collections::VecDeque; @@ -91,6 +93,15 @@ pub(crate) fn extract_command_path(parent_dir: &Path, exec: &str) -> String { .join(" ") } +pub(crate) fn substitute_templates(work_dir: &str, input_str: &str) -> Result { + let mut env = Environment::new(); + env.add_template("input_str", input_str)?; + let template = env.get_template("input_str")?; + let result = template.render(context! { working_dir => work_dir })?; + + Ok(result) +} + #[cfg(test)] mod tests { use super::*; @@ -109,4 +120,40 @@ mod tests { assert_eq!("foo", extract_command_path(base_path, "foo")); assert_eq!("foo bar", extract_command_path(base_path, "foo bar")); } + + mod substitute_templates_spec { + use super::*; + + #[test] + fn working_dir_is_subbed() { + let working_dir = "/some/path"; + let command = "{{ working_dir }}/foo.sh"; + + let actual = substitute_templates(&working_dir, &command).unwrap(); + + assert_eq!("/some/path/foo.sh".to_string(), actual) + } + + #[test] + fn text_without_templates_is_passed_through() { + let working_dir = "/some/path"; + let command = "./foo.sh"; + + let actual = substitute_templates(&working_dir, &command).unwrap(); + + assert_eq!("./foo.sh".to_string(), actual) + } + + #[test] + fn other_templates_are_erased() { + // I don't believe this is intentional behavior, + // but it is the current behavior. + let working_dir = "/some/path"; + let command = "{{ not_a_thing }}/foo.sh"; + + let actual = substitute_templates(&working_dir, &command).unwrap(); + + assert_eq!("/foo.sh".to_string(), actual) + } + } }