[Review] Include block improvements

[yonaskolb]

Fixes #95

This adds 2 features to the include block including tests:

1. If the include block is indented with any whitespace, any content in the included file is indented by the same amount
2. Optional argument using which lets you pass a context to the included file using the format: {% include "myFile.stencil" using myContext %}

[yonaskolb]

Any comments on this @kylef?

[ilyapuchka]

+1

[ilyapuchka]

@kylef this looks good to me and feels like very useful fix. Any chance you can look into it any time soon or you ok with me merging it? =)

[yonaskolb]

Any updates on this?

[ilyapuchka]

I was looking into this and I think there might be an issue with this implementation. What if include tag line is prefixed with something? i.e.

Something
  Something:  {% include \"include.html\" %}

If I understand the logic here you are getting the last line of the text preceding include tag, so this will be " Something: ". And then you are trimming everything which is not a whitespace. In this case nothing will be trimmed, so you will write to the indent the whole string, which will then add it to each line of included content.

Something
  Something:  first line
  Something:  second line

Right?

Also what do you think should be behaviour for indentation in this case. Should the second line of included content be indented at all? And if it should then how much? To match the offset of first line of included text or to match the offset of previous line in template? I.e. it should be

Something
  Something:  first line
              second line

or it should be

Something
  Something:  first line
  second line

? Should we give user an option to control this?

Also there is this pr that touches indentation too, so that if you do {%+ include ... %} it should keep spaces before tag. So this might conflict with each other.

I would therefore extract another feature that you've implemented here, for passing sub context to included template, into separate PR and maybe rethink indentation implementation. Maybe it should be done only after #92

@kylef what do you think?

[ilyapuchka]

Maybe alternative solution for this could be indent node that will wrap included content (or any other content, i.e. result of macro call) and will define how all the content lines should be indented. Jinja2 has indent filter for that, but that will require to render content into variable to which the filter can be then applied, which is not yet supported by Stencil (though it is implemented in StencilSwiftKit). I think we can do both, node and filter to cover all scenarios, similar to how we have filter node in addition to filters applicable to variables.

@kylef thoughts?

[ilyapuchka]

Here is what I ended up with #188 , there was no need for a node, only for a filter, thanks to filter tag.

[yonaskolb]

Closing this in favour of #188 in terms of indenting.

I might open a new PR for passing the context however. It seems the direction recently is to go with unnamed arguments, is that preferred to using?