Better whitespace processing proposal about liqp HOT 14 CLOSED

Here is an update:

I'm almost ready with my template generator, which works now both as a npm module and an Eclipse plug-in. As such, I had to re-write the templates in a portable way.

The first change I had to make was to give up using global switches, and explicitly use -%}; global switches are not standard and each implementation may do them differently.

Then I had to sort out the issue with the extent of the white space stripping; I'm more and more convinced that stripping should not go past the first line terminator, neither for tags nor for output.

I experimented with some changes applied to your parser (eclipse-embed-cdt@ddb1dcf); the code is functional for my templates, but I'm not sure I got it right for {%- and {{-, so it may require some more work.

from liqp.

I'll further investigate and probably open an issue with the Shopify/liquid project, to see what the gurus think about this issue.

from liqp.

Hi @ilg-ul, thanks!

If I scan the changed code/grammar, withStripSpaceAroundTags(true) is only affected by this change right? The {%- ... -%} is unchanged. In that case, could you create a PR against this repo?

from liqp.

yeah, I had many problems and I had to experiment several solutions. none are great and I'm not sure the current solution I use in my forked repo is final.

as for now, the big issues I identified are:

• generally, stripping spaces should stop at the first NL, and do not affect previous/subsequent lines; I don't know how Ruby implements it, but if it strips previous/subsequent line there is absolutely no way to insert blank lines next to tags, in the output.
• withStripSpaceAroundTags() probably should be reworked, enabling it makes manual space control impossible; my workaround was to disable it for {{ variable }} and to juggle with {% tag %} to have different behaviour for previous/subsequent lines, which is very confusing.

for you to better understand, in a normal use case, {% assign v1 = 'b' %}a {{ v1 }} c should be rendered as a b c, and

line 1
{% if true %}

line 3

{% endif %}
line 5

should be rendered as

line 1

line 3

line 5

with the current implementation, if you enable the withStripSpaceAroundTags() you probably get abc and all/most lines concatenated, which is unusable.

without withStripSpaceAroundTags() things are a little better, you get a b c, but manually deciding where to use dash ({%- and/or -%}) is even trickier, there are combinations when it is simply impossible to preserve an empty line in a conditional block.

I suggest you investigate a solution to stop striping white spaces at the first new line; I'm a bit reluctant to do it, since I'm not sure I fully understand the implications.

from liqp.

Thanks for the elaborate explanation!

from liqp.

you're welcome! hope it helps.

from liqp.

After re-reading this again, I've decided not to change the behaviour of space-stripping (which, as far as I can see by testing, is the same as Ruby + liquid 4.0.0). There are more quirks in Liquid that could be improved upon, but I'd rather keep it as close as possible with Ruby's version than improve it only in Liqp.

Again, thank you for your insights!

from liqp.

I took a look at the original liquid tests, and indeed they skip all left/right whitespaces, but their samples are exclusively focused on html.

maybe I'm missing something, but what syntax whould you suggest for cases when I need to preserve an extra empty line?

#include <micro-os-plus/board.h>
{% if trace != 'none' -%}
#include <micro-os-plus/diag/trace.h>
{% endif -%}
             <--- empty line 1
#include <sysclock.h>
{% if content == 'blinky' -%}
#include <led.h>
{% endif -%}
             <--- empty line 2
#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

{% if language == 'cpp' -%}
using namespace os;
             <--- empty line 3
{% endif -%}

If -%} skips all whitespaces, the empty lines 1 & 2 disapear, and I found no way to preseve them.
I tried to move the - to the starting tag, but this simply move the problem in a different place, making empty line 3 disappear.

Probably selectively placing the - on the left or on the right, never in the position where an empty line is present, might do the trick, but maintaining such a file becomes pretty difficult.

from liqp.

maybe I'm missing something, but what syntax whould you suggest for cases when I need to preserve an extra empty line?

I am not suggesting anything :)

"How would you do it in Ruby + liquid?" would be my counter question.

I just ran the following Ruby code:

require 'liquid'

source = <<-STR
#include <micro-os-plus/board.h>
{% if trace != 'none' -%}
#include <micro-os-plus/diag/trace.h>
{% endif -%}

#include <sysclock.h>
{% if content == 'blinky' -%}
#include <led.h>
{% endif -%}

#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

{% if language == 'cpp' -%}
using namespace os;

{% endif -%}
STR

@template = Liquid::Template.parse(source)

result = @template.render()

printf('source=>>>%s<<<', source)
print("\n\n\n")
printf('result=>>>%s<<<', result)

and did the same with the current master of Liquid, and they both produce this:

source=>>>#include <micro-os-plus/board.h>
{% if trace != 'none' -%}
#include <micro-os-plus/diag/trace.h>
{% endif -%}

#include <sysclock.h>
{% if content == 'blinky' -%}
#include <led.h>
{% endif -%}

#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

{% if language == 'cpp' -%}
using namespace os;

{% endif -%}
<<<


result=>>>#include <micro-os-plus/board.h>
#include <micro-os-plus/diag/trace.h>
#include <sysclock.h>
#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

<<<

If the above is not your desired output, then how would you change the Ruby code? Again, I find it more important to render the same output as Liquid does than to render something that is more closely to what is expected or desired output. Or did I misunderstand?

from liqp.

I'm not going to change anything about how Liqp is currently handling this, but feel free to send a PR that does what you think is a better way. I'm more than happy to include it, just as long as it doesn't change the default behaviour.

from liqp.

If the above is not your desired output ...

Unfortunately it is not the desired output, it lacks the empty lines before sysclock.h and before stdint.h. :-(

I'm not generating code to be immediately compiled without being seen by humans, I'm generating projects to be used as starting points for more elaborate projects, so the empty lines are required, for readability reasons.

Or did I misunderstand?

It looks like I failed to ask the correct question.

I agree that compatibility with Ruby is important, but the problem is that the Ruby behaviour is optimized for HTML, where striping all empty lines is ok, since the code needs not be inspected by humans.

And I'm not asking to change your implementation.

Assuming that the current behaviour is the correct one, it results that my Liquid syntax is wrong.

So I'm asking if you have a suggestion how to write the liquid code such that both Ruby and your implementation will preserve the empty lines marked with arrows? If I remove the - completely, I get too many empty lines. If I add them, either to the left or to the right, lines that I want preserved are striped. :-(

from liqp.

So I'm asking if you have a suggestion how to write the liquid code such that both Ruby and your implementation will preserve the empty lines marked with arrows?

No, no suggestion, I thinks it's not possible in Liquid.

[...] but feel free to send a PR that does what you think is a better way. I'm more than happy to include it, just as long as it doesn't change the default behaviour.

Coming back to my remark above, I think there is a solution with little changes. You will need to use plain open- and close tags like this:

#include <micro-os-plus/board.h>
{% if trace != 'none' %}
#include <micro-os-plus/diag/trace.h>
{% endif %}

#include <sysclock.h>
{% if content == 'blinky' %}
#include <led.h>
{% endif %}

#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

{% if language == 'cpp' %}
using namespace os;

{% endif %}

and then do this:

String source = "#include <micro-os-plus/board.h>\n" +
        "{% if trace != 'none' %}\n" +
        "#include <micro-os-plus/diag/trace.h>\n" +
        "{% endif %}\n" +
        "\n" +
        "#include <sysclock.h>\n" +
        "{% if content == 'blinky' %}\n" +
        "#include <led.h>\n" +
        "{% endif %}\n" +
        "\n" +
        "#include <stdint.h>\n" +
        "#include <stdio.h>\n" +
        "\n" +
        "// ----------------------------------------------------------------------------\n" +
        "\n" +
        "{% if language == 'cpp' %}\n" +
        "using namespace os;\n" +
        "\n" +
        "{% endif %}\n";

ParseSettings settings = new ParseSettings.Builder()
        .withStripSpaceAroundTags(true, true)
        .build();

String rendered = Template.parse(source, settings).render();
System.out.printf(">>>\n%s<<<", rendered);

which will output:

>>>
#include <micro-os-plus/board.h>
#include <micro-os-plus/diag/trace.h>

#include <sysclock.h>

#include <stdint.h>
#include <stdio.h>

// ----------------------------------------------------------------------------

<<<

which, I believe, is what you're after (or at least close to).

This change can be found here: #82 and is pretty much 1 extra boolean flag the lexer is fed. This flag indicates, when in space-stripping mode, only a single line is to be stripped instead of all space-chars around the tag.

If it's not what you're after, feel free to continue on that branch and submit a PR of your own.

from liqp.

No, no suggestion, I thinks it's not possible in Liquid.

hmmm... that's bad... it means Liquid has a problem. :-(

which, I believe, is what you're after (or at least close to).

yes, this is the expected result.

This change can be found here: #82 and is pretty much 1 extra boolean flag the lexer is fed. This flag indicates, when in space-stripping mode, only a single line is to be stripped instead of all space-chars around the tag.

thank you, I'll think about this.

unfortunately I need the same liquid code to work in JavaScript, and there I do not have this extra boolean flag... interestingly enough, the JavaScript liquid module, when using the - tags, works as expected, it stops striping at the first line terminator.

from liqp.

Hmm, that is a shame.

I double checked the Ruby output and Liquid version, and also tried providing the variables trace and content whose absence might be causing issues, but the result is the same.

require 'liquid' # gem 'liquid', '~> 4.0.0'

source = <<-STR
#include <micro-os-plus/board.h>
{% if trace != 'none' -%}
#include <micro-os-plus/diag/trace.h>
{% endif -%}

#include <sysclock.h>
{% if content == 'blinky' -%}
#include <led.h>
{% endif -%}

#include <stdint.h>
#include <stdio.h>
STR

@template = Liquid::Template.parse(source)

result = @template.render({
  'trace' => 'mu',
  'content' => 'blinky'
})

printf(">>>\n%s<<<", result)

# output:
#
# >>>
# #include <micro-os-plus/board.h>
# #include <micro-os-plus/diag/trace.h>
# #include <sysclock.h>
# #include <led.h>
# #include <stdint.h>
# #include <stdio.h>
# <<<

from liqp.