Skip to content

feat: add previous responses as a default#58

Merged
Keats merged 10 commits intoKeats:masterfrom
EclesioMeloJunior:add-previous-responses-as-default
Jan 19, 2022
Merged

feat: add previous responses as a default#58
Keats merged 10 commits intoKeats:masterfrom
EclesioMeloJunior:add-previous-responses-as-default

Conversation

@EclesioMeloJunior
Copy link
Copy Markdown
Contributor

Description

Use a previous response/default value as value to another variable into toml file

Example

The following toml file describes the feature included in this PR, basically will be possible to use a variable template to get already assigned values from other variables. eg will be assigned to the manifest variable the value my_project-other_project-manifest.md if the user leave the all the inputs empty, however if the user populate the variable project_one with tree and variable project_two with leaf and leave manifest variable empty then the value assigned to manifest will be tree-leaf-manifest.md.

[[variables]]
name = "project_one"
default = "my_project"
prompt = "What's the name of your first project?"

[[variables]]
name = "project_two"
default = "other_project"
prompt = "What's the name of your second project?"

[[variables]]
name = "manifest"
default = "{{project_one}}-{{project_two}}-manifest.md"
prompt = "What's the manifest name file?"

The prompt default value is updated too!
image

Issue

#22

@@ -0,0 +1,19 @@
name = "Super basic"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default_from_variable would be a better name for the file

}

fn has_template_variables<'a>(s: &'a String, _already_vars: &HashMap<String, Value>) -> Option<HashSet<&'a str>> {
let re = Regex::new(r"\{\{(?:[a-zA-Z][0-9a-zA-Z_]*)\}\}").unwrap();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's going to compile that regex every time this function is called, you can lift it in some lazy static or once_cell, whatever this crate is using (or add one of these libs to it)

Some(variables) => replace_with_previous_responses(
variables, &vals, &s),
None => s.clone(),
};
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think doing it that way is more complicated than it actually requires.
A default value will always be a valid Tera template as it's just a string so you can just check whether {{ and }} are in the string and handle both cases:

  1. no: just use that string a default, no need to do anything
  2. yes: render that string as a one off template with the variables so far added to the Tera context.

@EclesioMeloJunior
Copy link
Copy Markdown
Contributor Author

got it! never used Tera before, I will address your comments asap!

Some(variables) => replace_with_previous_responses(
variables, &vals, &s),
None => s.clone(),
let contains_template = has_template_variables(&s);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just check if s.contains("{{") && s.contains("}}") rather than using regex

src/utils.rs Outdated
context: &Context,
path: Option<PathBuf>,
) -> Result<String> {
let mut tera = Tera::default();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's actually a better way to do that now: https://docs.rs/tera/latest/tera/struct.Tera.html#method.one_off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should we set the autoscape to true or transform it into a function parameter?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no autoescape needed there

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keats autoscape set to false!

@Keats
Copy link
Copy Markdown
Owner

Keats commented Jan 6, 2022

Thanks, I'll merge it and tweak a bit later this week

@Keats Keats merged commit df2d817 into Keats:master Jan 19, 2022
chevdor pushed a commit to chevdor/kickstart that referenced this pull request Jan 12, 2023
* feat: use previous responses as default values

* chore: split in different functions

* chore: update examples folder

* chore: include unit test

* chore: add lazy_static crate to avoid regex eval evey time

* chore: change to `default-fro-variable`

* chore: use tera and render template to replace values

* chore: remove regex and use `s.contains` instead

* chore: use `tera::one_off` function

* chore: set `autoscape` to false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants