fix: modifying start and end variable strings #1997
Conversation
96f9bbe to
774e4f1
Compare
tests/test_find.py
Outdated
| ), | ||
| ], | ||
| ) | ||
| def test_find_template(repo_name, env, expected): |
There was a problem hiding this comment.
Would you add a negative find() test? I'd like to see demonstrated behavior when the environment variables and the directory name don't align, like "Jinja vars are {%{ and }%} but the directory is {{ repo }}".
I'm not advocating that {{ / }} is a fallback, just wanting a negative test to demonstrate the expected failure case.
There was a problem hiding this comment.
I've added a parametrized test here: template with custom jinja strings but folder with default jinja strings
where the jinja configuration is overriden with {%{, but the template folder is uses {{
In this case cookiecutter should not be able to find the template folder because it is not accurately named, so it should fail with NonTemplatedInputDirException
| @pytest.mark.parametrize('invalid_dirname', ['', '{foo}', '{{foo', 'bar}}']) | ||
| def test_ensure_dir_is_templated_raises(invalid_dirname): | ||
| """Verify `ensure_dir_is_templated` raises on wrong directories names input.""" | ||
| with pytest.raises(exceptions.NonTemplatedInputDirException): | ||
| generate.ensure_dir_is_templated(invalid_dirname) |
There was a problem hiding this comment.
What is the expected behavior if the directory is not templated?
There was a problem hiding this comment.
True, I removed this function because I felt it was redundant with the way the folder was found in the first place; both were using the same logic & failing the same way
I had deleted this test but did not add a new test case to exemplify this scenario, this is now fixed 🙂
I've added an additional test case in test_find_template::template missing folder
The error thrown will be NonTemplatedInputDirException
Cookiecutter supports using _jinja2_env_vars to modify the jinja environment. However when modifying variable_start_string or variable_end_string it will not work because we are hard-coding the start and end strings for the main directory creation. This change leverages the _jinja2_env_vars to search for the main directory, to allow this customization of variable_start_string and variable_end_string. I also remove the check because the same logic is used to find the dir, which is therefore redundant.
prompt_and_delete was a function in utils which also made use of read_user_yes_no which was in prompt. This means that prompt could not import utils because it would create a circular import. This change moves prompt_and_delete to prompt, so that prompt may make use of util functions
774e4f1 to
e12ac18
Compare
for more information, see https://pre-commit.ci
|
What's the latest update on this? Would be extremely useful to have this ability |
|
|
@kurtmckee @ericof @jensens |
|
@sacha-c I don't have rights to merge in this repo. A maintainer will need to merge. |
|
When can we expect a release to include this enhancement? |
|
I plan to look first at some other PR's, but expect it within next 2-3 weeks. |
On older versions of cookiecutter (pre cookiecutter/cookiecutter#1997) the find.find_template function took one parameter but after that PR it now takes two (in an apparent breaking change).
Description
Cookiecutter supports using _jinja2_env_vars to modify the jinja environment. However when modifying
variable_start_stringorvariable_end_stringit will not work because we are hard-coding the start and end strings for the main directory creation (see here). In addition, the jinja context is not always populated with the _jinja2_env_vars, for example when getting the placeholder prompts hereTechnical details
This change does 3 things (which are in separate commits for readability):
create_env_with_contextso the jinja environment is created similarly and with context each time.prompt_and_deletefromutilstoprompt. It was causing circular import issues when havingpromptimportutils, becauseutilswas importingprompt. And in any case I would say it makes more sense for it to live inpromptbecause it is a prompt as well after all.I also removed the
ensure_dir_is_templatedcheck because the same logic is used to find the dir, and is therefore redundant.Use-case
In react-native projects, it's extremely common to use
{{and}}to pass objects to components. For example:Since
{{is the default jinja variable string prefix, jinja goes crazy with react-native templates 🤪 .A solution to this is to set
_jinja2_env_vars.variable_start_string == '{%{'(or something else). But cookiecutter currently has issues when overriding this setting.Testing
Tested locally on a react-native project with
_jinja2_env_vars.variable_start_string == '{%{'and_jinja2_env_vars.variable_end_string == '}%}'