Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added: Path templates will be rendered when copy_without_render used #839

Merged
merged 1 commit into from May 29, 2020

Conversation

noirbizarre
Copy link
Contributor

@noirbizarre noirbizarre commented Oct 23, 2016

This pull-request ensures paths variables are rendered when using _copy_without_render.

It should fix #456.

It does not keep the existing behavior as it was inconsistent, different for files and directories (matched files had their path rendered but not the matched directories).
If this behavior should be kept, I'll update this pull-request with a boolean option like _copy_without_render_paths.

@noirbizarre
Copy link
Contributor Author

Any chance to see that one merged ? It would be great !!!
Should I change something ? ...
Any comment, any review, I'll update so it can be merge 👍

@noirbizarre
Copy link
Contributor Author

Hi !
@pydanny @audreyr @michaeljoseph @pfmoore @hackebrot 👋
I never had any feedback on this. As this a this a really simple fix to a really annoying issue (to me), I just want to know:

  • is there something I need to change ?
  • will this be merged ?
  • If not, why ?

Any review, any comment, I'll make the changes ASAP !

@theodesp
Copy link

theodesp commented Aug 8, 2017

I think it looks good @noirbizarre

Copy link
Contributor

@michaeljoseph michaeljoseph left a comment

Choose a reason for hiding this comment

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

Yep, nice fix 😸

@michaeljoseph
Copy link
Contributor

@noirbizarre could we please have a small update to the docs to include a note about the path vs file content behaviour of this functionality.
Perhaps with an explanatory example?

@noirbizarre
Copy link
Contributor Author

noirbizarre commented Sep 22, 2017

Done.
@michaeljoseph @theodesp Sorry for the late response, I just noticed yours

Copy link
Member

@hackebrot hackebrot left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you 🙇

Minor docs fix.

docs/advanced/copy_without_render.rst Outdated Show resolved Hide resolved
@hackebrot
Copy link
Member

I wonder if we should treat this as a breaking change... 🤔

@audreyr @pydanny @michaeljoseph

@noirbizarre
Copy link
Contributor Author

I fixed the typo.

My opinion: I don't think this should be considered as a breaking change but as a fix because the previous behavior was inconsistent (different behavior for files and directories) and unless you are making a meta cookiecutter (ie. a cookiecutter template rendering cookiecutter templates), I don't think there is any reason to render brackets into path (which was the behavior for directories before this fix because path variables were not rendered).

@noirbizarre
Copy link
Contributor Author

@audreyr @pydanny @michaeljoseph @theodesp @hackebrot
Could this be merged before the 1.7 is released ?

@insspb insspb added this to the 2.0.0 milestone May 28, 2020
@insspb insspb added breaking-change Marks an important and likely breaking change. Require update for major version feature This issue/PR relates to major feature request. labels May 28, 2020
@insspb insspb self-assigned this May 28, 2020
@cookiecutter cookiecutter deleted a comment from codecov-io May 28, 2020
@insspb insspb force-pushed the render_copy_without_render branch from d6df586 to 840844d Compare May 28, 2020 23:58
@insspb insspb changed the title Render path templates in copy_without_render Added: Path templates will be rendered when copy_without_render used May 29, 2020
@insspb insspb merged commit d7e7b28 into cookiecutter:master May 29, 2020
@insspb
Copy link
Member

insspb commented May 29, 2020

@noirbizarre Thank you for contribution. Nothing lost. Merged after 4 year in queue :)

@noirbizarre noirbizarre deleted the render_copy_without_render branch June 22, 2020 23:41
@simobasso simobasso mentioned this pull request May 15, 2021
@newlog
Copy link

newlog commented Jul 4, 2021

Thanks for this change.

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 feature This issue/PR relates to major feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for copy_without_render to render output directory name
6 participants