Skip to content

Bugfix/jinja2 change delimiters of the template folder#1837

Closed
mariovagomarzal wants to merge 8 commits intocookiecutter:mainfrom
mariovagomarzal:fix-jinja2-delimiters
Closed

Bugfix/jinja2 change delimiters of the template folder#1837
mariovagomarzal wants to merge 8 commits intocookiecutter:mainfrom
mariovagomarzal:fix-jinja2-delimiters

Conversation

@mariovagomarzal
Copy link
Copy Markdown

Addresses #1736.

Based on the bugfix by @liortct in #1767 but using the organization name throughout the project.

Copy link
Copy Markdown
Member

@kurtmckee kurtmckee left a comment

Choose a reason for hiding this comment

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

@mariovagomarzal Thanks for re-triggering CI after the lint-fixing PR was merged, and for adding a test to demonstrate this bug is fixed!

@audreyfeldroy @jensens @tmeckel Please merge this PR to close #1736. Thanks!

Comment on lines +25 to +26
and environment.variable_start_string in str_path
and environment.variable_end_string in str_path
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This, and the similar change in generate.py, are the crucial elements of this bugfix. Nice!

@jensens
Copy link
Copy Markdown
Contributor

jensens commented Jun 12, 2023

This changes the signature of the functions -> breaking change.

@jensens jensens added the breaking-change Marks an important and likely breaking change. Require update for major version label Jun 12, 2023
@ericof ericof added this to the 3.0.0 milestone Jun 13, 2023
@ericof
Copy link
Copy Markdown
Member

ericof commented Jun 13, 2023

As this is a breaking change, we will add it to the 3.0.0 milestone

@kurtmckee
Copy link
Copy Markdown
Member

Would it be sufficient to give the environment parameter a default of None and then conditionally use its settings? For example:

def find(x, environment = None):
    left_delimiter = "{{"
    right_delimiter = "}}"
    if environment is not None:
        left_delimiter = environment.x
        right_delimiter = environment.y
    ...

Would that allow this to get merged immediately as a bugfix instead of a breaking change?

@jensens
Copy link
Copy Markdown
Contributor

jensens commented Feb 21, 2024

Was this addressed with merge of #1997 ?

@mariovagomarzal
Copy link
Copy Markdown
Author

Yes, I think we can close this pull request.

Sorry for the late response.

@mariovagomarzal mariovagomarzal deleted the fix-jinja2-delimiters branch March 21, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Marks an important and likely breaking change. Require update for major version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants