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

[Feature] Inline Includes #15

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

Conversation

d-simon
Copy link

@d-simon d-simon commented Sep 1, 2015

I needed this to prepend an include when generating the styleguide over here.
For this I integrated inline includes as implemented in a different gulp-twig implementation.

A noteworthy change is line 78 in index.js. Instead of passing a file path, the plugin now passes the file.contents as string to Twig. This was necessary for my use case. Is there any downside to this?

Secondly, I introduced three dependenies (lodash.merge, lodash.isarray and glob).

And lastly, I have not written any tests for this. Though the current ones are passing.

What are your thoughts on this? :-)

Important: Not ready to merge!
To make this work, I had to disable the cache of Twig, which has a very subtle bug with the id of templates (I will open a PR and cross-reference it here).
=> Before anything the bugfix will first have to be approved and make it into a release of Twig.js.

@olets
Copy link
Collaborator

olets commented May 10, 2017

I notice you're working off a significantly outdated version of twig.js. Was there a reason you chose backflip's fork? At the time of his work, the only difference was a small change to ID validation. Have you tried using gulp-twig with the latest twig.js, for example via

https://github.com/olets/gulp-twig#update-dependencies

? (see also #32)

@olets olets added the stale label Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants