fix: modifying start and end variable strings by sacha-c · Pull Request #1997 · cookiecutter/cookiecutter

sacha-c · 2023-12-16T14:22:26Z

Note: I recreated this closed PR #1963 as requested)

Description

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 (see here). In addition, the jinja context is not always populated with the _jinja2_env_vars, for example when getting the placeholder prompts here

Technical details

This change does 3 things (which are in separate commits for readability):

Creates a common utility function create_env_with_context so the jinja environment is created similarly and with context each time.
- Note that I also considered creating a single jinja env object & passing it around, however it requires much more refactoring (especially of tests), and we need to create it at least twice anyway (once to create the cookiecutter prompt, and once after the prompts are passed to the context), so I decided to go for the easier solution for now.
Leverages the _jinja2_env_vars to search for the initial cookiecutter directory, to allow this customization of variable_start_string and variable_end_string.
Small refactor to move prompt_and_delete from utils to prompt. It was causing circular import issues when having prompt import utils, because utils was importing prompt. And in any case I would say it makes more sense for it to live in prompt because it is a prompt as well after all.

I also removed the ensure_dir_is_templated check 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:

<MyComponent
    some-prop={{ key: 'value' }}
</MyComponent>

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 == '}%}'

kurtmckee

Thanks for your work on this, @sacha-c!

This looks really good to me. I had a couple questions, and I've left a request for one additional test.

cookiecutter/generate.py

kurtmckee · 2023-12-16T14:50:50Z

tests/test_find.py

+        ),
+    ],
+)
+def test_find_template(repo_name, env, expected):


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.

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

kurtmckee · 2023-12-16T14:51:41Z

tests/test_generate_files.py

-@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)


What is the expected behavior if the directory is not templated?

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

for more information, see https://pre-commit.ci

eric-musliner · 2024-01-19T17:42:34Z

What's the latest update on this? Would be extremely useful to have this ability

eric-musliner · 2024-01-30T15:42:05Z

What's the latest update on this? Would be extremely useful to have this ability

@ericof

sacha-c · 2024-02-19T16:25:43Z

@kurtmckee @ericof @jensens
This PR looks ready & has been reviewed, is there anything I can help with to get merged?
Thanks!

kurtmckee · 2024-02-19T16:28:52Z

@sacha-c I don't have rights to merge in this repo. A maintainer will need to merge.

eric-musliner · 2024-02-20T14:57:25Z

When can we expect a release to include this enhancement?

jensens · 2024-02-20T16:37:45Z

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).

fix: create jinja environment using context

0f556fb

sacha-c force-pushed the fix/jinja-variable-directory-name branch from 96f9bbe to 774e4f1 Compare December 16, 2023 14:27

sacha-c mentioned this pull request Dec 16, 2023

fix: modifying start and end variable strings #1963

Closed

kurtmckee requested review from ericof and kurtmckee December 16, 2023 14:37

kurtmckee reviewed Dec 16, 2023

View reviewed changes

sacha-c added 2 commits December 16, 2023 16:59

sacha-c force-pushed the fix/jinja-variable-directory-name branch from 774e4f1 to e12ac18 Compare December 16, 2023 15:59

[pre-commit.ci] auto fixes from pre-commit.com hooks

c1da1f7

for more information, see https://pre-commit.ci

kurtmckee approved these changes Dec 16, 2023

View reviewed changes

jensens added 2 commits January 23, 2024 10:29

Merge branch 'main' into fix/jinja-variable-directory-name

2abc909

Merge branch 'main' into fix/jinja-variable-directory-name

88b4e1e

jensens approved these changes Feb 19, 2024

View reviewed changes

jensens added the enhancement This issue/PR relates to a feature request. label Feb 19, 2024

jensens merged commit 04b42e1 into cookiecutter:main Feb 19, 2024

sacha-c deleted the fix/jinja-variable-directory-name branch February 20, 2024 18:50

jensens mentioned this pull request Feb 21, 2024

Bugfix/jinja2 change delimiters of the template folder #1837

Closed

rmartin16 mentioned this pull request Apr 8, 2024

Whitespace control behavior different in v2.6.0 #2023

Closed

Lindelanimcebo mentioned this pull request Nov 6, 2024

[Bug] Unable to create template if jinja block character changed cruft/cruft#294

Open

sacha-c mentioned this pull request Mar 24, 2026

Cannot change jinja2 variable_start_string and variable_end_string, NonTemplatedInputDirException #1966

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: modifying start and end variable strings#1997

fix: modifying start and end variable strings#1997
jensens merged 6 commits intocookiecutter:mainfrom
sacha-c:fix/jinja-variable-directory-name

sacha-c commented Dec 16, 2023

Uh oh!

kurtmckee left a comment

Uh oh!

Uh oh!

kurtmckee Dec 16, 2023

Uh oh!

sacha-c Dec 16, 2023

Uh oh!

kurtmckee Dec 16, 2023

Uh oh!

sacha-c Dec 16, 2023

Uh oh!

eric-musliner commented Jan 19, 2024

Uh oh!

eric-musliner commented Jan 30, 2024

Uh oh!

sacha-c commented Feb 19, 2024

Uh oh!

kurtmckee commented Feb 19, 2024

Uh oh!

eric-musliner commented Feb 20, 2024

Uh oh!

jensens commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

sacha-c commented Dec 16, 2023

Uh oh!

kurtmckee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kurtmckee Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

sacha-c Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

kurtmckee Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

sacha-c Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

eric-musliner commented Jan 19, 2024

Uh oh!

eric-musliner commented Jan 30, 2024

Uh oh!

sacha-c commented Feb 19, 2024

Uh oh!

kurtmckee commented Feb 19, 2024

Uh oh!

eric-musliner commented Feb 20, 2024

Uh oh!

jensens commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants