include tag whitespace

[arnederuwe]

Hi,

When using the include tag, I would expect the contents of the included liquid file to render with the whitespace of the root file appended.
For example:
Class.liquid:

...
{% for col in Table.Columns %}
        {% include 'AutoProperty' %}
{% endfor %}
...

AutoProperty.liquid:

public {{ col.PropertyType }} {{ col.PropertyName }} { get; set; }

Currently outputs like this:

public string Name { get; set; }
public int PrincipalId { get; set; }
public int DiagramId { get; set; }
public int Version { get; set; }

where I would expect this:

        public string Name { get; set; }
        public int PrincipalId { get; set; }
        public int DiagramId { get; set; }
        public int Version { get; set; }

[hishamco]

As I can see from your code snippet before, seems the for statement can't preserve the spaces.

@sebastienros is this how it works?, or is it a bug? Coz I see DotLiquid preserve the spaces

[sebastienros]

The issue is that I strip whitespace between block tags by default. I think I documented it on the readme. There might be an option to disable it but then you will have to specify the behavior or each tag. Let's see what Hisham says.

…

On Nov 7, 2017 7:18 AM, "Hisham Bin Ateya" ***@***.***> wrote: Let me check that .. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#42 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABHJ7fzMcG6jetUes01cKKlw_uBfWdtKks5s0HTAgaJpZM4QUZF1> .

[hishamco]

For simplification we should preserve the whitespace, or add an option as you suggest before, but I don't think specifying the behavior per tag is a good option

[sebastienros]

Per tag is just from liquid specification... And it's already supported, look at the unit tests.

…

On Nov 7, 2017 8:09 AM, "Hisham Bin Ateya" ***@***.***> wrote: For simplification we should preserve the whitespace, or add an option as you suggest before, but I don't think specifying the behavior per tag is a good option — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABHJ7XblYGjT5LQwm5XRwKlvygF2-MFNks5s0ICegaJpZM4QUZF1> .

[hishamco]

Sounds good, let me check ..

[arnederuwe]

just fyi, dotLiquid supports this scenario only if the contents of my AutoProperty.liquid example are only one line.

If my AutoProperty.liquid would be this:

[Column]
public {{ col.PropertyType }} {{ col.PropertyName }} { get; set; }

The results in dotLiquid would be

        [Column]
public string Name { get; set; }
        [Column]
public int PrincipalId { get; set; }

I would expect multiline included files to be evenly whitespaced

[sebastienros]

if the contents of my templates are only one line

That's not what it does, it's just rendering the whitespace, for each time the block is rendered. Nothing to do with {% include %} or the fact that it's only one line. It happens to look how you would like because it does it.

What could be done though is to add a property to the include tag such that new lines are also padded. And the padded value could be global variable in your main template such that you can call it recursively.

We should still handle the whitespace issue that you have found. And look at liquid if it supports what I just described.

[sebastienros]

Sorry if I am being verbose on this thread, but it's important that we all understand the pros and cons of each solution.

Right now Fluid will strip the whitespace before a block tag content, which happens in this case. The idea is that in most cases we don't want to render indentations before code flow tags, like assign, if, for, ...

Take this example:

{% for i in (1..10) %}
  {% assign foo = i %}
{% endfor %}

Without this default behavior, we would get an output containing 10 empty lines, though nothing should be rendered.

You can take a look a this file for tests that show how it handles whitespace: https://github.com/sebastienros/fluid/blob/dev/Fluid.Tests/WhiteSpaceTests.cs

What I would suggest right now to solve your problem, is that we create a custom tag that would indent a section, like this:

{% indent: 4 %}
  {% include 'foo' %}
{% endindent %}

The result would be that every line in the block would be indented by 4 spaces. We would add an optional parameter with the string to repeat, like tabs.

[sebastienros]

/cc @RSuter I assume you would love that

[arnederuwe]

relevant liquid issue: Shopify/liquid#925

It seems liquid and dotLiquid behave exactly the same in this scenario.
Just a question, but why does liquid strip whitespaces by default? Is'nt that what {% -%} is for?
EDIT, I read your docs ;) and it indeed seems like a nice improvement over liquid, but I think the Include tag is the exception to this rule

I think your indent solution is a good idea, as it would keep the compatibility with liquid templates. Depending on how Shopify/liquid#925 turns out, you could add the new tag syntax discussed there and deprecate the indent tag, or keep both.

For me this isn't that big (urgent) of an issue, I'm currently using Roslyn at the end of my code generation to format everything nicely, but it would be nice to ditch that dependency

[sebastienros]

You are right, we still need to fix the fact that the whitespace is ignored in this case.

To behave exactly the same as the reference implementation we could have a flag. Let me check.

[hishamco]

Make an option such as EnableWhitespace will be handy

[sebastienros]

The 2.x version doesn't strip whitespaces by default anymore, and there are options to change the behavior for each type of tags.