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

Extra Spacing in HTML Source Due To Liquid Tag Indentation #162

Closed
jcrawford opened this Issue Dec 22, 2012 · 16 comments

Comments

Projects
None yet
9 participants
@jcrawford

jcrawford commented Dec 22, 2012

I have the following code

<div id="slider_module">
    <div id="slider_module_inner">
        <div class="miss_preloader_large" style="text-align:center;">
            <img src="http://www.josephcrawford.com/wp-content/themes/radiostation/assets/images/production/transparent.gif" style="background-image: url(http://www.josephcrawford.com/wp-content/themes/radiostation/assets/images/production/preloader.png);">
        </div>
        <div id="miss_flexslider">
            <ul class="slides">
                {% for post in site.posts %}
                    {% if post.is_review == true and post.image != nil limit:10 %}
                            <li>
                                <a href="{{ page.url }}" class="flex-imageLink">
                                    <img src="images/posts/featured/{{ post.image }}" title="" alt="{{ post.title }}" width="720" height="auto" />
                                    <div class="flex-caption">
                                        <h2 class="slider_title">{{ post.title }}</h2>
                                        <div class="slider_desc">{{ post.excerpt }} </div>
                                    </div>
                                </a>
                            </li>
                    {% endif %}
                {% endfor %}
            </ul>
        </div><!-- #miss_flexslider -->
        <div class="clearboth"></div>
    </div>
</div>

When I have my HTML source and Liquid tags indented properly for readability and I build the site with Jekyll then there appears to be A TON of whitespace in my HTML source. This is caused by the if statement being false so the indentation before the beginning {% if post.is_review == true and post.image != nil limit:10 %} and the ending {% endfor %} is being included into the output of the HTML source.

You can see a pastebin of my source to see just how much whitespace is added. I originally reported this with the Jekyll project and was informed that it was a Liquid issue as Jekyll just prints the output of what Liquid parses and returns. White Space Issue

Screen Shot 2012-12-21 at 8 39 39 PM

See the above screen shot to see what I mean. While the little bit of whitespace shown in the image may not seem like a big deal when it loops through 100 posts that do not match the if statement that is 100 lines of spaces for the indentation. In my site it leads to a ton of blanke white space only lines.

This may not seem like that big of an issue but to keep the tags on the same line screws with readability in my opinion. Not only does it make readability difficult it is actually increasing the size of the page thereby effecting the load times.

A work-around for this is to keep the endif and endfor on the same line such as the code below

<div id="slider_module">
    <div id="slider_module_inner">
        <div class="miss_preloader_large" style="text-align:center;">
            <img src="http://www.josephcrawford.com/wp-content/themes/radiostation/assets/images/production/transparent.gif" style="background-image: url(http://www.josephcrawford.com/wp-content/themes/radiostation/assets/images/production/preloader.png);">
        </div>
        <div id="miss_flexslider">
            <ul class="slides">
                {% for post in site.posts %}{% if post.is_review == true and post.image != nil limit:10 %}
                        <li>
                            <a href="{{ page.url }}" class="flex-imageLink">
                                <img src="images/posts/featured/{{ post.image }}" title="" alt="{{ post.title }}" width="720" height="auto" />
                                <div class="flex-caption">
                                    <h2 class="slider_title">{{ post.title }}</h2>
                                    <div class="slider_desc">{{ post.excerpt }} </div>
                                </div>
                            </a>
                        </li>
                {% endif %}{% endfor %}
            </ul>
        </div><!-- #miss_flexslider -->
        <div class="clearboth"></div>
    </div>
</div>
@peterwongpp

This comment has been minimized.

Show comment
Hide comment
@peterwongpp

peterwongpp Jan 2, 2013

+1

Is it possible to have something like this in erb?

{%- if ... %}

the minus after {% means remove this blank line after compiled

peterwongpp commented Jan 2, 2013

+1

Is it possible to have something like this in erb?

{%- if ... %}

the minus after {% means remove this blank line after compiled

@bcomnes

This comment has been minimized.

Show comment
Hide comment
@bcomnes

bcomnes Jan 18, 2013

This is super annoying and forces me to either show very little love to the resulting HTML, or try to wield massive one liner liquid monstrosities. Is there any way around this?

bcomnes commented Jan 18, 2013

This is super annoying and forces me to either show very little love to the resulting HTML, or try to wield massive one liner liquid monstrosities. Is there any way around this?

@nickpearson

This comment has been minimized.

Show comment
Hide comment
@nickpearson

nickpearson Jan 18, 2013

Contributor

It seems that the simplest way to achieve this might be to update Liquid's tag regex to allow newline characters inside of the tag. So instead of this code, which will produce clean output with no blank lines:

<ul>{% for person in people %}{% if person.visible? %}
  <li>{{ person.name }}</li>{% endif %}{% endfor %}
</ul>

... you could do this, which is more readable but would produce identical output:

<ul>{%
  for person in people %}{%
    if person.visible? %}
      <li>{{ person.name }}</li>{%
    endif %}{%
  endfor %}
</ul>

This seems simpler than handling {%- ... -%} where Liquid would be responsible for processing its own output to remove blank lines that once contained a {%- and -%}. If the tag regex is just modified to allow newlines, then the output wouldn't need to be reprocessed.

Contributor

nickpearson commented Jan 18, 2013

It seems that the simplest way to achieve this might be to update Liquid's tag regex to allow newline characters inside of the tag. So instead of this code, which will produce clean output with no blank lines:

<ul>{% for person in people %}{% if person.visible? %}
  <li>{{ person.name }}</li>{% endif %}{% endfor %}
</ul>

... you could do this, which is more readable but would produce identical output:

<ul>{%
  for person in people %}{%
    if person.visible? %}
      <li>{{ person.name }}</li>{%
    endif %}{%
  endfor %}
</ul>

This seems simpler than handling {%- ... -%} where Liquid would be responsible for processing its own output to remove blank lines that once contained a {%- and -%}. If the tag regex is just modified to allow newlines, then the output wouldn't need to be reprocessed.

@stomar

This comment has been minimized.

Show comment
Hide comment
@stomar

stomar Mar 31, 2013

Contributor

+1.

Liquid tags in my case cause blank lines at the beginning of an .rss file, leading to warnings about malformed XML:
error on line 3 at column 6: XML declaration allowed only at the start of the document. Writing the template in a way that avoids this is of course possible but pretty annoying.

Contributor

stomar commented Mar 31, 2013

+1.

Liquid tags in my case cause blank lines at the beginning of an .rss file, leading to warnings about malformed XML:
error on line 3 at column 6: XML declaration allowed only at the start of the document. Writing the template in a way that avoids this is of course possible but pretty annoying.

@hv15

This comment has been minimized.

Show comment
Hide comment
@hv15

hv15 Apr 17, 2013

+1

I like neat source code.

hv15 commented Apr 17, 2013

+1

I like neat source code.

@CoryG89

This comment has been minimized.

Show comment
Hide comment
@CoryG89

CoryG89 May 26, 2013

+1 Just came to this realization with the problem indenting causes. I would much rather do this in my template, but you'll end up with bad indentation in the final HTML:

{% if page.something %}
  <sometag></sometag>
{% endif %}

Instead your forced to do the following, which ends up making the template less readable, but then the indentation works out correctly in the final HTML source:

{% if page.something %}
<sometag></sometag>
{% endif %}

I'm hoping this won't always be the case, surely there is a decent solution for this.

CoryG89 commented May 26, 2013

+1 Just came to this realization with the problem indenting causes. I would much rather do this in my template, but you'll end up with bad indentation in the final HTML:

{% if page.something %}
  <sometag></sometag>
{% endif %}

Instead your forced to do the following, which ends up making the template less readable, but then the indentation works out correctly in the final HTML source:

{% if page.something %}
<sometag></sometag>
{% endif %}

I'm hoping this won't always be the case, surely there is a decent solution for this.

@fw42 fw42 referenced this issue Jun 26, 2013

Closed

Whitespace issues #216

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 3, 2013

Member

It looks like this specific issue (the one from the screenshot) might be solved by #218. Regarding introducing additional tags, please see #215.

Member

fw42 commented Jul 3, 2013

It looks like this specific issue (the one from the screenshot) might be solved by #218. Regarding introducing additional tags, please see #215.

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 3, 2013

Member

Closing for now...

Member

fw42 commented Jul 3, 2013

Closing for now...

@fw42 fw42 closed this Jul 3, 2013

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 3, 2013

Member

Actually, on second thought, #218 will not fix this, but we wont introduce new tags at the moment either (see #215). I might extend #218 in the future though to not only strip whitespace during parsing but also during rendering.

Member

fw42 commented Jul 3, 2013

Actually, on second thought, #218 will not fix this, but we wont introduce new tags at the moment either (see #215). I might extend #218 in the future though to not only strip whitespace during parsing but also during rendering.

@bcomnes

This comment has been minimized.

Show comment
Hide comment
@bcomnes

bcomnes Jul 5, 2013

Something like that would be great. We need to show love for both the template markup as well as the final html results :)

bcomnes commented Jul 5, 2013

Something like that would be great. We need to show love for both the template markup as well as the final html results :)

@nickpearson

This comment has been minimized.

Show comment
Hide comment
@nickpearson

nickpearson Jul 8, 2013

Contributor

@fw42 here's a quick regex that you could use to extend your work in #218 that would likely handle what you're looking for (trimming all that extra whitespace from non-blank blocks):

output.gsub(/[ \t]+$/, '').gsub(/(\r?\n)+/, "\n")

That will first trim all trailing whitespace on each line and then get rid of blank lines. It will leave leading (indentation) whitespace alone.

Contributor

nickpearson commented Jul 8, 2013

@fw42 here's a quick regex that you could use to extend your work in #218 that would likely handle what you're looking for (trimming all that extra whitespace from non-blank blocks):

output.gsub(/[ \t]+$/, '').gsub(/(\r?\n)+/, "\n")

That will first trim all trailing whitespace on each line and then get rid of blank lines. It will leave leading (indentation) whitespace alone.

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 8, 2013

Member

Thanks @nickpearson, will give it a thought!

Member

fw42 commented Jul 8, 2013

Thanks @nickpearson, will give it a thought!

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 8, 2013

Member

The only concern I have (and that's the reason why I didn't remove whitespace in #218 during rendering yet for blank blocks): I want whitespace inside of raw tags to not get stripped.

Member

fw42 commented Jul 8, 2013

The only concern I have (and that's the reason why I didn't remove whitespace in #218 during rendering yet for blank blocks): I want whitespace inside of raw tags to not get stripped.

@nickpearson

This comment has been minimized.

Show comment
Hide comment
@nickpearson

nickpearson Jul 11, 2013

Contributor

This may be further than you want to take this, but keeping the whitespace inside the raw tags could be done the same way GitHub Flavored Markdown handles <pre> tags. It pulls out each <pre> and replaces it with an MD5 hash of the tag's content (and stores the tag's content in a Ruby hash with the MD5 as its key). After the rest of the content is processed, the MD5 hashes are replaced with the original <pre> tags.

So, this:

hi
<pre>some code</pre>
done

becomes this during processing:

hi
gfm-extraction-c7951391aa93bc9a21e91b41c98f6917
done

with the extractions hash containing:

{ "c7951391aa93bc9a21e91b41c98f6917" => "<pre>some code</pre>" }

You can see this in lines 5-10 and 23-25 in the gist shown at the bottom of the GitHub Flavored Markdown page.

Since the extraction/replacement code is pretty short and simple, it could be used to keep raw tags exactly as they are while trimming the extra whitespace around them.

Contributor

nickpearson commented Jul 11, 2013

This may be further than you want to take this, but keeping the whitespace inside the raw tags could be done the same way GitHub Flavored Markdown handles <pre> tags. It pulls out each <pre> and replaces it with an MD5 hash of the tag's content (and stores the tag's content in a Ruby hash with the MD5 as its key). After the rest of the content is processed, the MD5 hashes are replaced with the original <pre> tags.

So, this:

hi
<pre>some code</pre>
done

becomes this during processing:

hi
gfm-extraction-c7951391aa93bc9a21e91b41c98f6917
done

with the extractions hash containing:

{ "c7951391aa93bc9a21e91b41c98f6917" => "<pre>some code</pre>" }

You can see this in lines 5-10 and 23-25 in the gist shown at the bottom of the GitHub Flavored Markdown page.

Since the extraction/replacement code is pretty short and simple, it could be used to keep raw tags exactly as they are while trimming the extra whitespace around them.

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 11, 2013

Member

Huh, that's a very interesting yet simple idea! @boourns, what do you think?

Member

fw42 commented Jul 11, 2013

Huh, that's a very interesting yet simple idea! @boourns, what do you think?

@narthur

This comment has been minimized.

Show comment
Hide comment
@narthur

narthur Mar 24, 2016

Any idea when this might be reopened?

narthur commented Mar 24, 2016

Any idea when this might be reopened?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment