Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/liquid.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ module Liquid
ArgumentSeparator = ','
FilterArgumentSeparator = ':'
VariableAttributeSeparator = '.'
WhiteSpaceControl = /-/
WhiteSpace = /[ \t]*\r?\n?[ \t]*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This name is too generic when it is for a very specific purpose.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the newline should be optional. For instance, in the strip_newlines filter we use /\r?\n/.

Also, what is the advantage of having it defined like this rather than just stripping all space characters preceding or following a tag using /\s*/ like with go templates?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not against a full trim. If anything the more it cleans up the whitespace the better. How useful that is i'm not sure. Maybe if your designing a template and you are using empty lines to break apart different blocks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not saying a full trim is more useful. It is just simpler. If it isn't useful to have it stop stripping whitespace on the second newline preceding or following a tag, then why don't we just use lstrip and rstrip which are easier to document and understand.

TagStart = /\{\%/
TagEnd = /\%\}/
VariableSignature = /\(?[\w\-\.\[\]]\)?/
Expand All @@ -39,7 +41,7 @@ module Liquid
SpacelessFilter = /^(?:'[^']+'|"[^"]+"|[^'"])*#{FilterSeparator}(?:#{StrictQuotedFragment})(?:#{FirstFilterArgument}(?:#{OtherFilterArgument})*)?/o
Expression = /(?:#{QuotedFragment}(?:#{SpacelessFilter})*)/o
TagAttributes = /(\w+)\s*\:\s*(#{QuotedFragment})/o
AnyStartingTag = /\{\{|\{\%/
AnyStartingTag = /#{TagStart}|#{VariableStart}/o
PartialTemplateParser = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/o
TemplateParser = /(#{PartialTemplateParser}|#{AnyStartingTag})/o
VariableParser = /\[[^\]]+\]|#{VariableSegment}+\??/o
Expand Down
40 changes: 36 additions & 4 deletions lib/liquid/block.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,38 @@
module Liquid
class Block < Tag
IsTag = /^#{TagStart}/o
IsVariable = /^#{VariableStart}/o
FullToken = /^#{TagStart}\s*(\w+)\s*(.*)?#{TagEnd}$/o
ContentOfVariable = /^#{VariableStart}(.*)#{VariableEnd}$/o
IsTag = /^#{TagStart}/o
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You seem to be working off a really old commit. See how different the file is now: https://github.com/Shopify/liquid/blob/master/lib/liquid/block.rb

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That explains the merge conflict message. Updating my fork now as your right its from 2013.

IsVariable = /^#{VariableStart}/o
FullToken = /^#{TagStart}#{WhiteSpaceControl}?\s*(\w+)\s*(.*)?#{WhiteSpaceControl}?#{TagEnd}$/o
ContentOfVariable = /^#{VariableStart}#{WhiteSpaceControl}?(.*)#{WhiteSpaceControl}?#{VariableEnd}$/o
IsWhiteSpaceTagStart = /^#{TagStart}#{WhiteSpaceControl}/o
IsWhiteSpaceTagEnd = /#{WhiteSpaceControl}#{TagEnd}$/o
IsWhiteSpaceVariableStart = /^#{VariableStart}#{WhiteSpaceControl}/o
IsWhiteSpaceVariableEnd = /#{WhiteSpaceControl}#{VariableEnd}$/o
WhiteSpaceStart = /\A#{WhiteSpace}/o
WhiteSpaceEnd = /#{WhiteSpace}\z/o


def blank?
@blank || false
end

def whitespace?
@@whitespace ||= false
end

def parse(tokens)
@blank = true
@nodelist ||= []
@nodelist.clear
@@whitespace ||= false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please don't maintain parse state in class variables.


while token = tokens.shift
case token
when IsTag
if token =~ IsWhiteSpaceTagStart
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using a regex to check a single byte seems like overkill if you are worried about performance. E.g. s.getbyte(2) == TRIM_CHAR where TRIM_CHAR = '-'.ord

whitespace_lookback
end
@@whitespace = (token =~ IsWhiteSpaceTagEnd)
if token =~ FullToken

# if we found the proper block delimiter just end parsing here and let the outer block
Expand All @@ -40,11 +56,19 @@ def parse(tokens)
raise SyntaxError, "Tag '#{token}' was not properly terminated with regexp: #{TagEnd.inspect} "
end
when IsVariable
if token =~ IsWhiteSpaceVariableStart
whitespace_lookback
end
@@whitespace = (token =~ IsWhiteSpaceVariableEnd)
@nodelist << create_variable(token)
@blank = false
when ''
# pass
else
if whitespace?
token.gsub!(WhiteSpaceStart, '')
end
@@whitespace = false
@nodelist << token
@blank &&= (token =~ /\A\s*\z/)
end
Expand All @@ -59,6 +83,14 @@ def parse(tokens)
def end_tag
end

def whitespace_lookback
previous_token = @nodelist.pop
if previous_token.is_a? String
previous_token.gsub!(WhiteSpaceEnd, '')
@nodelist << previous_token
end
end

def unknown_tag(tag, params, tokens)
case tag
when 'else'
Expand Down
1 change: 1 addition & 0 deletions lib/liquid/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def render!(*args)
def tokenize(source)
source = source.source if source.respond_to?(:source)
return [] if source.to_s.empty?

tokens = source.split(TemplateParser)

# removes the rogue empty element at the beginning of the array
Expand Down
74 changes: 74 additions & 0 deletions performance/tests/dropify-whitespace/article.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<div class="article">
<h2 class="article-title">{{- article.title -}}</h2>
<p class="article-details">posted <span class="article-time">{{- article.created_at | date: "%Y %h" -}}</span> by <span class="article-author">{{- article.author -}}</span></p>

<div class="article-body textile">
{{- article.content -}}
</div>

</div>

<!-- Comments -->
{%- if blog.comments_enabled? -%}
<div id="comments">
<h3>Comments</h3>

<!-- List all comments -->
<ul id="comment-list">
{%- for comment in article.comments -%}
<li>
<div class="comment-details">
<span class="comment-author">{{- comment.author -}}</span> said on <span class="comment-date">{{- comment.created_at | date: "%B %d, %Y" -}}</span>:
</div>

<div class="comment">
{{- comment.content -}}
</div>
</li>
{%- endfor -%}
</ul>

<!-- Comment Form -->
<div id="comment-form">
{%- form article -%}
<h3>Leave a comment</h3>

<!-- Check if a comment has been submitted in the last request, and if yes display an appropriate message -->
{%- if form.posted_successfully? -%}
{%- if blog.moderated? -%}
<div class="notice">
Successfully posted your comment.<br />
It will have to be approved by the blog owner first before showing up.
</div>
{%- else -%}
<div class="notice">Successfully posted your comment.</div>
{%- endif -%}
{%- endif -%}

{%- if form.errors -%}
<div class="notice error">Not all the fields have been filled out correctly!</div>
{%- endif -%}

<dl>
<dt class="{%- if form.errors contains 'author' -%}error{%- endif -%}"><label for="comment_author">Your name</label></dt>
<dd><input type="text" id="comment_author" name="comment[author]" size="40" value="{{-form.author-}}" class="{%- if form.errors contains 'author' -%}input-error{%- endif -%}" /></dd>

<dt class="{%- if form.errors contains 'email' -%}error{%- endif -%}"><label for="comment_email">Your email</label></dt>
<dd><input type="text" id="comment_email" name="comment[email]" size="40" value="{{-form.email-}}" class="{%- if form.errors contains 'email' -%}input-error{%- endif -%}" /></dd>

<dt class="{%- if form.errors contains 'body' -%}error{%- endif -%}"><label for="comment_body">Your comment</label></dt>
<dd><textarea id="comment_body" name="comment[body]" cols="40" rows="5" class="{%- if form.errors contains 'body' -%}input-error{%- endif -%}">{{-form.body-}}</textarea></dd>
</dl>

{%- if blog.moderated? -%}
<p class="hint">comments have to be approved before showing up</p>
{%- endif -%}

<input type="submit" value="Post comment" id="comment-submit" />
{%- endform -%}
</div>
<!-- END Comment Form -->

</div>
{%- endif -%}
<!-- END Comments -->
33 changes: 33 additions & 0 deletions performance/tests/dropify-whitespace/blog.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<div id="page">
<h2>{{-page.title-}}</h2>

{%- paginate blog.articles by 20 -%}

{%- for article in blog.articles -%}

<div class="article">
<div class="headline">
<h3 class="title">
<a href="{{-article.url-}}">{{- article.title -}}</a>
</h3>
<h4 class="date">Posted on {{- article.created_at | date: "%B %d, '%y" -}} by {{- article.author -}}.</h4>
</div>

<div class="article-body textile">
{{- article.content | strip_html | truncate: 250 -}}
</div>

{%- if blog.comments_enabled? -%}
<p style="text-align: right"><a href="{{-article.url-}}#comments">{{- article.comments_count -}} comments</a></p>
{%- endif -%}
</div>

{%- endfor -%}

<div id="pagination">
{{- paginate | default_pagination -}}
</div>

{%- endpaginate -%}

</div>
66 changes: 66 additions & 0 deletions performance/tests/dropify-whitespace/cart.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<script type="text/javascript">
function remove_item(id) {
document.getElementById('updates_'+id).value = 0;
document.getElementById('cartform').submit();
}
</script>

<div>

{%- if cart.item_count == 0 -%}
<h4>Your shopping cart is looking rather empty...</h4>
{%- else -%}
<form action="/cart" method="post" id="cartform">

<div id="cart">

<h3>You have {{- cart.item_count -}} {{- cart.item_count | pluralize: 'product', 'products' -}} in here!</h3>

<ul id="line-items">
{%- for item in cart.items -%}
<li id="item-{{-item.id-}}" class="clearfix">
<div class="thumb">
<div class="prodimage">
<a href="{{-item.product.url-}}" title="View {{-item.title-}} Page"><img src="{{-item.product.featured_image | product_img_url: 'thumb' -}}" alt="{{-item.title | escape -}}" /></a>
</div></div>
<h3 style="padding-right: 150px">
<a href="{{-item.product.url-}}" title="View {{-item.title | escape -}} Page">
{{- item.title -}}
{%- if item.variant.available == true -%}
({{-item.variant.title-}})
{%- endif -%}
</a>
</h3>
<small class="itemcost">Costs {{- item.price | money -}} each, <span class="money">{{-item.line_price | money -}}</span> total.</small>
<p class="right">
<label for="updates">How many? </label>
<input type="text" size="4" name="updates[{{-item.variant.id-}}]" id="updates_{{-item.variant.id-}}" value="{{-item.quantity-}}" onfocus="this.select();"/><br />
<a href="#" onclick="remove_item({{-item.variant.id-}}); return false;" class="remove"><img style="padding:15px 0 0 0;margin:0;" src="{{- 'delete.gif' | asset_url -}}" /></a>
</p>
</li>
{%- endfor -%}
<li id="total">
<input type="image" id="update-cart" name="update" value="Update My Cart" src="{{- 'update.gif' | asset_url -}}" />
Subtotal:
<span class="money">{{- cart.total_price | money_with_currency -}}</span>
</li>
</ul>

</div>

<div class="info">
<input type="image" value="Checkout!" name="checkout" src="{{- 'checkout.gif' | asset_url -}}" />
</div>

{%- if additional_checkout_buttons -%}
<div class="additional-checkout-buttons">
<p>- or -</p>
{{- content_for_additional_checkout_buttons -}}
</div>
{%- endif -%}

</form>

{%- endif -%}

</div>
22 changes: 22 additions & 0 deletions performance/tests/dropify-whitespace/collection.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{%- paginate collection.products by 20 -%}

<ul id="product-collection">
{%- for product in collection.products -%}
<li class="singleproduct clearfix">
<div class="small">
<div class="prodimage"><a href="{{-product.url-}}"><img src="{{- product.featured_image | product_img_url: 'small' -}}" /></a></div>
</div>
<div class="description">
<h3><a href="{{-product.url-}}">{{-product.title-}}</a></h3>
<p>{{- product.description | strip_html | truncatewords: 35 -}}</p>
<p class="money">{{- product.price_min | money -}}{%- if product.price_varies -%} - {{- product.price_max | money -}}{%- endif -%}</p>
</div>
</li>
{%- endfor -%}
</ul>

<div id="pagination">
{{- paginate | default_pagination -}}
</div>

{%- endpaginate -%}
47 changes: 47 additions & 0 deletions performance/tests/dropify-whitespace/index.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<div id="frontproducts"><div id="frontproducts-top"><div id="frontproducts-bottom">
<h2 style="display: none;">Featured Items</h2>
{%- for product in collections.frontpage.products limit:1 offset:0 -%}
<div class="productmain">
<a href="{{- product.url -}}"><img src="{{- product.featured_image | product_img_url: 'small' -}}" alt="{{- product.title | escape -}}" /></a>
<h3><a href="{{- product.url -}}">{{- product.title -}}</a></h3>
<div class="description">{{- product.description | strip_html | truncatewords: 18 -}}</div>
<p class="money">{{- product.price_min | money -}}</p>
</div>
{%- endfor -%}
{%- for product in collections.frontpage.products offset:1 -%}
<div class="product">
<a href="{{- product.url -}}"><img src="{{- product.featured_image | product_img_url: 'thumb' -}}" alt="{{- product.title | escape -}}" /></a>
<h3><a href="{{- product.url -}}">{{- product.title -}}</a></h3>
<p class="money">{{- product.price_min | money -}}</p>
</div>
{%- endfor -%}
</div></div></div>

<div id="mainarticle">
{%- assign article = pages.frontpage -%}

{%- if article.content != "" -%}
<h2>{{- article.title -}}</h2>
<div class="article-body textile">
{{- article.content -}}
</div>
{%- else -%}
<div class="article-body textile">
In <em>Admin &gt; Blogs &amp; Pages</em>, create a page with the handle <strong><code>frontpage</code></strong> and it will show up here.<br />
{{- "Learn more about handles" | link_to "http://wiki.shopify.com/Handle" -}}
</div>
{%- endif -%}

</div>
<br style="clear: both;" />
<div id="articles">
{%- for article in blogs.news.articles offset:1 -%}
<div class="article">
<h2>{{- article.title -}}</h2>
<div class="article-body textile">
{{- article.content -}}
</div>
</div>
{%- endfor -%}
</div>

8 changes: 8 additions & 0 deletions performance/tests/dropify-whitespace/page.liquid
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<div id="page">
<h2>{{-page.title-}}</h2>

<div class="article textile">
{{-page.content-}}
</div>

</div>
Loading