Extra context: API + CLI #260
Conversation
Add extra_context parameter to `cookiecutter.main.cookiecutter` and `cookiecutter.generate.generate_context`
Refactor file io to utils
Add and use CLI --parameters option
Fix context naming
Document context and overrides
Escape string interpolation character
| @@ -45,17 +45,6 @@ def test_make_sure_path_exists(self): | |||
| utils.rmtree('tests/blah/') | |||
| utils.rmtree('tests/trailingslash/') | |||
|
|
|||
| def test_unicode_open(self): | |||
There was a problem hiding this comment.
Is this a deliberate removal? I removed this when I switched from unicode_open to io.open but @dannyr asked me to put it back (to make sure that we read Unicode files correctly, however we did it).
There was a problem hiding this comment.
Yes, I don't think we need to test stdlib. Right @pydanny ?
There was a problem hiding this comment.
I insisted on keeping the test before because I wanted to be certain that the behavior of cookiecutter remained the same. So it wasn't a test of stdlib but rather the potential difference between stdlib and unicode_open.
Now that 0.7.2 is gone this test can be removed. However, I submit that it's removal doesn't belong in this pull request. Rather it's own pull request.
There was a problem hiding this comment.
Agree, I'll revert.
|
@pydanny @audreyr I don't think there's a conflict as such with the new config format. What I see happening (see my comments in #249) is that we end up with a new external format that we parse into our internal format. The current "context" will become a subkey of the new internal dictionary format, but can otherwise remain the same. So the impact from this change is minimal (although as both changes touch the same code, there will be merge conflicts to resolve, but that's not a big deal). What will be impacted is the various prompt improvements covered under https://github.com/audreyr/cookiecutter/milestones/0.8.x:%20Prompt%20and%20CLI%20Improvements (is there a better way to link to a milestone???) - these generally propose modifying the context dict to add extra information. But that can't be done without some work anyway, as the context is passed straight to Jinja, so we need to maintain both forms somehow. I'll try to get a PR for #249 up today, that shows how I'd see this working. (Coding it's easy, it's writing the tests that will take the time ;-)) |
| return dict( | ||
| [override.split('=') for override in args.override] | ||
| ) | ||
| return {} |
There was a problem hiding this comment.
I'm nervous about adding this, because it's a custom parser and a bit more brittle than I'd like. The syntax would make it hard to provide nested multi-level overrides, and I suspect that there are some missing edge cases here. Can you take this out so that overrides are only provided via the API at least for now?
|
@michaeljoseph huge thanks again for all your work. Seriously, really appreciate this. I must confess that I'm having a hard time following what's going on, because the scope of this pull request goes far beyond extra context. I'm a bit finicky about what goes into the codebase, so I'd prefer not to bring in changes until I fully understand them. What I'm going to do is start cherry-picking the pieces that I know are extra-context-related, and merge them in one at a time. Then I'll cherry-pick the cleanup improvements commit by commit, as I figure out how each one works, as well as the implications of each. No need to respond to the additional comments -- you've worked so hard on this and been so patient, so let me take it from here. |
| no_input=True, | ||
| overrides={'repo_name': 'bar'} | ||
| ) | ||
| self.assertTrue(os.path.isdir('bar')) |
There was a problem hiding this comment.
I added this test in 3fcf3bc (some slight but important differences, see diff)
|
I've decided to limit the scope of Extra Context in 0.8.0 to just the |
|
OK. This PR did not automatically close, but it's merged into master now. Note: this was a bit tricky. Not all of it was merged in: I cherry-picked the parts of this that I felt comfortable with as per my comments, and then I added additional docs and tests. To better understand, go through the diffs one-by-one and read the comments in there, and see the final diff in #274. |
Fixes #12
Synthesizes parts of: #75, #100, #116, #161 and #199.
Thanks @fcurella, @aventurella, @emonty, @schacki, @ryanolson!