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

implementation/tests of the template inheritance mechanism #336

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

Conversation

did
Copy link

@did did commented Apr 8, 2014

hi @fw42 & @tylerball,
Based on the conversation here (#264), this is my implementation of template inheritance, similar to what offers Django (https://docs.djangoproject.com/en/dev/topics/templates/).
Code mainly extracted from the LocomotiveCMS fork of Liquid.
Feel free to comment and ask me questions.
Thanks!

@fw42
Copy link
Contributor

fw42 commented Apr 9, 2014

cc @Shopify/liquid


end

class InheritedBlockDrop < Drop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this into it's own file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but where? Any thoughts?

@fw42
Copy link
Contributor

fw42 commented Apr 9, 2014

@tylerball, can you check it out and see if this is close to what you had in mind?

@did
Copy link
Author

did commented Apr 9, 2014

hi @fw42, I pushed a new version of my PR. It reflects the modifications you brought up in your comments. Let me know if you have other comments / questions.
Thanks!

# {% block content }Hello world{% endblock %}
#
class Extends < Block
Syntax = /(#{QuotedFragment}+)/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs memoization

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @tobi! do you mean for the overall implementation or in some specific methods?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the regexp. It's done by adding /o to it.

http://stackoverflow.com/questions/13334807/what-does-the-o-modifier-for-a-regexp-mean

  • tobi
    CEO Shopify

On Wed, Apr 9, 2014 at 10:00 AM, Didier Lafforgue
notifications@github.comwrote:

In lib/liquid/tags/extends.rb:

@@ -0,0 +1,76 @@
+module Liquid
+

  • Extends allows designer to use template inheritance

  • {% extends home %}

  • {% block content }Hello world{% endblock %}

  • class Extends < Block
  • Syntax = /(#{QuotedFragment}+)/

hi @tobi https://github.com/tobi! do you mean for the overall
implementation or in some specific methods?


Reply to this email directly or view it on GitHubhttps://github.com//pull/336/files#r11436430
.

@tobi
Copy link
Member

tobi commented Apr 9, 2014

This is a large departure from the current way to use liquid. It's implemented well but it will divide liquid users into two camps: The more traditional composition and the Django style block approach. Should this perhaps be an addon gem?

@did
Copy link
Author

did commented Apr 9, 2014

I understand your concern about this feature, knowing that this might have "consequences" for your Shopify customers.
That said, nobody is forced to use the inheritance template. Going further, making it as an add-on would be a good compromise. Would it be the first add-on for Liquid?

@did
Copy link
Author

did commented Apr 10, 2014

@tobi, is that possible this add-on becomes an "official" gem in a sense its sources would be stored in your github organization so that people can contribute / maintain it in more official way than it's actually. Because I've got this code for a couple of years in my forked version of Liquid but it was kind of hidden finally.
What are your thoughts?

# {% block content }Hello world{% endblock %}
#
class InheritedBlock < Block
Syntax = /(#{QuotedFragment}+)/o
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be SYNTAX, why do we use that style everywhere?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea :-) I just followed the pattern of the other existing tags.

@fw42 fw42 added the Feature label Jul 23, 2014
@did
Copy link
Author

did commented Jan 18, 2015

btw @fw42 & @tylerball, it works now with liquid 3.0.

@jeroenvisser101
Copy link
Contributor

jeroenvisser101 commented Jun 9, 2016

@did I've tried to port changes from 7e4eacd to the gem, but it doesn't seem to work. Instead, the tests throw Liquid::SyntaxError: Liquid syntax error: 'extends' tag was never closed, I assume it expects a {% endextends %} (which shouldn't be required). Any idea how to proceed?

@did
Copy link
Author

did commented Jun 9, 2016

hey @jeroenvisser101, I think they changed a couple of stuff in their API.
The liquid version we're using for locomotivecms (http://www.locomotivecms.com) can be found here: https://github.com/locomotivecms/liquid. The last commit from the original master branch was from Jan 24, 2015.
Hope it helps...

@jeroenvisser101
Copy link
Contributor

@did thanks! I'll have a look at that for sure. My best outcome would be to get it working with the 'original' version of liquid, and to get it working with the master branch (there's some things that changed there, options doesn't seem to be writable anymore, but I'll have to investigate more)

I'll let you know when I've got something solid. I don't think it'll be able to be merged into locomotivecms's fork, since it might not work with the liquid fork, but let's have a look when/if I get this to work.

@maogongzi
Copy link

Do we have a plan to merge it at some point? Since it's coming to the end of 2021 and roughly 7 years passed, and this feature really helps to facilitate the works to maintain a bunch of templates that people don't need to copy and paste again and again those same snippets everywhere :)

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.

None yet

7 participants