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

Make data inheritance work for multilevel file includes #9

Open
vikalp-sajwan opened this issue Apr 8, 2019 · 3 comments · May be fixed by #10
Open

Make data inheritance work for multilevel file includes #9

vikalp-sajwan opened this issue Apr 8, 2019 · 3 comments · May be fixed by #10

Comments

@vikalp-sajwan
Copy link

Hi @dmolsen, firstly thanks for your plugin.
I am using this plugin in project and facing an issue with data-inheritance. This does not work for multilevel file includes, when the sub-included files are lexicographically greater.
This could be solved using numeric prefix in filenames, but I would really like to avoid that.

For eg. let say we have three patterns in atoms directory:

  • a.twig

    This is a.twig {{ data1 }} {% include 'atoms-b' %}
  • a.json

    { "data1": "hello from A" }
  • b.twig

    This is b.twig {{ data2 }} {% include 'atoms-c' %}
  • b.json

    { "data2": "hello from B" }
  • c.twig

    This is c.twig {{ data3 }}
  • c.json

    { "data3": "hello from C" }

In this case, pattern a will inherit data for only first level include i.e for b, and not for the c. As b is processed later by the plugin.

This can be easily solved by recursively processing the patterns with lineage first, keeping track of the processed patterns at the same time.

Let me know what you think about this, I'll be happy to submit a PR for this.

@bradfrost
Copy link
Member

hey @vikalp-sajwan, thanks for this. Just a heads up @dmolsen isn't an active maintainer on this project anymore. Another heads up is that there's a chance this plugin isn't relevant anymore (@EvanLovely can you confirm?).

@vikalp-sajwan vikalp-sajwan linked a pull request May 28, 2019 that will close this issue
@vikalpsajwan
Copy link

Hi @bradfrost, thanks for your response. This plugin is very much relevant for a project I am working on.
I made the PR of the code that we are using, #10. Please, review and merge if seems relevant.

@bradfrost
Copy link
Member

Thanks @vikalp-sajwan!

@EvanLovely, would you be able to take a look at this PR?

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 a pull request may close this issue.

3 participants