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

Big refactoring around cutter API #15

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

benoitbryon
Copy link
Member

Not ready to merge yet!
But you may start review with examples in (updated) docs (starting with overview).

Many changes around piecutter's API.

@Natim
Copy link
Contributor

Natim commented Dec 22, 2014

Keep going 👍 That's great!

@benoitbryon
Copy link
Member Author

Keeping working at it... Check doc about generating directories.

@Natim
Copy link
Contributor

Natim commented May 16, 2016

I tried the documentation and I have some remarks:

  • As for json.dump and json.dumps I think we should have render and renders()
  • In the quickstart we should pass alternative writer to piecutter.Cutter() rather than modifying the underlying object.

I would also use the word engine rather than render and not generate a callable but a real engine:

>>> import tempfile
>>> output_directory = tempfile.mktempd()
>>> renderer = piecutter.Cutter(writer=piecutter.FileWriter(target=output_directory,
                                engine=piecutter.Jinja2Engine(),
                                loader=piecutter.FileDirectory())
>>> renderer.render('/home/git/cookiecutter-pypackage', {"name": "diecutter"})

@benoitbryon
Copy link
Member Author

As for json.dump and json.dumps I think we should have render and renders()

Do you mean renders() would be a shortcut for some render(writer=TextWriter())?

In the quickstart we should pass alternative writer to piecutter.Cutter() rather than modifying the underlying object.

Ok.

I would also use the word engine rather than render and not generate a callable but a real engine:

At the moment, cutters and engines are not the same! They have different goals and different API:

  • cutters are high-level. Cutters primarily expect location (i.e. "file://...") as input. They support raw text, file objects or template objects for convenience. Cutters support directories (because they manage loaders).
  • engines are low-level. Engines primarily expect template object as input. They support raw text and file objects for convenience. Engines support only files (because they don't know about loaders).

I first tried to make Cutter and Engine use the same API, but it introduces complexity and confusion with loaders. Making them distinct simplified implementation of each.

@benoitbryon
Copy link
Member Author

In the quickstart we should pass alternative writer to piecutter.Cutter() rather than modifying the underlying object.

Done.

@benoitbryon
Copy link
Member Author

While rendering dynamic directories (i.e. using special .directory-tree template) we can include files and directories from third-party locations. See http://piecutter.readthedocs.io/en/cutter-api-reloaded/dynamic-trees.html#include-locations

Sample use case: in some "Django project" template, include "setup.py" template located in third-party "Python project" template.

@benoitbryon
Copy link
Member Author

Looks like refactored API is getting better.
What's missing for this branch to be merged in master?

Other ideas?

Once this pull-request is merged, we could work on further improvements:

  • Python 3 support
  • writers to generate archives such as ZIP or TAR.GZ
  • user-friendly documentation: more examples and stories?
  • diecutter's upgrade at last!

@Natim
Copy link
Contributor

Natim commented Jul 1, 2016

Looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants