Added command line option to keep trailing newlines #135
Conversation
|
Coverage decreased (-0.62%) when pulling 9b85b1300615c05e165c730f569c071de2759fdf on luked:master into 9b04c23 on audreyr:master. |
ghost
mentioned this pull request
| @@ -181,7 +181,8 @@ def generate_files(repo_dir, context=None, output_dir="."): | |||
| run_hook('pre_gen_project', project_dir) | |||
|
|
|||
| with work_in(template_dir): | |||
| env = Environment() | |||
| if environment_kwargs is None: environment_kwargs = {} | |||
There was a problem hiding this comment.
Don't like this construction, there's nothing wrong with another newline.
|
Coverage increased (+0.03%) when pulling cf4f058c5651804848910fc21404920532c82937 on luked:master into 9b04c23 on audreyr:master. |
|
Yes, sorry about that. I had missed adding a docstring for the argument also. |
|
🍰 |
That made me think about whether the kwargs part was necessary. Perhaps Could ending it with kwargs also be misleading as an argument name? |
|
|
|
Coverage decreased (-1.89%) when pulling 5a2def4b05f48894a593e3bd60cf86868b9317b7 on luked:master into 9b04c23 on audreyr:master. |
Much better 😄 With the second patch that changes that changes the default, travis failed the first time. I think it was just network issue though. 😕 |
|
Cool 🍕 🍔 👍 |
|
@audreyr Any chance this can get merged? Otherwise it either has to be corrected in a hook or git will complain on any generated file. |
|
Example of a hook script to work around this issue https://github.com/poise/cookiecutter-cookbook/blob/master/hooks/post_gen_project.py |
|
We're running into some issues with source files that have trailing newlines being output without those trailing newlines. It would be nice to have this merged! :-) |
|
👍 this is a big problem for Python projects especially which very rarely lack trailing newlines at the ends of files. I actually think #79 may be a better short-term solution to this problem. I think a global switch is not granular enough. A default of adding trailing newlines is almost certainly the most sane. There are legitimate reasons to keep trailing newlines out of specific files (or add them only to specific files). It might be nice to eventually allow a flag to keep trailing newlines out of specific files. I have no specific use for such a feature, but it is conceivable that some files may disallow trailing newlines while others allow/require them. |
1 similar comment
|
@treyhunner Yeah, I don't know why a command line argument is needed, but @michaeljoseph had asked for it. I had thought that perhaps allowing the cookiecutter.json file of the individual cookiecutters to specify jinja2 options might be a better solution, as the author of a cookiecutter is probably more likely to care about this stuff than the end user. I thought I might work on a patch that would do this, but I thought I would wait and see if this problem would get resolved first :) |
|
I think we would prefer a cli switch and the current default to stay the same, so that we can preserve existing functionality / backwards compatibility? |
|
@michaeljoseph I agree with @LukeD that allowing cookiecutter authors to tweak the behavior is most desirable. As a user I don't want to remember to add a command line option to every cookiecutter I use. As a maintainer of a cookiecutter I am okay with tweaking my configuration to add trailing newlines to my files as appropriate. |
|
@treyhunner 👍 to that. |
ghost
mentioned this pull request
|
I realized that the keep_trailing_newline behavior only causes newline absence/presence to be maintained. I believe this is the expected behavior, so I created a simple pull request which I believe solves this problem in the most appropriate way: #183. Please chime in there. |
|
Closing in favour of #183 |
This adds the command line options '--keep-trailing-newline' (or '-n').
I'm not sure whether 'n' is the right letter to shorten it to and might require some thought.
I added a newline to one of the test samples. I'm not sure whether that would be okay, or whether it would be better to add a new file to check this with.